Bug 21389 - Javascript error on article requests page
Summary: Javascript error on article requests page
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Fridolin Somers
QA Contact: Testopia
URL:
Keywords:
Depends on: 21076
Blocks:
  Show dependency treegraph
 
Reported: 2018-09-21 09:15 UTC by Fridolin Somers
Modified: 2019-10-14 19:58 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 21389: Correct Javascript error on article requests page (1.97 KB, patch)
2018-09-21 09:24 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 21389: Correct Javascript error on article requests page (2.01 KB, patch)
2018-09-21 11:55 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21389: Correct Javascript error on article requests page (2.07 KB, patch)
2018-10-09 00:28 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2018-09-21 09:15:47 UTC
Bug 21076 fixed a Javascript error on article requests page when patron is found.

There is also an error when patron is not found, with message : 
  Patron not found
  No patron with this name, please, try another
Comment 1 Fridolin Somers 2018-09-21 09:24:43 UTC
Created attachment 79200 [details] [review]
Bug 21389: Correct Javascript error on article requests page

Bug 21076 fixed a Javascript error on article requests page when patron is found.

There is also an error when patron is not found, with message :
  Patron not found
  No patron with this name, please, try another

This patch fixes by replacing TT condition by a JS condition testing the existance of the patron input.

Test plan:
1 - Do not apply patch
2 - Enable article requests
3 - Search on staff client and click 'Request article'
4 - Search for a patron that does not exist
5 - You get a yellow message "Patron not found"
6 - View error in console
7 - Apply patch
8 - Reload
9 - Not error is gone
10 - Search for a patron that does exist
11 - Check there is no error in console
12 - Check that autocompletion works
Comment 2 Owen Leonard 2018-09-21 11:55:56 UTC
Created attachment 79226 [details] [review]
Bug 21389: Correct Javascript error on article requests page

Bug 21076 fixed a Javascript error on article requests page when patron is found.

There is also an error when patron is not found, with message :
  Patron not found
  No patron with this name, please, try another

This patch fixes by replacing TT condition by a JS condition testing the existance of the patron input.

Test plan:
1 - Do not apply patch
2 - Enable article requests
3 - Search on staff client and click 'Request article'
4 - Search for a patron that does not exist
5 - You get a yellow message "Patron not found"
6 - View error in console
7 - Apply patch
8 - Reload
9 - Not error is gone
10 - Search for a patron that does exist
11 - Check there is no error in console
12 - Check that autocompletion works

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 3 Jonathan Druart 2018-09-21 17:47:46 UTC
I am wondering what is best here:
1. Check in JS or
2. Keep the same TT conditionals (i.e. [% UNLESS no_patrons_found || patrons || patron %])
Comment 4 Fridolin Somers 2018-09-24 14:43:12 UTC
(In reply to Jonathan Druart from comment #3)
> I am wondering what is best here:
> 1. Check in JS or
> 2. Keep the same TT conditionals (i.e. [% UNLESS no_patrons_found || patrons
> || patron %])

For me it logical to prefer JS checks for a JS code.
Comment 5 Chris Cormack 2018-10-09 00:28:02 UTC
Created attachment 80247 [details] [review]
Bug 21389: Correct Javascript error on article requests page

Bug 21076 fixed a Javascript error on article requests page when patron is found.

There is also an error when patron is not found, with message :
  Patron not found
  No patron with this name, please, try another

This patch fixes by replacing TT condition by a JS condition testing the existance of the patron input.

Test plan:
1 - Do not apply patch
2 - Enable article requests
3 - Search on staff client and click 'Request article'
4 - Search for a patron that does not exist
5 - You get a yellow message "Patron not found"
6 - View error in console
7 - Apply patch
8 - Reload
9 - Not error is gone
10 - Search for a patron that does exist
11 - Check there is no error in console
12 - Check that autocompletion works

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Comment 6 Nick Clemens (kidclamp) 2018-10-09 16:24:26 UTC
Awesome work all!

Pushed to master for 18.11
Comment 7 Martin Renvoize (ashimema) 2018-10-15 12:47:32 UTC
Pushed to 18.05.x for 18.05.05
Comment 8 Fridolin Somers 2018-11-09 07:09:45 UTC
Pushed to 17.11.x for 17.11.12