Bug 27251 - Rewrite the QOTD editor using the Koha REST API
Summary: Rewrite the QOTD editor using the Koha REST API
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: System Administration (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Katrin Fischer
URL:
Keywords: Manual
: 8797 24702 (view as bug list)
Depends on: 27247
Blocks:
  Show dependency treegraph
 
Reported: 2020-12-16 12:59 UTC by Jonathan Druart
Modified: 2021-03-16 17:24 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Medium patch
Text to go in the release notes:
This patch replaces the QOTD editor with the standard way used for creating, editing, and deleting QOTD entries (now uses edit and delete buttons, and a standard form to add and edit entries).
Version(s) released in:
21.05.00


Attachments
Bug 27251: Rewrite QOTD with the Koha REST API (59.26 KB, patch)
2020-12-16 13:01 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27251: Rewrite QOTD with the Koha REST API (62.71 KB, patch)
2020-12-17 13:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27251: Rewrite QOTD with the Koha REST API (53.48 KB, patch)
2020-12-21 13:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27251: Rewrite QOTD with the Koha REST API (54.12 KB, patch)
2021-01-05 06:38 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27251: Don't hide the table on delete if empty (1.49 KB, patch)
2021-01-06 09:48 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27251: Rewrite QOTD with the Koha REST API (54.28 KB, patch)
2021-01-10 19:49 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 27251: Don't hide the table on delete if empty (1.56 KB, patch)
2021-01-10 19:49 UTC, Andrew Fuerste-Henry
Details | Diff | Splinter Review
Bug 27251: Fix sort by displayed_on (1.05 KB, patch)
2021-01-18 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27251: Don't export the actions column (988 bytes, patch)
2021-01-18 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27251: Add missing html filter (1.01 KB, patch)
2021-01-18 10:22 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 27251: Rewrite QOTD with the Koha REST API (54.34 KB, patch)
2021-01-27 22:35 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27251: Don't hide the table on delete if empty (1.62 KB, patch)
2021-01-27 22:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27251: Fix sort by displayed_on (1.11 KB, patch)
2021-01-27 22:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27251: Don't export the actions column (1.02 KB, patch)
2021-01-27 22:36 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27251: Add missing html filter (1.06 KB, patch)
2021-01-27 22:36 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2020-12-16 12:59:07 UTC
The quote of the day editor has some bugs and is not like the other screens.
We should rewrite it to fit better the libraries and SMTP servers administration pages (for instance).
Comment 1 Jonathan Druart 2020-12-16 13:01:33 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 :)
Comment 2 Jonathan Druart 2020-12-16 13:02:56 UTC
*** Bug 8797 has been marked as a duplicate of this bug. ***
Comment 3 Jonathan Druart 2020-12-16 13:03:50 UTC
*** Bug 24702 has been marked as a duplicate of this bug. ***
Comment 4 Caroline Cyr La Rose 2020-12-16 13:54:41 UTC
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.
Comment 5 Caroline Cyr La Rose 2020-12-16 13:57:55 UTC
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
Comment 6 Jonathan Druart 2020-12-17 13:24:42 UTC
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 :)
Comment 7 Jonathan Druart 2020-12-17 14:24:29 UTC
Sorry Caroline, I forgot to attach a file to the patch. That should work now.
Comment 8 Caroline Cyr La Rose 2020-12-18 16:07:26 UTC
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.
Comment 9 Caroline Cyr La Rose 2020-12-18 16:15:57 UTC
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
Comment 10 Jonathan Druart 2020-12-21 12:47:01 UTC
I think we should re-add the CSV import feature.
Comment 11 Jonathan Druart 2020-12-21 13:58:20 UTC
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
Comment 12 Jonathan Druart 2020-12-21 13:59:36 UTC
This last patch fixes the capitalization and re-add the CSV import feature.

Thanks Caroline!
Comment 13 Owen Leonard 2020-12-21 18:33:45 UTC
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.
Comment 14 Jonathan Druart 2021-01-05 06:38:15 UTC
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
Comment 15 Andrew Fuerste-Henry 2021-01-05 18:05:35 UTC
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.
Comment 16 Jonathan Druart 2021-01-06 09:48:51 UTC
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.
Comment 17 Jonathan Druart 2021-01-06 09:51:10 UTC
(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.
Comment 18 Tomás Cohen Arazi 2021-01-06 11:00:31 UTC
(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.
Comment 19 Andrew Fuerste-Henry 2021-01-10 19:49:46 UTC
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>
Comment 20 Andrew Fuerste-Henry 2021-01-10 19:49:49 UTC
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>
Comment 21 Katrin Fischer 2021-01-17 17:43:24 UTC
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.
Comment 22 Katrin Fischer 2021-01-17 17:47:24 UTC
3) missing filter in QA tools (almost forgot, had done the follow-up already :))
Comment 23 Jonathan Druart 2021-01-18 10:22:03 UTC
Created attachment 115234 [details] [review]
Bug 27251: Fix sort by displayed_on
Comment 24 Jonathan Druart 2021-01-18 10:22:07 UTC
Created attachment 115235 [details] [review]
Bug 27251: Don't export the actions column
Comment 25 Jonathan Druart 2021-01-18 10:22:12 UTC
Created attachment 115236 [details] [review]
Bug 27251: Add missing html filter
Comment 26 Jonathan Druart 2021-01-18 10:22:30 UTC
All fixed, Katrin. Thanks!
Comment 27 Jonathan Druart 2021-01-18 10:29:46 UTC
(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.
Comment 28 Katrin Fischer 2021-01-27 22:35:57 UTC
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>
Comment 29 Katrin Fischer 2021-01-27 22:36:01 UTC
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>
Comment 30 Katrin Fischer 2021-01-27 22:36:05 UTC
Created attachment 115917 [details] [review]
Bug 27251: Fix sort by displayed_on

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 31 Katrin Fischer 2021-01-27 22:36:09 UTC
Created attachment 115918 [details] [review]
Bug 27251: Don't export the actions column

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 32 Katrin Fischer 2021-01-27 22:36:13 UTC
Created attachment 115919 [details] [review]
Bug 27251: Add missing html filter

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 33 Jonathan Druart 2021-01-29 07:59:57 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 34 Fridolin Somers 2021-02-05 15:35:10 UTC
Enhancement not pushed to 20.11.x
Comment 35 Tomás Cohen Arazi 2021-03-16 17:24:00 UTC
\o/