[Bug 33973] New: Sorting broken on ERM tables
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Bug ID: 33973 Summary: Sorting broken on ERM tables Change sponsored?: --- Product: Koha Version: unspecified Hardware: All OS: All Status: ASSIGNED Severity: normal Priority: P5 - low Component: ERM Assignee: jonathan.druart+koha@gmail.com Reporter: jonathan.druart+koha@gmail.com CC: jonathan.druart+koha@gmail.com, jonathan.field@ptfs-europe.com, martin.renvoize@ptfs-europe.com, matt.blenkinsop@ptfs-europe.com, pedro.amorim@ptfs-europe.com Depends on: 33066 Regression from the KohaTable Vue component added by bug 33066. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33066 [Bug 33066] We need a KohaTable Vue component -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 --- Comment #1 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- Created attachment 152254 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152254&action=edit Bug 33973: Restore sorting on Vue tables Regression from the KohaTable Vue component added by bug 33066. _dt_default_ajax expects options.columns to contain the columns. Test plan: Add some agreements, go the the agreement list view and confirm that you can now sort the agreements using the header columns. Note that the CSS is not correct yet. My understanding is that the DataTable Vue component keep the 'sorting' class on the th which display the grey up/down icon over the blue icon which indicate the current sort column. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off Patch complexity|--- |Small patch -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 --- Comment #2 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152270 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152270&action=edit Bug 30002: (QA follow-up) Add .perltidyrc to Makefile.PL mapping Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Sign-off not quite right as I only re-uploaded the second patch (I'll try to fix later, won't mind if someone else does) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152270|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Needs Signoff --- Comment #4 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Katrin Fischer from comment #3)
Sign-off not quite right as I only re-uploaded the second patch (I'll try to fix later, won't mind if someone else does)
There was only one patch. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152254|0 |1 is obsolete| | --- Comment #5 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Created attachment 152471 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152471&action=edit Bug 33973: Restore sorting on Vue tables Regression from the KohaTable Vue component added by bug 33066. _dt_default_ajax expects options.columns to contain the columns. Test plan: Add some agreements, go the the agreement list view and confirm that you can now sort the agreements using the header columns. Note that the CSS is not correct yet. My understanding is that the DataTable Vue component keep the 'sorting' class on the th which display the grey up/down icon over the blue icon which indicate the current sort column. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 --- Comment #6 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- This fixes the reported issue. However, Jonathan, I think we should consider moving cypress tests into db_dependent and test with live data coming from the API instead of mocking, otherwise creating a test for this (and other things) is pointless. In this example, "_order_by: -me.publication_title" was not being sent on the request headers. We can't write a test for this as we're mocking the response instead of getting what the API actually returns. I think we discussed moving in this direction in the past, but I can't recall what we decided upon or what were the reasons for not doing it! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 --- Comment #7 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Pedro Amorim from comment #6)
This fixes the reported issue.
However, Jonathan, I think we should consider moving cypress tests into db_dependent and test with live data coming from the API instead of mocking, otherwise creating a test for this (and other things) is pointless.
In this example, "_order_by: -me.publication_title" was not being sent on the request headers. We can't write a test for this as we're mocking the response instead of getting what the API actually returns. I think we discussed moving in this direction in the past, but I can't recall what we decided upon or what were the reasons for not doing it!
We need a way to access the config from cypress tests (for the DB connection). Tests must create the data they need not rely on existing data. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 --- Comment #8 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- I was thinking create the test data by POSTing to the REST API beforehand, and DELETE it after the tests have finished running? Is this reasonable? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 --- Comment #9 from Jonathan Druart <jonathan.druart+koha@gmail.com> --- (In reply to Pedro Amorim from comment #8)
I was thinking create the test data by POSTing to the REST API beforehand, and DELETE it after the tests have finished running? Is this reasonable?
Yes but it is not always easy to create all the depend data you need. Eg for a patron you need a library and a patron category. Is is possible but we certainly need a lib to help us with dependent objects. Having a direct access to the DB would bring us the usage of TestBuilder. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact| |katrin.fischer@bsz-bw.de --- Comment #10 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Sorry for the earlier patch mess, but now I can QA :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34107 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34107 [Bug 34107] Sorting agreements by Name actually sorts by ID -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 --- Comment #11 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- There is still something wrong when searching by Name. Add agreements in this sequence: t b z They get sequential IDs (1 2 3 or similar). They get sequential IDs, when you sort, they by Name, they won't sort by the description, but by their ID: zbt or tbz Filed separately as: Bug 34107 - Sorting agreements by Name actually sorts by ID -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #152471|0 |1 is obsolete| | --- Comment #12 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Created attachment 152605 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=152605&action=edit Bug 33973: Restore sorting on Vue tables Regression from the KohaTable Vue component added by bug 33066. _dt_default_ajax expects options.columns to contain the columns. Test plan: Add some agreements, go the the agreement list view and confirm that you can now sort the agreements using the header columns. Note that the CSS is not correct yet. My understanding is that the DataTable Vue component keep the 'sorting' class on the th which display the grey up/down icon over the blue icon which indicate the current sort column. Signed-off-by: Pedro Amorim <pedro.amorim@ptfs-europe.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |23.11.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 --- Comment #13 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Pushed to master for 23.11. Nice work everyone, thanks! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Jonathan Druart <jonathan.druart+koha@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |34201 --- Comment #14 from Jonathan Druart <jonathan.druart+koha@gmail.com> ---
Note that the CSS is not correct yet. My understanding is that the DataTable Vue component keep the 'sorting' class on the th which display the grey up/down icon over the blue icon which indicate the current sort column.
I've opened bug 34201. Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34201 [Bug 34201] Missing sorting indicator on the ERM tables -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |Pushed to stable Version(s)|23.11.00 |23.11.00,23.05.02 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 --- Comment #15 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for all the hard work! Pushed to 23.05.x for the next release -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 Pedro Amorim <pedro.amorim@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to stable |Pushed to oldstable Version(s)|23.11.00,23.05.02 |23.11.00,23.05.02,22.11.08 released in| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33973 --- Comment #16 from Pedro Amorim <pedro.amorim@ptfs-europe.com> --- Nice work everyone! Pushed to 22.11.x for next release -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org