Regression from the KohaTable Vue component added by bug 33066.
Created attachment 152254 [details] [review] 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.
Created attachment 152270 [details] [review] 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>
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)
(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.
Created attachment 152471 [details] [review] 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>
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!
(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.
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?
(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.
Sorry for the earlier patch mess, but now I can QA :)
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
Created attachment 152605 [details] [review] 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>
Pushed to master for 23.11. Nice work everyone, thanks!
> 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.
Thanks for all the hard work! Pushed to 23.05.x for the next release
Nice work everyone! Pushed to 22.11.x for next release