Sorting by wbsValue field does not work correctly with applyChangeset method

[chuckn0rris]

Works as expected when using taskStore.add(task), but doesn't if used project.applyProjectChanges(change).

Forum post

We're trying to build live updates into our app and we're using the applyProjectChanges() method to apply incoming changes from other clients. We're not seeing the expected behavior from reapplySortersOnAdd or reapplyFilterOnAdd. Specifically, the filter seems to get applied to incoming changes regardless the setting for reapplyFilterOnAdd, and the sorter is not re-applied when reapplySortersOnAdd is true.

I put together a little demo of the issue by:

copying the basic example
changing the load url to example.json
turning on the filterBar feature
adding an initial sort and filter
adding a runChange(nameOfTask) function on window so we can simulate an incoming change in the console.

Code is in the attached zip file which can be expanded and dropped in the examples directory and then run like the other Bryntum examples.

Loading the page and entering:

runChange("foo")

in the console results in foo not showing in the gantt (it doesn't match the 'b' filter). Reloading and entering:

runChange("b foo")

in the console results in "b foo" showing in the gantt because it does match the filter. In the code, reapplyFilterOnAdd and reapplyFilterOnUpdate are both set to false. It seems like that config doesn't apply to changes coming in via applyProjectChanges, since clearly filtering is happening with it set to false.

I've set reapplySortersOnAdd to true and expect when you apply the change for "b foo" above, that the gannt would resort by WBS. and I would see rows 1, 3 ,4. Instead, I still see an arrow indicating the sort is on, but the rows are not sorted correctly as the order is 1, 4, 3.

The goal is to get filtering and sorting to both be applied after incoming project changes. How is that done? Also, it would be nice if there were a reapplySortersOnUpdate so that both Adds and Updates can both get filtered and sorted.

[canonic-epicure]

Hello. The applyProjectChanges uses another store update method - applyChangeset which is parallel to regular store.add()/remove(). The reapplyFilterOnAdd/Update options are not used for it. For this method, the filtering/sorting behavior is controlled with applyChangesetFilterSortTarget option which generally matches your expectations that both filtering and sorting should be applied on the incoming changes.

So if we rephrase the ticket in terms of applyChangesetFilterSortTarget, the issue is that sorting by "Wbs" code works incorrectly? Can you please check if sorting will be correct, if you sort by another field?

Also FYI - I see you have both wbsValue and orderedParentIndex values in the incoming changes. I'm not quite sure yet, but very likely, the wbsValue update will be ignored, since orderedParentIndex defines the position of the record, by which the wbsValue is calculated.

[canonic-epicure]

Perhaps related? #6090

[drolin-suffolk]

Here is a screenshot with the example changed to sort by orderedParentIndex:

You can see the console at the bottom. applyChangesetFilterSortTarget is set to the default "changes". The filter part is doing the right thing, the added record meets the filter criteria and it's added to the filtered set. The UI is showing that it's sorted by orderedParentIndex, but clearly it's not.

Here is a screenshot of the docs:

I'm assuming that sort is getting applied to the filtered set and not just the changeset. (In the example there are 3 rows in the filter set, and just 1 row in the changeset, the one row just added). Can you confirm whether or not sort applies to the changes only, and not the whole set of filtered rows?

Cleary we're expecting sorting to apply to the whole set of rows in the UI. If it's just sorting the one row that got "filtered in", that would explain the issue, and we'd like to see that behavior changed so that the full set gets sorted.

[canonic-epicure]

You've chosen the orderedParentIndex which is also a bit "system" field, in the same way as wbsValue. Perhaps you can try with regular simple field, like name?

Can you confirm whether or not sort applies to the changes only, and not the whole set of filtered rows?

I'll be asking the team and looking through the code to figure this out. In general, the applyChangeset path was implemented according to the previous requirements of your company, so it should be matching those.

I'll be posting the updates here.

[drolin-suffolk]

I tried name as you suggest, and slightly modified the names to correspond with the expected WBS after the change. These indeed sort correctly. I get that that orderedParentIndex is a system field, but I chose that one because it's going to sort things the same as WBS, which is our ultimate goal. I assumed sorting worked for any field. I guess your earlier comment is correct and we could change this to: "...the issue is that sorting by "Wbs" code works incorrectly".

[canonic-epicure]

Thanks for checking! I'll take the sorting by wbs as a first immediate goal for this ticket. We can review the issue again after fixing that first.

[drolin-suffolk]

Thanks!

[canonic-epicure]

Hm.. In my experiment, the sorting does not seem to be applied when adding new records. I'm adding a new record w/o orderedParentIndex and filtering is disabled. Record is just appended to the end.

[drolin-suffolk]

Here's my code. I just copied the basic folder in examples and modified app.js as well as adding an example.json file with the small set of data. The page loads with a filter and sort applied, and I just run:

runChange("3 b foo")

to add another task (named 3 b foo). Code for that is at the bottom of app,js

I only ran one trial. I'll try a few more variations.

basicReapplyIssue.zip

[canonic-epicure]

Pardon, it seems sorting is still applied if reapplySortersOnAdd is enabled.

[drolin-suffolk]

No worries. I did 3 trials adding to the top, middle, and bottom and all worked as expected. It would be nice to get to one set of config items (applyChangesetFilterSortTarget vs reapplySortersOnAdd)

[canonic-epicure]

So this issue was resolved and sorting by wbsValue should work now. Would be great if you could verify it in the tomorrow nightly build. Or, you can wait till 5.5.3.

[drolin-suffolk]

I grabbed the gantt-2023-09-05-release nightly build and tested this. I am not seeing the correct behavior. Rather than re-sorting by WBS, I see the added task always show up in the bottom of the view. I have attached a small example:

basicReapplyIssue.zip

I just put this in the examples folder and run. In the console, I enter:

runChange("top")
// or runChange("middle")
// or runChange("bottom")

You need to re-load the page between tests. I always see the added task at the bottom. Are there an new config properties that need to be set?

[canonic-epicure]

Sorry for delay, we'll be checking this soon.

[drolin-suffolk]

Any update?

[canonic-epicure]

Sorry for delay, busy with other tasks. In your test case, do you update the orderedParentIndex for all events? For added and for the rest? Also, did you enable the reapplySortingOnAdd (its required for sorting to be applied after applyChangeset)?

[drolin-suffolk]

I am updating orderedParentIndex if it changes. If there are 5 events under the same parent orderedParentIndex 0-4, then if I add an orderedParentIndex 5 it's just an add. If the new one is orderedParentIndex 3, then the existing 3 and 4 get an update, etc. Are you suggesting I need to construct an update for the entire tree even if much of it stays the same?

I have reapplySortersOnAdd set to true.

[canonic-epicure]

Ok, all looks fine then. No, update for the whole tree is not necessary. Its weird that it does not work, since our test for this case passes. I'll be checking it soon.

[drolin-suffolk]

If you provide the file path and name for the test, I can take a look and see if I spot something different from my test case.

[canonic-epicure]

Sorry, I just checked and found that I did not commit that test. Thankfully found it in the stash. Juggling too many PRs at the same time. Few scenarios are failing in it, most passes though.

For example one that passes:

        const project = new ProjectModel({
            startDate : '2023-08-28',

            eventStore : {
                tree                           : true,
                wbsMode                        : 'auto',
                useOrderedTreeForWbs           : true,
                reapplySortersOnAdd            : true,
                applyChangesetFilterSortTarget : 'changes'
            },

            eventsData : [
                { id : 1, name : '3' },
                { id : 2, name : '2' },
                { id : 3, name : '1' }
            ]
        });

        await project.commitAsync();

        const eventStore = project.eventStore;

        eventStore.sort('wbsValue');
        // there's a unclear early exit in the `filterChangeset` method, which happens if store has no filtering
        // we want to make sure this case works with `filterChangeset` enabled, so we add a dummy filter
        eventStore.filterBy(() => true);

        t.isDeeply(eventStore.map(r => r.name), ['3', '2', '1']);

        await project.applyProjectChanges({
            tasks : {
                added : [{ id : 4, name : '12', orderedParentIndex : 0 }, { id : 5, name : '11', orderedParentIndex : 2 }],
                updated : [
                    { id : 1, orderedParentIndex : 1 },
                    { id : 2, orderedParentIndex : 3 },
                    { id : 3, orderedParentIndex : 4 }
                ]
            }
        });

        t.isDeeply(collectOrderedWbs(eventStore, r => r.name), ['12', '3', '11', '2', '1']);
        t.isDeeply(collectVisibleWbs(eventStore, r => r.name), ['12', '3', '11', '2', '1']);

[canonic-epicure]

I'll need to revisit this issue. Noticed also eventStore.filterBy(() => true); thing in the test, which was added to activate the filterChangeset method, which has an early exit otherwise. Perhaps that changes something.

[canonic-epicure]

This ticket was included in the 5.5.3 release by mistake. Please follow the #7489 instead