|
Description
Jonathan Druart
2020-12-16 12:59:07 UTC
Created attachment 114435 [details] [review] Bug 27251: Rewrite QOTD with the Koha REST API This patch replace the QOTD editor with our new way to CRUD the adminitration page (like libraries and STMP servers) Test plan: Play with the QOTD by adding, removing, updating quotes Try to find bugs :) *** Bug 8797 has been marked as a duplicate of this bug. *** *** Bug 24702 has been marked as a duplicate of this bug. *** Test plan : 1) In Administration > Global system preferences, enable QuoteOfTheDay in both OPAC and staff interface 2) Go to Tools > Edit quotes for QOTD feature Add new quote 3) Click New quote 4) Enter a Source and a Text 5) Click Submit --> There should be a message saying "Quote added successfully." and the added quote should appear in the table. Hi Jonathan, I started a more detailed test plan, however, I'm stuck. I did the first 5 steps of my test plan, but my quote doesn't appear in the table. The message "Quote added successfully" is there, the table goes into "Processing", but the quote doesn't appear. I also went back to the tools page and came back to the QOTD page and the quote still wasn't there. Other things I noticed : - The button "New Quote" should be capitalized "New quote" - In the page to add a quote, "text:" should be capitalized "Text:" Caroline Created attachment 114477 [details] [review] Bug 27251: Rewrite QOTD with the Koha REST API This patch replace the QOTD editor with our new way to CRUD the adminitration page (like libraries and STMP servers) Test plan: Play with the QOTD by adding, removing, updating quotes Try to find bugs :) Sorry Caroline, I forgot to attach a file to the patch. That should work now. Test plan : 1) In Administration > Global system preferences, enable QuoteOfTheDay in both OPAC and staff interface 2) Go to Tools > Edit quotes for QOTD feature Add new quote 3) Click New quote 4) Enter a Source and a Text 5) Click Submit --> There should be a message saying "Quote added successfully." and the added quote should appear in the table. Edit quote 6) Click Edit next to a quote 7) Change the source or the text 8) Click Submit --> There should be a message saying "Quote updated successfully." and the change should be visible in the table Delete a quote 9) Click Delete next to a quote 10) Click Delete again in the confirmation box --> There should be a message saying "Quote #X was deleted successfully." and the quote should not be in the table anymore View quote in the staff interface 11) Go to Koha' staff interface's home page --> Under the news blocks, there should be a block called "Quote of the day" with a quote and source from the table View quote in the OPAC 12) Go to the OPAC's home page --> Under the news blocks, there should be a block called "Quote of the day" with the same quote. Everything works well with the patch! :) The only things left to correct are the capitalizations and one space: - New Quote should be New quote on the New quote button - text should be Text in the form to add a quote - There should be a space between the trash can and the word Delete on the Delete button - In the staff interface, Quote of the Day should be Quote of the day in the title of the block (this was not introduced by this patch, the capitalization was wrong before) Should these go in another bug? I'm also mentioning this, but it doesn't bother me too much: we lost the ability to import quote from a csv and we lost the ability to select multiple quotes to delete. I'll leave it to QA to decide if those two functions are essential and should be re-added. Caroline I think we should re-add the CSV import feature. Created attachment 114556 [details] [review] Bug 27251: Rewrite QOTD with the Koha REST API This patch replace the QOTD editor with our new way to CRUD the adminitration page (like libraries and STMP servers) Test plan: Play with the QOTD by adding, removing, updating quotes Try to find bugs :) Bug 27251: Fix capitalization This last patch fixes the capitalization and re-add the CSV import feature. Thanks Caroline! Jonathan told me in IRC that one should not be able to "select" a line anymore during the import process because the option to delete has been removed. Created attachment 114832 [details] [review] Bug 27251: Rewrite QOTD with the Koha REST API This patch replace the QOTD editor with our new way to CRUD the adminitration page (like libraries and STMP servers) Test plan: Play with the QOTD by adding, removing, updating quotes Try to find bugs :) Bug 27251: Fix capitalization Small issue: it'd be nice to be able to delete more than one quote at a time. Maybe checkboxes to select multiple and delete them all?
Larger issue:
- perform a search that returns just one quote ("bear" works)
- delete that quote
- your search is now filtering to zero results
- click "clear filter," observe you're still shown zero results
That said, thanks for writing this patch! This interface is frustratingly buggy and nonstandard as-is.
Created attachment 114885 [details] [review] Bug 27251: Don't hide the table on delete if empty If we filtered the result, delete the only row displayed then the table was hidden. This is a nice behaviour, especially if there was no filter. However we must show it again if the filters are removed. I am suggesting to simply to not hide the table and avoid adding more JS code to deal with this very specific use case. (In reply to Andrew Fuerste-Henry from comment #15) > Small issue: it'd be nice to be able to delete more than one quote at a > time. Maybe checkboxes to select multiple and delete them all? > > Larger issue: > - perform a search that returns just one quote ("bear" works) > - delete that quote > - your search is now filtering to zero results > - click "clear filter," observe you're still shown zero results > > That said, thanks for writing this patch! This interface is frustratingly > buggy and nonstandard as-is. Thanks for testing Andrew, good catch! I've suggested a patch and asked Tomas what he thinks about it. I would prefer to not add more features and stay as close as possible from the "SMTP servers" table. (In reply to Jonathan Druart from comment #17) > (In reply to Andrew Fuerste-Henry from comment #15) > > Small issue: it'd be nice to be able to delete more than one quote at a > > time. Maybe checkboxes to select multiple and delete them all? > > > > Larger issue: > > - perform a search that returns just one quote ("bear" works) > > - delete that quote > > - your search is now filtering to zero results > > - click "clear filter," observe you're still shown zero results > > > > That said, thanks for writing this patch! This interface is frustratingly > > buggy and nonstandard as-is. > > Thanks for testing Andrew, good catch! > I've suggested a patch and asked Tomas what he thinks about it. > I would prefer to not add more features and stay as close as possible from > the "SMTP servers" table. I hadn't spotted that problematic use case on writing the SMTP servers table. I agree we cannot loose the filter options, to clear them. Created attachment 114992 [details] [review] Bug 27251: Rewrite QOTD with the Koha REST API This patch replace the QOTD editor with our new way to CRUD the adminitration page (like libraries and STMP servers) Test plan: Play with the QOTD by adding, removing, updating quotes Try to find bugs :) Bug 27251: Fix capitalization Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Created attachment 114993 [details] [review] Bug 27251: Don't hide the table on delete if empty If we filtered the result, delete the only row displayed then the table was hidden. This is a nice behaviour, especially if there was no filter. However we must show it again if the filters are removed. I am suggesting to simply to not hide the table and avoid adding more JS code to deal with this very specific use case. Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> I am sorry, but I found something strange (broken) and something small: 1) When sorting on the "last display" column by clicking on the table header, I end up with a never-ending "Processing". Sorting on the title or Id works nicely. 2) The Actions column should be removed from the export. I know we already do this in other places, but need to clean up some still. 3) missing filter in QA tools (almost forgot, had done the follow-up already :)) Created attachment 115234 [details] [review] Bug 27251: Fix sort by displayed_on Created attachment 115235 [details] [review] Bug 27251: Don't export the actions column Created attachment 115236 [details] [review] Bug 27251: Add missing html filter All fixed, Katrin. Thanks! (In reply to Jonathan Druart from comment #24) > Created attachment 115235 [details] [review] [review] > Bug 27251: Don't export the actions column See also bug 26602. Created attachment 115915 [details] [review] Bug 27251: Rewrite QOTD with the Koha REST API This patch replace the QOTD editor with our new way to CRUD the adminitration page (like libraries and STMP servers) Test plan: Play with the QOTD by adding, removing, updating quotes Try to find bugs :) Bug 27251: Fix capitalization Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 115916 [details] [review] Bug 27251: Don't hide the table on delete if empty If we filtered the result, delete the only row displayed then the table was hidden. This is a nice behaviour, especially if there was no filter. However we must show it again if the filters are removed. I am suggesting to simply to not hide the table and avoid adding more JS code to deal with this very specific use case. Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com> Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 115917 [details] [review] Bug 27251: Fix sort by displayed_on Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 115918 [details] [review] Bug 27251: Don't export the actions column Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Created attachment 115919 [details] [review] Bug 27251: Add missing html filter Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de> Pushed to master for 21.05, thanks to everybody involved! Enhancement not pushed to 20.11.x \o/ |