Bug 23753 - Add missing humanMsg library to pages using background job JavaScript
Summary: Add missing humanMsg library to pages using background job JavaScript
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Owen Leonard
QA Contact: Testopia
URL:
Keywords:
Depends on: 19417
Blocks:
  Show dependency treegraph
 
Reported: 2019-10-04 16:12 UTC by Owen Leonard
Modified: 2020-04-14 21:15 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.05, 19.05.10


Attachments
Bug 23753: Background job JavaScript should not try to call humanMsg library (3.83 KB, patch)
2019-10-11 17:33 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 23753: Add missing humanMsg library to pages using background job JavaScript (6.12 KB, patch)
2020-02-28 16:50 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 23753: Add missing humanMsg library to pages using background job JavaScript (6.12 KB, patch)
2020-02-28 19:31 UTC, Maryse Simard
Details | Diff | Splinter Review
Bug 23753: Add missing humanMsg library to pages using background job JavaScript (6.22 KB, patch)
2020-02-29 20:42 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 Owen Leonard 2019-10-04 16:12:23 UTC
While trying to fix Bug 8645 I noticed that background-job-progressbar.js has error-handling code which tries to call the "humanMsg" function which is defined in the "HUMANIZED MESSAGES" plugin humanmsg.js. The plugin is currently loaded on the following pages:

admin\preferences.tt
cataloguing\editor.tt
tools\stage-marc-import.tt

Whereas background-job-progressbar.js is used on these pages:

offline_circ\process_koc.tt
tools\batch_record_modification.tt
tools\batchMod-edit.tt
tools\manage-marc-import.tt
tools\stage-marc-import.tt
tools\upload-images.tt

The background job error handling doesn't do anything other than return the AJAX error message, so I don't think it's worth it to add the humanmsg plugin everywhere. Better, I think, to change it to an alert.
Comment 1 Owen Leonard 2019-10-11 17:33:44 UTC Comment hidden (obsolete)
Comment 2 Cori Lynn Arnold 2020-01-20 16:12:09 UTC
I tested these and they worked as expected with the alert:

Tools -> Batch record modification
Tools -> Batch item modification
Tools -> Stage MARC records for import
Tools -> Staged MARC record management

I did *not* get an alert for the following interfaces:

Tools -> Upload local cover image
Circulation -> Upload offline circulation data (but I wasn't giving it a real file... if it needs a real-ish file, can you provide a test one that I can manipulate?)
Comment 3 Owen Leonard 2020-02-28 16:50:33 UTC
Created attachment 99786 [details] [review]
Bug 23753: Add missing humanMsg library to pages using background job JavaScript

The JavaScript used to process background uploads calls the humanMsg
library under some circumstances. There are several templates which use
background-job-progressbar.js which don't include the humanMsg assets.
This patch corrects those templates.

Note: upload-images.tt has been modified to remove the inclusion of
background-job-progressbar.js because it was unused.

To test, apply the patch and test the following processes which include
background job processing:

 - Batch item modification -- the background job is triggered when you
   submit your changes
 - Batch record modification -- the background job is triggered when you
   submit your changes
 - Stage MARC for import -- the background job is triggered when you
   click the "Sage for import" button after uploading a file
 - Staged MARC management -- the background job is triggered when you
   click "Import this batch into the catalog"

In each case the process should complete correctly. If you would like to
test the error message handling provided by the humanMsg library you can
deliberately introduce an error into the JavaScript. After applying the
patch you can modify js/background-job-progressbar.js to alter line 63
from:

   url: f.action,

...to:

   url: f.action + "foo",

Perform the same tests and confirm that you see an error message. Don't
forget to undo your changes to background-job-progressbar.js before
signing off.
Comment 4 Owen Leonard 2020-02-28 16:51:40 UTC
I changed the title of this bug to reflect the new aim of the patch: To include the humanMsg library on pages which need it (rather than removing it as a dependency).
Comment 5 Maryse Simard 2020-02-28 19:31:27 UTC
Created attachment 99790 [details] [review]
Bug 23753: Add missing humanMsg library to pages using background job JavaScript

The JavaScript used to process background uploads calls the humanMsg
library under some circumstances. There are several templates which use
background-job-progressbar.js which don't include the humanMsg assets.
This patch corrects those templates.

Note: upload-images.tt has been modified to remove the inclusion of
background-job-progressbar.js because it was unused.

To test, apply the patch and test the following processes which include
background job processing:

 - Batch item modification -- the background job is triggered when you
   submit your changes
 - Batch record modification -- the background job is triggered when you
   submit your changes
 - Stage MARC for import -- the background job is triggered when you
   click the "Sage for import" button after uploading a file
 - Staged MARC management -- the background job is triggered when you
   click "Import this batch into the catalog"

In each case the process should complete correctly. If you would like to
test the error message handling provided by the humanMsg library you can
deliberately introduce an error into the JavaScript. After applying the
patch you can modify js/background-job-progressbar.js to alter line 63
from:

   url: f.action,

...to:

   url: f.action + "foo",

Perform the same tests and confirm that you see an error message. Don't
forget to undo your changes to background-job-progressbar.js before
signing off.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>
Comment 6 Katrin Fischer 2020-02-29 20:42:52 UTC
Created attachment 99828 [details] [review]
Bug 23753: Add missing humanMsg library to pages using background job JavaScript

The JavaScript used to process background uploads calls the humanMsg
library under some circumstances. There are several templates which use
background-job-progressbar.js which don't include the humanMsg assets.
This patch corrects those templates.

Note: upload-images.tt has been modified to remove the inclusion of
background-job-progressbar.js because it was unused.

To test, apply the patch and test the following processes which include
background job processing:

 - Batch item modification -- the background job is triggered when you
   submit your changes
 - Batch record modification -- the background job is triggered when you
   submit your changes
 - Stage MARC for import -- the background job is triggered when you
   click the "Sage for import" button after uploading a file
 - Staged MARC management -- the background job is triggered when you
   click "Import this batch into the catalog"

In each case the process should complete correctly. If you would like to
test the error message handling provided by the humanMsg library you can
deliberately introduce an error into the JavaScript. After applying the
patch you can modify js/background-job-progressbar.js to alter line 63
from:

   url: f.action,

...to:

   url: f.action + "foo",

Perform the same tests and confirm that you see an error message. Don't
forget to undo your changes to background-job-progressbar.js before
signing off.

Signed-off-by: Maryse Simard <maryse.simard@inlibro.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Martin Renvoize 2020-03-02 09:47:29 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 8 Joy Nelson 2020-04-01 20:43:45 UTC
Backported to 19.11.x branch for 19.11.05
Comment 9 Lucas Gass 2020-04-08 21:59:37 UTC
backported to 19.05.x for 19.05.10
Comment 10 Hayley Mapley 2020-04-14 21:15:15 UTC
Dependency not in 18.11., not backporting to 18.11.x series