EZP-24998: Order of the elements when doing a search is wrong (and random)

[dpobel]

JIRA: https://jira.ez.no/browse/EZP-24998

Description

This patch makes sure the Content Tree and the Subitem views receives the Location lists sorted as indicated by the parent Location sortOrder and sortField property.

Screencast:

5 sort clauses seem to not be fully implemented server side (see also https://github.com/ezsystems/PlatformUIBundle/pull/622/files#diff-3d4c2abab2b4c380f20d2e09ec804e3cR367):

• (Location)Priority https://jira.ez.no/browse/EZP-25907
• (Location)Path https://jira.ez.no/browse/EZP-25911
• (Location)Depth https://jira.ez.no/browse/EZP-25910 (ok this one is a bit stupid in that case, but can still be set the sort field for a Location)
• Content Type identifier https://jira.ez.no/browse/EZP-25909
• Content Type name https://jira.ez.no/browse/EZP-25908

So now the question is: how do we handle those before having the necessary server side code ?

Tasks

• Fix EZP-25846
• Expose the Location search as a public method in addition to the event so that it can be used from others plugins
• Allow to sort the search result based on a Location model
• Fix the Content Tree
• Fix the Subitem views
• Make sure the Subitem views and tree are updated when a Location sort order and/or sort field are updated

Tests

manual test + unit test

[dpobel]

(ping @andrerom @bdunogier for the question)

[andrerom]

So now the question is: how do we handle those before having the necessary server side code ?

got the impression you where handling this with @bdunogier this morning, but ok; how does it behave on unimplemented once? crashing or just sorting wrong?

[dpobel]

well, in the subitem view it displays an error notification saying the subitems could not be loaded (the subitem view is an inconsistent state, I'm currently fixing that). In the tree, you can not unfold the element and there's a red cross next to the Content name.

[andrerom]

well, in the subitem view it displays an error notification saying the subitems could not be loaded (the subitem view is an inconsistent state, I'm currently fixing that). In the tree, you can not unfold the element and there's a red cross next to the Content name.

Ok, it's clear we need to add implementation on that before we merge this then.

[dpobel]

for the unimplemented sort clauses, we can also force another sort clause to be used instead, then the sorting would be wrong but the interface would be usable.

[andrerom]

Whats @bdunogier's take? (as I said, I understood the two of you had talked about this already)

[dpobel]

I'm sorry but given the time frame regarding the freeze it's a product decision: either no sorting, either a buggy one.

[bdunogier]

I don't see any reasons why those can't be implemented. I'll give it a shot.

Can I test the REST changes by pulling your PR in, @dpobel ?

[bdunogier]

Implemented the three (priority, path, depth) SortClauses that exist in the Public API. Unfortunately, ContentTypeName and ContentTypeIdentifier don't exist in the Public API.

[andrerom]

Unfortunately, ContentTypeName and ContentTypeIdentifier don't exist in the Public API.

for the unimplemented sort clauses, we can also force another sort clause to be used instead, then the sorting would be wrong but the interface would be usable.

@dpobel For the two mentioned by @bdunogier that we on purpose are missing, and any undefined once, map them to not use anything at all or sane default as you prefer.

[andrerom]

These two will not be implemented.

Could for instance (since warning is not needed in this case, swap out console.log if something else if better):

+                case 'CLASS_IDENTIFIER':
+                case 'CLASS_NAME':
+                    console.log("Deprecated sort field used '" + this.get('sortField') + "', falling back to 'DateModified'.");
+                    return 'DateModified';

[mathieupoulenard]

I test some diff yesterday and sorting was correct. Great job !

[andrerom]

+1, but @StephaneDiot and/or @dew326 should have a look perhaps, as it's big.

[dew326]

it's ok for me, +1