Sort correctly in Bulk Order Managment

[cyrillefr]

What? Why?

• Closes [Bulk Order Management] Incorrect sorting by date: months sorted alphabetically instead of chronologically #9661

Inaccurate sorting @ /admin/orders/bulk_management
Even though so some numeric representation of a Date could be used at first thought, the advantage here is also to get a utc date first then to format it to an ISO8601. No locale in a numeric representation.

Avdi Grimm has a post on it: https://avdi.codes/iso8601-dates-in-ruby/

What should we test?

The same procedure as stated in #9661

• Go to /admin/orders/bulk_management
• Select date range covering three or more months
• Sort by date (click on 'completed at' in the table header)

I would personally use July, September and August

[jibees]

Great documentation, clean PR, thanks!

Wondering if this worth a spec. @filipefurtad0 what to you think?

[filipefurtad0]

Wondering if this worth a spec.

Agree @jibees , wondering as well - something like we already to for the orders page.

I'd suggest we'd make this check on the file spec/system/admin/bulk_order_management_spec.rb, within the block describe "sorting of line items" do.

Would you be available to add a test to this PR @cyrillefr?

[cyrillefr]

Hello @filipefurtad0.
Just added the spec.

[filipefurtad0]

LGTM, in what the spec concerns. Thanks @cyrillefr !

Seems like a minor code change - should we move it to test ready @jibees ?

[jibees]

Yes!

[filipefurtad0]

Hey @cyrillefr ,

I've staged the PR and repeated the automated test, on the server with "real" data.

Before the PR:

After this PR - with the data for this enterprise, I somehow cannot observer the correct ordering:

After clicking once on "Completed at", I get:

After clicking a second time:

This seems to work in some cases, but not always. Could there be another factor interfering in the ordering, on this page?

[cyrillefr]

This seems to work in some cases, but not always. Could there be another factor interfering in the ordering, on this page?

Hello @filipefurtad0.
I need to check. The thing that surprises me a bit also is it seems to me there is no lexicographical order either ...

[cyrillefr]

@filipefurtad0 , it should work now.
I had forgotten to put the serializer ....
Strangely, my combination of months names worked ...
So I have taken the same months as your example both in local and and test, and it is ok after(ko before this change)

[sigmundpetersen]

Thanks @cyrillefr .
We should get a short review of the latest changes before moving back to testing 👍

[mkllnk]

Great work!

[drummer83]

Hi @cyrillefr,
Thanks for following up on this. I have tested your updated PR and it looks good now.

Before your PR - clicking 'Completed at' ONCE
Sorted alphabetically, ascending:

Before your PR - clicking 'Completed at' TWICE
Sorted alphabetically, descending:

After your PR - clicking 'Completed at' ONCE
Sorted chronologically, ascending:

After your PR - clicking 'Completed at' TWICE
Sorted chronologically, descending:

Conclusion

This is great improvement, thanks a lot! It's ready to be merged! 🚀

Note

I find it strange that the columns are sorted per page only (and not all results of the query). So on other pages there may be older or newer line items, even if the table is sorted by date. On page /orders the behavior is different and more intuitive, I think. But we can see that each sorting process requires a new database query there - maybe there was a technical reason to implement the BOM page like this. I will do some digging and see if we want to improve this even more. Also the little indicator arrow for ascending and descending might be helpful sometimes.
All of this is clearly out of scope of this issue and your PR. Just noting my observation.
Thanks again! 💪