Bug 30903

Summary: CSV import of quotes broken
Product: Koha Reporter: Caroline Cyr La Rose <caroline.cyr-la-rose>
Component: ToolsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Kyle M Hall <kyle>
Severity: normal    
Priority: P5 - low CC: david, dcook, fridolin.somers, jonathan.druart, kyle, lucas, m.de.rooy, martin.renvoize, victor
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This fixes the import of quotes from a CSV file for the Quote of the Day feature.
Version(s) released in:
22.11.00, 22.05.04, 21.11.11
Bug Depends on:    
Bug Blocks: 31012, 31063, 31014    
Attachments: CSV file with quotes to test the import feature
Bug 30903: Fix POST /quote
Bug 30903: Fix UI glitch on the quotes upload view
Browse button off center from the box
Editing box is very small
Bug 30903: Fix UI glitch on the quotes upload view
Bug 30903: Fix POST /quote
Bug 30903: (follow-up) Fix error message class
Bug 30903: Fix POST /quote
Bug 30903: Fix UI glitch on the quotes upload view
Bug 30903: (follow-up) Fix error message class
Bug 30903: Fix POST /quote
Bug 30903: Fix UI glitch on the quotes upload view
Bug 30903: (follow-up) Fix error message class
Bug 30903: Fix POST /quote
Bug 30903: Fix UI glitch on the quotes upload view
Bug 30903: (follow-up) Fix error message class

Description Caroline Cyr La Rose 2022-06-03 21:46:31 UTC
Created attachment 135696 [details]
CSV file with quotes to test the import feature

I think the CSV quote import feature is broken...

To test:

1- Download the attached csv or create a comma-separated csv with two columns, one for source and one for text (no headers)
2- Go to Tools > Edit quotes for QOTD feature
3- Click on Import quotes
--> Note that the layout of the page is a bit strange... there is a big space between the title of the page and the first content box, and there is no space above the browse button
4- Click browse and choose the CSV file
--> The quotes should be imported in an editable table, you can click on either the source or text of a quote to change it (on my browser the editable field is really small, I wonder if it's the page or my browser (Firefox) or my OS (Ubuntu))
5- Click Save quote
--> Nothing happens
Comment 1 Caroline Cyr La Rose 2022-06-03 21:51:58 UTC
You can compare with what it used to look like in the screen captures in the manual : https://koha-community.org/manual/21.11/en/html/tools.html#edit-quotes-for-qotd-feature

Upload page (step 3) : https://koha-community.org/manual/21.11/en/html/_images/importquotes.png

Editing quotes (step 4) : https://koha-community.org/manual/21.11/en/html/_images/editquote.png
Comment 2 Jonathan Druart 2022-06-07 13:59:06 UTC
Created attachment 135773 [details] [review]
Bug 30903: Fix POST /quote

quote_id should not be required
Comment 3 Jonathan Druart 2022-06-07 13:59:10 UTC
Created attachment 135774 [details] [review]
Bug 30903: Fix UI glitch on the quotes upload view
Comment 4 Caroline Cyr La Rose 2022-06-07 14:44:19 UTC
Created attachment 135781 [details]
Browse button off center from the box
Comment 5 Caroline Cyr La Rose 2022-06-07 14:50:10 UTC
Created attachment 135782 [details]
Editing box is very small
Comment 6 Caroline Cyr La Rose 2022-06-07 14:56:34 UTC
I tested and here are my comments:

1) The Browse/Choose file button is still off center for some reason, it's at the top of the box, rather than vertically centered. (see attached screenshot https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135781 ) I tried both in Firefox and Chrome and it's at the top in both browsers

2) The editing boxes are still really small, when compared to how they were previously, as illustrated in the manual 
(currently https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=135782
previously https://koha-community.org/manual/latest/en/html/_images/editquote.png ) Again, I tried both in Firefox and Chrome and it's the same

3) The quotes seem to have been imported, if I trust the confirmation message, but I stayed on the same page and the two options available to me are "Save quotes", which I already clicked (multiple times because it was slow) and "Cancel import", which, if clicked, asks me a confirmation if I really want to cancel and sends me back to the import page with the Browse/Choose file button. I think once the quotes are imported, we should be sent back to the main QOTD editor page with the list of quotes already in the system.

