fix tree sorting

Assigns the weight in the preorder traversal of the tree. Now one without every
object (e.g. missing motions/items) still have the correct sorting. Intorduces
the level attribute of items giving the amount of parents in the agenda. This
allows to reduce complexits in the client.
This commit is contained in:
FinnStutzenstein 2019-05-13 15:50:27 +02:00
parent b3c2b5f899
commit 10c329da8d
17 changed files with 69 additions and 143 deletions

View File

@ -1,6 +1,6 @@
import { Injectable } from '@angular/core'; import { Injectable } from '@angular/core';
import { tap, map } from 'rxjs/operators'; import { map } from 'rxjs/operators';
import { Observable } from 'rxjs'; import { Observable } from 'rxjs';
import { TranslateService } from '@ngx-translate/core'; import { TranslateService } from '@ngx-translate/core';
@ -12,7 +12,7 @@ import { DataStoreService } from '../../core-services/data-store.service';
import { HttpService } from 'app/core/core-services/http.service'; import { HttpService } from 'app/core/core-services/http.service';
import { Item } from 'app/shared/models/agenda/item'; import { Item } from 'app/shared/models/agenda/item';
import { ViewItem } from 'app/site/agenda/models/view-item'; import { ViewItem } from 'app/site/agenda/models/view-item';
import { TreeService, TreeIdNode } from 'app/core/ui-services/tree.service'; import { TreeIdNode } from 'app/core/ui-services/tree.service';
import { BaseAgendaViewModel } from 'app/site/base/base-agenda-view-model'; import { BaseAgendaViewModel } from 'app/site/base/base-agenda-view-model';
import { ViewModelStoreService } from 'app/core/core-services/view-model-store.service'; import { ViewModelStoreService } from 'app/core/core-services/view-model-store.service';
import { BaseViewModel } from 'app/site/base/base-view-model'; import { BaseViewModel } from 'app/site/base/base-view-model';
@ -48,8 +48,7 @@ export class ItemRepositoryService extends BaseRepository<ViewItem, Item> {
viewModelStoreService: ViewModelStoreService, viewModelStoreService: ViewModelStoreService,
translate: TranslateService, translate: TranslateService,
private httpService: HttpService, private httpService: HttpService,
private config: ConfigService, private config: ConfigService
private treeService: TreeService
) { ) {
super(DS, dataSend, mapperService, viewModelStoreService, translate, Item, [ super(DS, dataSend, mapperService, viewModelStoreService, translate, Item, [
Topic, Topic,
@ -152,29 +151,6 @@ export class ItemRepositoryService extends BaseRepository<ViewItem, Item> {
await this.httpService.post('/rest/agenda/item/sort/', data); await this.httpService.post('/rest/agenda/item/sort/', data);
} }
/**
* Add custom hook into the observables. The ViewItems get a virtual agendaListWeight (a sequential number)
* for the agenda topic order, and a virtual level for the hierarchy in the agenda list tree. Both values can be used
* for sorting and ordering instead of dealing with the sort parent id and weight.
*
* @override
*/
public getViewModelListObservable(): Observable<ViewItem[]> {
return super.getViewModelListObservable().pipe(
tap(items => {
const iterator = this.treeService.traverseItems(items, 'weight', 'parent_id');
let m: IteratorResult<ViewItem>;
let virtualWeightCounter = 0;
while (!(m = iterator.next()).done) {
m.value.agendaListWeight = virtualWeightCounter++;
m.value.agendaListLevel = m.value.parent_id
? this.getViewModel(m.value.parent_id).agendaListLevel + 1
: 0;
}
})
);
}
/** /**
* Calculates the estimated end time based on the configured start and the * Calculates the estimated end time based on the configured start and the
* sum of durations of all agenda items * sum of durations of all agenda items

View File

@ -3,13 +3,12 @@ import { DomSanitizer, SafeHtml } from '@angular/platform-browser';
import { TranslateService } from '@ngx-translate/core'; import { TranslateService } from '@ngx-translate/core';
import { Observable } from 'rxjs'; import { Observable } from 'rxjs';
import { tap, map } from 'rxjs/operators'; import { map } from 'rxjs/operators';
import { Category } from 'app/shared/models/motions/category'; import { Category } from 'app/shared/models/motions/category';
import { ChangeRecoMode, ViewMotion } from 'app/site/motions/models/view-motion'; import { ChangeRecoMode, ViewMotion } from 'app/site/motions/models/view-motion';
import { CollectionStringMapperService } from '../../core-services/collection-string-mapper.service'; import { CollectionStringMapperService } from '../../core-services/collection-string-mapper.service';
import { ConfigService } from 'app/core/ui-services/config.service'; import { ConfigService } from 'app/core/ui-services/config.service';
import { DataSendService } from '../../core-services/data-send.service'; import { DataSendService } from '../../core-services/data-send.service';
import { DataStoreService } from '../../core-services/data-store.service'; import { DataStoreService } from '../../core-services/data-store.service';
import { DiffLinesInParagraph, DiffService, LineRange, ModificationType } from '../../ui-services/diff.service'; import { DiffLinesInParagraph, DiffService, LineRange, ModificationType } from '../../ui-services/diff.service';
@ -21,7 +20,7 @@ import { Motion } from 'app/shared/models/motions/motion';
import { MotionBlock } from 'app/shared/models/motions/motion-block'; import { MotionBlock } from 'app/shared/models/motions/motion-block';
import { MotionChangeRecommendation } from 'app/shared/models/motions/motion-change-reco'; import { MotionChangeRecommendation } from 'app/shared/models/motions/motion-change-reco';
import { MotionPoll } from 'app/shared/models/motions/motion-poll'; import { MotionPoll } from 'app/shared/models/motions/motion-poll';
import { TreeService, TreeIdNode } from 'app/core/ui-services/tree.service'; import { TreeIdNode } from 'app/core/ui-services/tree.service';
import { User } from 'app/shared/models/users/user'; import { User } from 'app/shared/models/users/user';
import { ViewMotionChangeRecommendation } from 'app/site/motions/models/view-change-recommendation'; import { ViewMotionChangeRecommendation } from 'app/site/motions/models/view-change-recommendation';
import { ViewMotionAmendedParagraph } from 'app/site/motions/models/view-motion-amended-paragraph'; import { ViewMotionAmendedParagraph } from 'app/site/motions/models/view-motion-amended-paragraph';
@ -44,7 +43,7 @@ import { ViewPersonalNote } from 'app/site/users/models/view-personal-note';
import { OperatorService } from 'app/core/core-services/operator.service'; import { OperatorService } from 'app/core/core-services/operator.service';
import { CollectionIds } from 'app/core/core-services/data-store-update-manager.service'; import { CollectionIds } from 'app/core/core-services/data-store-update-manager.service';
type SortProperty = 'callListWeight' | 'identifier'; type SortProperty = 'weight' | 'identifier';
/** /**
* Describes the single paragraphs from the base motion. * Describes the single paragraphs from the base motion.
@ -122,7 +121,6 @@ export class MotionRepositoryService extends BaseAgendaContentObjectRepository<V
private readonly sanitizer: DomSanitizer, private readonly sanitizer: DomSanitizer,
private readonly lineNumbering: LinenumberingService, private readonly lineNumbering: LinenumberingService,
private readonly diff: DiffService, private readonly diff: DiffService,
private treeService: TreeService,
private operator: OperatorService private operator: OperatorService
) { ) {
super(DS, dataSend, mapperService, viewModelStoreService, translate, Motion, [ super(DS, dataSend, mapperService, viewModelStoreService, translate, Motion, [
@ -306,25 +304,6 @@ export class MotionRepositoryService extends BaseAgendaContentObjectRepository<V
} }
} }
/**
* Add custom hook into the observables. The motions get a virtual weight (a sequential number) for the
* call list order. One can just sort for this number instead of dealing with the sort parent id and weight.
*
* @override
*/
public getViewModelListObservable(): Observable<ViewMotion[]> {
return super.getViewModelListObservable().pipe(
tap(motions => {
const iterator = this.treeService.traverseItems(motions, 'weight', 'sort_parent_id');
let m: IteratorResult<ViewMotion>;
let virtualWeightCounter = 0;
while (!(m = iterator.next()).done) {
m.value.callListWeight = virtualWeightCounter++;
}
})
);
}
/** /**
* Set the state of a motion * Set the state of a motion
* *
@ -934,9 +913,9 @@ export class MotionRepositoryService extends BaseAgendaContentObjectRepository<V
if (a[this.sortProperty] === b[this.sortProperty]) { if (a[this.sortProperty] === b[this.sortProperty]) {
return this.languageCollator.compare(a.title, b.title); return this.languageCollator.compare(a.title, b.title);
} else { } else {
if (this.sortProperty === 'callListWeight') { if (this.sortProperty === 'weight') {
// handling numerical values // handling numerical values
return a.callListWeight - b.callListWeight; return a.weight - b.weight;
} else { } else {
return this.languageCollator.compare(a[this.sortProperty], b[this.sortProperty]); return this.languageCollator.compare(a[this.sortProperty], b[this.sortProperty]);
} }

View File

@ -179,25 +179,6 @@ export class TreeService {
return getChildren(parentItems); return getChildren(parentItems);
} }
/**
* Traverses the given tree in pre order.
*
* @param tree The tree to traverse
* @returns An iterator for all items in the right order.
*/
public *traverseTree<T>(tree: OSTreeNode<T>[]): Iterator<T> {
const nodesToVisit = tree.reverse();
while (nodesToVisit.length > 0) {
const node = nodesToVisit.pop();
if (node.children) {
node.children.reverse().forEach(n => {
nodesToVisit.push(n);
});
}
yield node.item;
}
}
/** /**
* Removes `item` from the tree. * Removes `item` from the tree.
* *
@ -217,25 +198,6 @@ export class TreeService {
}); });
} }
/**
* Traverses items in pre-order givem (implicit) by the weight and parentId.
*
* Just builds the tree with `makeTree` and get the iterator from `traverseTree`.
*
* @param items All items to traverse
* @param weightKey The key giving access to the weight property
* @param parentIdKey The key giving access to the parentId property
* @returns An iterator for all items in the right order.
*/
public traverseItems<T extends Identifiable & Displayable>(
items: T[],
weightKey: keyof T,
parentIdKey: keyof T
): Iterator<T> {
const tree = this.makeTree(items, weightKey, parentIdKey);
return this.traverseTree(tree);
}
/** /**
* Reduce a list of items to nodes independent from each other in a given * Reduce a list of items to nodes independent from each other in a given
* branch of a tree * branch of a tree

View File

@ -40,6 +40,7 @@ export class Item extends BaseModel<Item> {
public content_object: ContentObject; public content_object: ContentObject;
public weight: number; public weight: number;
public parent_id: number; public parent_id: number;
public level: number;
public constructor(input?: any) { public constructor(input?: any) {
super(Item.COLLECTIONSTRING, input); super(Item.COLLECTIONSTRING, input);

View File

@ -43,7 +43,7 @@
<ng-container matColumnDef="title"> <ng-container matColumnDef="title">
<mat-header-cell *matHeaderCellDef mat-sort-header>Topic</mat-header-cell> <mat-header-cell *matHeaderCellDef mat-sort-header>Topic</mat-header-cell>
<mat-cell *matCellDef="let item"> <mat-cell *matCellDef="let item">
<div [ngStyle]="{ 'margin-left': item.agendaListLevel * 25 + 'px' }"> <div [ngStyle]="{ 'margin-left': item.level * 25 + 'px' }">
<span *ngIf="item.closed"> <mat-icon class="done-check">check</mat-icon> </span> <span *ngIf="item.closed"> <mat-icon class="done-check">check</mat-icon> </span>
<span class="table-view-list-title">{{ item.getListTitle() }}</span> <span class="table-view-list-title">{{ item.getListTitle() }}</span>
</div> </div>

View File

@ -127,7 +127,7 @@ export class AgendaListComponent extends ListViewBaseComponent<ViewItem, Item> i
protected onFilter(): void { protected onFilter(): void {
this.filterService.filter().subscribe(newAgendaItems => { this.filterService.filter().subscribe(newAgendaItems => {
newAgendaItems.sort((a, b) => a.agendaListWeight - b.agendaListWeight); newAgendaItems.sort((a, b) => a.weight - b.weight);
this.dataSource.data = newAgendaItems; this.dataSource.data = newAgendaItems;
this.checkSelection(); this.checkSelection();
}); });

View File

@ -70,7 +70,7 @@ export class AgendaSortComponent extends BaseViewComponent implements CanCompone
public seenNodes: [number, number] = [0, 0]; public seenNodes: [number, number] = [0, 0];
/** /**
* All agendaItems sorted by their virtual weight {@link ViewItem.agendaListWeight} * All agendaItems sorted by their weight {@link ViewItem.weight}
*/ */
public itemsObservable: Observable<ViewItem[]>; public itemsObservable: Observable<ViewItem[]>;

View File

@ -10,19 +10,6 @@ export class ViewItem extends BaseViewModel {
private _item: Item; private _item: Item;
private _contentObject: BaseAgendaViewModel; private _contentObject: BaseAgendaViewModel;
/**
* virtual weight defined by the order in the agenda tree, representing a shortcut to sorting by
* weight, parent_id and the parents' weight(s)
* TODO will be accurate if the viewMotion is observed via {@link getSortedViewModelListObservable}, else, it will be undefined
*/
public agendaListWeight: number;
/**
* The amount of parents in the agenda list tree.
* TODO will be accurate if the viewMotion is observed via {@link getSortedViewModelListObservable}, else, it will be undefined
*/
public agendaListLevel: number;
public get item(): Item { public get item(): Item {
return this._item; return this._item;
} }
@ -66,6 +53,10 @@ export class ViewItem extends BaseViewModel {
return this.item.comment; return this.item.comment;
} }
public get level(): number {
return this.item.level;
}
/** /**
* Gets the string representation of the item type * Gets the string representation of the item type
* @returns The visibility for this item, as defined in {@link itemVisibilityChoices} * @returns The visibility for this item, as defined in {@link itemVisibilityChoices}

View File

@ -65,12 +65,6 @@ export class ViewMotion extends BaseAgendaViewModel implements Searchable {
protected _changeRecommendations: ViewMotionChangeRecommendation[]; protected _changeRecommendations: ViewMotionChangeRecommendation[];
public personalNote?: PersonalNoteContent; public personalNote?: PersonalNoteContent;
/**
* Is set by the repository; this is the order of the flat call list given by
* the properties weight and sort_parent_id
*/
public callListWeight: number;
public get motion(): Motion { public get motion(): Motion {
return this._motion; return this._motion;
} }

View File

@ -68,7 +68,7 @@ export class CallListComponent extends BaseViewComponent implements CanComponent
this.motionsObservable = this.motionRepo.getViewModelListObservable(); this.motionsObservable = this.motionRepo.getViewModelListObservable();
this.motionsObservable.subscribe(motions => { this.motionsObservable.subscribe(motions => {
// Sort motions and make a copy, so it will stay sorted. // Sort motions and make a copy, so it will stay sorted.
this.motions = motions.map(x => x).sort((a, b) => a.callListWeight - b.callListWeight); this.motions = motions.map(x => x).sort((a, b) => a.weight - b.weight);
}); });
} }

View File

@ -123,7 +123,7 @@ export class MotionCsvExportService {
/** /**
* Exports the call list. * Exports the call list.
* *
* @param motions All motions in the CSV. They should be ordered by callListWeight correctly. * @param motions All motions in the CSV. They should be ordered by weight correctly.
*/ */
public exportCallList(motions: ViewMotion[]): void { public exportCallList(motions: ViewMotion[]): void {
this.csvExport.export( this.csvExport.export(

View File

@ -581,7 +581,7 @@ export class MotionPdfService {
* @returns definitions ready to be opened or exported via {@link PdfDocumentService} * @returns definitions ready to be opened or exported via {@link PdfDocumentService}
*/ */
public callListToDoc(motions: ViewMotion[]): object { public callListToDoc(motions: ViewMotion[]): object {
motions.sort((a, b) => a.callListWeight - b.callListWeight); motions.sort((a, b) => a.weight - b.weight);
const title = { const title = {
text: this.translate.instant('Call list'), text: this.translate.instant('Call list'),
style: 'title' style: 'title'

View File

@ -12,10 +12,10 @@ import { _ } from 'app/core/translate/translation-marker';
}) })
export class MotionSortListService extends BaseSortListService<ViewMotion> { export class MotionSortListService extends BaseSortListService<ViewMotion> {
public sortOptions: OsSortingDefinition<ViewMotion> = { public sortOptions: OsSortingDefinition<ViewMotion> = {
sortProperty: 'callListWeight', sortProperty: 'weight',
sortAscending: true, sortAscending: true,
options: [ options: [
{ property: 'callListWeight', label: 'Call list' }, { property: 'weight', label: 'Call list' },
{ property: 'identifier' }, { property: 'identifier' },
{ property: 'title' }, { property: 'title' },
{ property: 'submitters' }, { property: 'submitters' },

View File

@ -318,6 +318,20 @@ class Item(RESTModelMixin, models.Model):
self.parent is not None and self.parent.is_hidden() self.parent is not None and self.parent.is_hidden()
) )
@property
def level(self):
"""
Returns the level in agenda (=tree of all items). Level 0 means this
item is a root item in the agenda. Level 1 indicates that the parent is
a root item, level 2 that the parent's parent is a root item and so on.
Attention! This executes one query for each ancestor of the item.
"""
if self.parent is None:
return 0
else:
return self.parent.level + 1
def get_next_speaker(self): def get_next_speaker(self):
""" """
Returns the speaker object of the speaker who is next. Returns the speaker object of the speaker who is next.

View File

@ -61,4 +61,5 @@ class ItemSerializer(ModelSerializer):
"content_object", "content_object",
"weight", "weight",
"parent", "parent",
"level",
) )

View File

@ -68,6 +68,8 @@ class ItemViewSet(
def update(self, *args, **kwargs): def update(self, *args, **kwargs):
""" """
Customized view endpoint to update all children if the item type has changed. Customized view endpoint to update all children if the item type has changed.
We do not check the level (affected by changing the parent) in fact that this
change is currentl only done via the sort view.
""" """
old_type = self.get_object().type old_type = self.get_object().type

View File

@ -46,8 +46,8 @@ class TreeSortMixin:
self, request: Any, model: models.Model, weight_key: str, parent_id_key: str self, request: Any, model: models.Model, weight_key: str, parent_id_key: str
) -> None: ) -> None:
""" """
Sorts the all model objects represented in a tree of ids. The request data should be a list (the root) Sorts the all model objects represented in a tree of ids. The request data should
of all main agenda items. Each node is a dict with an id and optional children: be a list (the root) of all main models. Each node is a dict with an id and optional children:
{ {
id: <the id> id: <the id>
children: [ children: [
@ -55,56 +55,62 @@ class TreeSortMixin:
] ]
} }
Every id has to be given. Every id has to be given.
This function traverses this tree in preorder to assign the weight. So even if a client
does not have every model, the remaining models are sorted correctly.
""" """
if not isinstance(request.data, list): if not isinstance(request.data, list):
raise ValidationError("The data must be a list.") raise ValidationError("The data must be a list.")
# get all item ids to verify, that the user send all ids. # get all item ids to verify, that the user send all ids.
all_item_ids = set(model.objects.all().values_list("pk", flat=True)) all_model_ids = set(model.objects.all().values_list("pk", flat=True))
ids_found: Set[int] = set() # Set to save all found ids.
fake_root: Dict[str, Any] = {"id": None, "children": []}
fake_root["children"].extend(
request.data
) # this will prevent mutating the request data.
# The stack where all nodes to check are saved. Invariant: Each node # The stack where all nodes to check are saved. Invariant: Each node
# must be a dict with an id, a parent id (may be None for the root # must be a dict with an id, a parent id (may be None for the root
# layer) and a weight. # layer) and a weight.
nodes_to_check = [] nodes_to_check = [fake_root]
ids_found: Set[int] = set() # Set to save all found ids.
# Insert all root nodes.
for index, node in enumerate(request.data):
if not isinstance(node, dict) or not isinstance(node.get("id"), int):
raise ValidationError("node must be a dict with an id as integer")
node[parent_id_key] = None
node[weight_key] = index
nodes_to_check.append(node)
# Traverse and check, if every id is given, valid and there are no duplicate ids. # Traverse and check, if every id is given, valid and there are no duplicate ids.
weight = 1
while len(nodes_to_check) > 0: while len(nodes_to_check) > 0:
node = nodes_to_check.pop() node = nodes_to_check.pop()
id = node["id"] id = node["id"]
if id in ids_found: if id is not None: # exclude the fake_root
raise ValidationError(f"Duplicate id: {id}") node[weight_key] = weight
if id not in all_item_ids: weight += 1
raise ValidationError(f"Id does not exist: {id}") if id in ids_found:
raise ValidationError(f"Duplicate id: {id}")
if id not in all_model_ids:
raise ValidationError(f"Id does not exist: {id}")
ids_found.add(id)
ids_found.add(id)
# Add children, if exist. # Add children, if exist.
if isinstance(node.get("children"), list): if isinstance(node.get("children"), list):
for index, child in enumerate(node["children"]): node["children"].reverse()
for child in node["children"]:
# ensure invariant for nodes_to_check # ensure invariant for nodes_to_check
if not isinstance(node, dict) or not isinstance( if not isinstance(child, dict) or not isinstance(
node.get("id"), int child.get("id"), int
): ):
raise ValidationError( raise ValidationError(
"node must be a dict with an id as integer" "child must be a dict with an id as integer"
) )
child[parent_id_key] = id child[parent_id_key] = id
child[weight_key] = index
nodes_to_check.append(child) nodes_to_check.append(child)
if len(all_item_ids) != len(ids_found): if len(all_model_ids) != len(ids_found):
raise ValidationError( raise ValidationError(
f"Did not recieved {len(all_item_ids)} ids, got {len(ids_found)}." f"Did not recieved {len(all_model_ids)} ids, got {len(ids_found)}."
) )
# Do the actual update:
nodes_to_update = [] nodes_to_update = []
nodes_to_update.extend( nodes_to_update.extend(
request.data request.data