Caroline
Comment 7 Jonathan Druart 2022-06-07 15:21:24 UTC
I think you are raising too many problems on the same bug :)
We should first fix the import error (it's blocking the feature) then focus on the UI glitches (which are not major IMO)
Comment 8 Fridolin Somers 2022-06-20 06:13:17 UTC
+                    $("#messages .import_error").text($("Something went wrong, check your CSV file."));

Sure about this code ? Looks not translatable.
Is it a type with _("bla bla") ?
Comment 9 Jonathan Druart 2022-06-20 06:53:31 UTC
Created attachment 136336 [details] [review]
Bug 30903: Fix UI glitch on the quotes upload view
Comment 10 Jonathan Druart 2022-06-20 06:53:56 UTC
(In reply to Fridolin Somers from comment #8)
> +                    $("#messages .import_error").text($("Something went
> wrong, check your CSV file."));
> 
> Sure about this code ? Looks not translatable.
> Is it a type with _("bla bla") ?

Yes!
Comment 11 Fridolin Somers 2022-06-20 21:13:04 UTC
Created attachment 136383 [details] [review]
Bug 30903: Fix POST /quote

quote_id should not be required

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 12 Fridolin Somers 2022-06-20 21:24:01 UTC
Created attachment 136384 [details] [review]
Bug 30903: (follow-up) Fix error message class
Comment 13 Fridolin Somers 2022-06-20 21:26:22 UTC
I signed the 2 first patches.
Quotes import works well.

But then I generated the error case (by dropping quotes table).
It does not appear because its class is "import_errors" instead of "import_error", see my follow-up.
Comment 14 Fridolin Somers 2022-06-20 21:29:27 UTC
(In reply to Caroline Cyr La Rose from comment #6)
> 3) The quotes seem to have been imported, if I trust the confirmation
> message, but I stayed on the same page and the two options available to me
> are "Save quotes", which I already clicked (multiple times because it was
> slow) and "Cancel import", which, if clicked, asks me a confirmation if I
> really want to cancel and sends me back to the import page with the
> Browse/Choose file button. I think once the quotes are imported, we should
> be sent back to the main QOTD editor page with the list of quotes already in
> the system.
> 
> Caroline

Same for me, please report it in a new report.
Clicking on "Save quotes" will import them again, clicking on "Cancel import" does not revert the import.
I think think after clicking on "Save quotes" we should hide those buttons and propose a link to come back to quotes table.
Comment 15 Caroline Cyr La Rose 2022-06-22 13:27:08 UTC
I created Bug 31012 for the usability problem after uploading the quotes.

I created Bug 31014 for the UI problems reported in the description.
Comment 16 Owen Leonard 2022-06-22 17:01:40 UTC
I'm getting this error:

POST /api/v1/quotes: unhandled exception (DBIx::Class::Exception)<<DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Column 'timestamp' cannot be null
Comment 17 Marcel de Rooy 2022-06-22 18:42:57 UTC
Critical ?
Comment 18 Fridolin Somers 2022-06-23 08:53:40 UTC
Created attachment 136437 [details] [review]
Bug 30903: Fix POST /quote

quote_id should not be required

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 19 Fridolin Somers 2022-06-23 08:53:55 UTC
Created attachment 136438 [details] [review]
Bug 30903: Fix UI glitch on the quotes upload view

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 20 Fridolin Somers 2022-06-23 08:54:09 UTC
Created attachment 136439 [details] [review]
Bug 30903: (follow-up) Fix error message class
Comment 21 David Cook 2022-06-27 00:44:39 UTC
Downgrading the severity of this one since it's not critical to Koha operations
Comment 22 David Nind 2022-06-29 01:23:29 UTC
Created attachment 136702 [details] [review]
Bug 30903: Fix POST /quote

quote_id should not be required

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: David Nind <david@davidnind.com>
Comment 23 David Nind 2022-06-29 01:23:34 UTC
Created attachment 136703 [details] [review]
Bug 30903: Fix UI glitch on the quotes upload view

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>

Signed-off-by: David Nind <david@davidnind.com>
Comment 24 David Nind 2022-06-29 01:23:38 UTC
Created attachment 136704 [details] [review]
Bug 30903: (follow-up) Fix error message class

Signed-off-by: David Nind <david@davidnind.com>
Comment 25 Martin Renvoize 2022-07-05 15:03:51 UTC
Created attachment 137176 [details] [review]
Bug 30903: Fix POST /quote

quote_id should not be required

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 26 Martin Renvoize 2022-07-05 15:03:55 UTC
Created attachment 137177 [details] [review]
Bug 30903: Fix UI glitch on the quotes upload view

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 27 Martin Renvoize 2022-07-05 15:03:59 UTC
Created attachment 137178 [details] [review]
Bug 30903: (follow-up) Fix error message class

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 28 Martin Renvoize 2022-07-05 15:04:27 UTC
Fixes the issue at hand, QA scripts are happy.

Passing QA
Comment 29 Tomás Cohen Arazi 2022-07-06 19:43:00 UTC
Pushed to master for 22.11.

Nice work everyone, thanks!
Comment 30 Lucas Gass 2022-08-12 15:42:49 UTC
Backported to 22.05.x for 22.05.04
Comment 31 Victor Grousset/tuxayo 2022-08-19 20:40:13 UTC
note: was backported to 21.11.x for 21.11.11
Comment 32 Victor Grousset/tuxayo 2022-08-19 20:40:59 UTC
Not backported to oldoldstable (21.05.x). Feel free to ask if it's needed.

Nothing to document, marking resolved.