Bug 17492

Summary: Show warning if patron's age is out of category limits
Product: Koha Reporter: Radek Šiman (R-Bit Technology, s.r.o.) <rbit>
Component: CirculationAssignee: Radek Šiman (R-Bit Technology, s.r.o.) <rbit>
Status: CLOSED FIXED QA Contact: Nick Clemens <nick>
Severity: enhancement    
Priority: P5 - low CC: black23, ethan.amohia, fridolin.somers, gmcharlt, jonathan.druart, josef.moravec, katrin.fischer, kyle.m.hall, m.de.rooy, martin.renvoize, sally.healey, tomascohen, veron
Version: MainKeywords: Academy
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17521
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
This development adds a warning at checkout if a patron is found to have an age that is outside their categories age range and allows the staff user to immediately update the patrons' category from the warning dialogue.
Version(s) released in:
19.11.00
Bug Depends on:    
Bug Blocks: 26417    
Attachments: Added warning about age being of-of-limits
Removed a line setting 'flagged' template parameter by mistake
Fixed URL generation of "Change category" button
Fixed forbidden patterns
Bug 17492: Added warning about age being of-of-limits
Bug 17492: Removed a line setting 'flagged' template parameter by mistake
Bug 17492: Fixed URL generation of "Change category" button
Bug 17492: Fixed forbidden patterns
Bug 17492: Removed extra IFs
Bug 17492: Removed extra IFs
Bug 17492: Added warning about age being of-limits
Bug 17492: Removed a line setting 'flagged' template parameter by mistake
Bug 17492: Fixed URL generation of "Change category" button
Bug 17492: Fixed forbidden patterns
Bug 17492: Removed extra IFs
Bug 17492: Add a warning about age being of-limits and a button to update it
Bug 17942 - Update style of the web installer with Bootstrap 3
Bug 17492: Add a warning about age being of-limits and a button to update it
Bug 17492: Followup - use bootstrap 3 button classes
Bug 17492: Followup - adapt the code to current master
Bug 17492: (followup) Fixed broken layout of the patron detail page
Bug 17492: (followup) Logic moved to Koha namespace
Bug 17492: (followup) Added missing tests
Bug 17509: Removed invalid patterns
Bug 17492: Add a warning about age being of-limits and a button to update it
Bug 17492: Followup - use bootstrap 3 button classes
Bug 17492: Followup - adapt the code to current master
Bug 17492: (followup) Fixed broken layout of the patron detail page
Bug 17492: (followup) Logic moved to Koha namespace
Bug 17492: (followup) Added missing tests
Bug 17509: Removed invalid patterns
Bug 17492: Add a warning about age being of-limits and a button to update it
Bug 17492: (follow-up) Add missing filters
Bug 17492: Add a warning about age being of-limits and a button to update it
Bug 17492: (follow-up) Add missing filters
Bug 17492: Add a warning about age being of-limits and a button to update it
Bug 17492: (follow-up) Add missing filters
Bug 17492: (QA follow-up) Correct template variable

Description Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-24 14:52:14 UTC
If a patron's age is out of limits if an assigned category, there should be a warning similar to outstanding fees. Add a button allowing to change the category as well.
Comment 1 Katrin Fischer 2016-10-24 15:17:46 UTC
+1
Comment 2 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-24 15:52:59 UTC
Created attachment 56805 [details] [review]
Added warning about age being of-of-limits

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
Comment 3 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-24 16:01:39 UTC
Created attachment 56806 [details] [review]
Removed a line setting 'flagged' template parameter by mistake

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
Comment 4 Josef Moravec 2016-10-24 19:00:09 UTC
Comment on attachment 56805 [details] [review]
Added warning about age being of-of-limits

Review of attachment 56805 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/includes/category-out-of-age-limit.inc
@@ +1,1 @@
> +[% IF age_limitations %]

IFs like this are use in .tt files, so this is duplicate

::: koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
@@ +790,4 @@
>                  [% IF ( odues ) %]<li>[% IF ( nonreturns ) %]<span class="circ-hlt">Overdues: Patron has ITEMS OVERDUE.</span> <a href="#checkouts">See highlighted items below</a>[% END %]</li>
>              [% END %]
>  
> +            [% IF ( charges || age_limitations ) %]

Is this IF really necessary?
Comment 5 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-24 21:04:56 UTC
You're right, the IFs are duplicated, they aren't truly necessary, but it's implemented intentionally. The same approach occures in more files within intranet-tmpl/prog/en/includes/*.inc and I copied it exactly from blocked-fines.inc. It can be understood as en extra security check, because these *.inc files can be included from anywhere and one cannot presume these checks to be truly performed BEFORE the file inclusion. So it's just a last resort to prevent a failure in code.
Comment 6 Josef Moravec 2016-10-25 16:45:43 UTC
What is the intended use case? It's imposible to follow the test plan as Koha does check whether the age is between category limits when on saving patron data.
Comment 7 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-25 20:48:47 UTC
Maybe one picture is worth a thousand words...
http://prntscr.com/cywxq0
Intended use case is NOT on saving patron data, but rather during usual circulation process.
Comment 8 Marc Véron 2016-10-26 13:31:51 UTC
Very useful feature e.g. if kids grow out of their category.

I tested it by changing the age limits of the category "Kid".

The message displays as expected e.g. for kids who do no longer match the age limits. (Would be nice to get the message on the patron's Detail page as well.)

While testing I found following error: If I click on the button "Change category", I get a 'page not found". 

It seems that the link is not correct:

(...)/cgi-bin/koha/circ/memberentry.pl?op=modify&borrowernumber=71&step=3
should be:
(...)/cgi-bin/koha/members/memberentry.pl?op=modify&borrowernumber=71&step=3
Comment 9 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-26 14:34:47 UTC
Created attachment 56874 [details] [review]
Fixed URL generation of "Change category" button

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
Comment 10 Marc Véron 2016-10-27 11:17:43 UTC
Great, links for changing category work now, message appears on Checkout and Detals tab.

However QA tools complain for first patch:
FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/members/moremember.tt
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 188)
		forbidden pattern: tab char (line 184)
		forbidden pattern: tab char (line 185)
		forbidden pattern: tab char (line 187)
		forbidden pattern: tab char (line 189)

...and for 3rd patch:
 FAIL	koha-tmpl/intranet-tmpl/prog/en/includes/category-out-of-age-limit.inc
   FAIL	  forbidden patterns
		forbidden pattern: tab char (line 5)
Comment 11 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-27 14:19:51 UTC
As I am new to Koha development, could you please tell me what tools and how have you run to find this? Maybe the problem is TAB instead of 4 spaces?
Comment 12 Marc Véron 2016-10-27 14:43:00 UTC
(In reply to Radek Šiman (R-Bit Technology, s.r.o.) from comment #11)
> As I am new to Koha development, could you please tell me what tools and how
> have you run to find this? Maybe the problem is TAB instead of 4 spaces?

Hi Radek

Yes, it's the indentation, see:
https://wiki.koha-community.org/wiki/Coding_Guidelines#PERL6:_Indentation

For the QA tools see:
https://wiki.koha-community.org/wiki/QA_Test_Tools

Marc
Comment 13 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-27 21:19:25 UTC
Created attachment 56933 [details] [review]
Fixed forbidden patterns
Comment 14 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-27 21:22:22 UTC
Can anybody help me, what am I doing wrong? Thanks.

../qa-test-tools/koha-qa.pl -v 2 -c 1
 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   OK	  tt_valid
   FAIL	  valid_template
		Attempt to reload Koha/Template/Plugin/Koha.pm aborted.
Compilation failed in require at /usr/lib/x86_64-linux-gnu/perl5/5.20/Template/Plugins.pm line 206.
Comment 15 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-27 22:25:11 UTC
Problem concerning the QA script solved in a cooperation with Katrin. Asking to sign-off again.
Comment 16 Marc Véron 2016-10-28 06:05:56 UTC
Created attachment 56934 [details] [review]
Bug 17492: Added warning about age being of-of-limits

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category

https://bugs.koha-community.org/show_bug.cgi?id=17492

Tested all 4 patches together, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 17 Marc Véron 2016-10-28 06:06:50 UTC
Created attachment 56935 [details] [review]
Bug 17492: Removed a line setting 'flagged' template parameter by mistake

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category

https://bugs.koha-community.org/show_bug.cgi?id=17492

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 18 Marc Véron 2016-10-28 06:07:48 UTC
Created attachment 56936 [details] [review]
Bug 17492: Fixed URL generation of "Change category" button

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q

https://bugs.koha-community.org/show_bug.cgi?id=17492

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 19 Marc Véron 2016-10-28 06:08:58 UTC
Created attachment 56937 [details] [review]
Bug 17492: Fixed forbidden patterns

https://bugs.koha-community.org/show_bug.cgi?id=17492

Signed-off-by: Marc Véron <veron@veron.ch>
Comment 20 Jonathan Druart 2016-10-28 16:22:11 UTC
Hi Radek,
The 2 remarks made by Josef in comment 4 are valid and should be fixed.
Could you also prefix the commit message with the bug number and remove the url to this bug report please (see https://wiki.koha-community.org/wiki/Commit_messages)?
Comment 21 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-28 19:41:57 UTC
Hi Jonathan, should I really remove those IFs, although the same approach is used in other *.inc files I learnt it from and commented in https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17492#c5 ?
Comment 22 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-30 14:42:21 UTC
Created attachment 56976 [details] [review]
Bug 17492: Removed extra IFs

This patch removes unnecessary "if" conditions from html template files.
Removed 'flagged' template parameter in circ/circulation.pl.

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
Comment 23 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-31 09:53:10 UTC
Created attachment 56985 [details] [review]
Bug 17492: Removed extra IFs

This patch removes unnecessary "if" conditions from html template files.
Removed 'flagged' template parameter in circ/circulation.pl.

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
Comment 24 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-31 10:18:12 UTC
Created attachment 56986 [details] [review]
Bug 17492: Added warning about age being of-limits

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
Comment 25 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-31 10:18:35 UTC
Created attachment 56987 [details] [review]
Bug 17492: Removed a line setting 'flagged' template parameter by mistake

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
Comment 26 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-31 10:18:50 UTC
Created attachment 56988 [details] [review]
Bug 17492: Fixed URL generation of "Change category" button

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
Comment 27 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-31 10:19:06 UTC
Created attachment 56989 [details] [review]
Bug 17492: Fixed forbidden patterns
Comment 28 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-31 10:19:23 UTC
Created attachment 56990 [details] [review]
Bug 17492: Removed extra IFs

This patch removes unnecessary "if" conditions from html template files.
Removed 'flagged' template parameter in circ/circulation.pl.

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
Comment 29 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-31 11:14:28 UTC
Fixed all problems mentioned by Jonathan in comment 20. Asking anybody interested in this issue to sign-off the attachments again. Thanks.
Comment 30 Jonathan Druart 2016-10-31 11:38:21 UTC
No other signoff requested.
Comment 31 Jonathan Druart 2016-10-31 11:48:57 UTC
Created attachment 56995 [details] [review]
Bug 17492: Add a warning about age being of-limits and a button to update it

This patch adds a warning if a patron's age is out of limits if an assigned category.
It also adds a button allowing to change the category.

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q

Tested all 4 patches together, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Removed a line setting 'flagged' template parameter by mistake

Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Fixed URL generation of "Change category" button

Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Fixed forbidden patterns

Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Removed extra IFs

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 32 Jonathan Druart 2016-10-31 11:53:38 UTC
Note:
- I have squashed the 5 patches to easy the readability (too many changes on the same lines).
- After confirmation with Radek, I have updated the commit author's name and email.

QA note:
It would have been better to move the logic code into a module (for instance Koha::Patron->is_category_valid or something) to permit test coverage and avoid code duplication. As this is the first Radek patch, I think it can be accepted as it.
Comment 33 Jonathan Druart 2016-10-31 12:00:15 UTC
Hm sorry, just found something:
If you click on the "change category" button, you have the "step=3" parameter. But this step does not check if the patron category is valid.
For instance if you edit a Kid (5 to 17) and the patron has 18, the form will prompt you that the category is not valid.
I think we should have this check on this page.

Failing QA to get feedback from author and/or testers.
Comment 34 Josef Moravec 2016-10-31 13:18:23 UTC
It looks like bug in master, we could probably pass this path now and open another bug report...
Comment 35 Jonathan Druart 2016-10-31 13:26:19 UTC
(In reply to Josef Moravec from comment #34)
> It looks like bug in master, we could probably pass this path now and open
> another bug report...

Yes, indeed.
Comment 36 Radek Šiman (R-Bit Technology, s.r.o.) 2016-10-31 13:48:48 UTC
Just FYI: bug 17521 will solve the problem mentioned in comment 34.
Comment 37 Josef Moravec 2017-01-18 09:13:49 UTC
This patch need rebase and adjusment due to bug 17557.

And minor adjustment due to bug 16239, but I can attach followup after...
Comment 38 Marc Véron 2017-04-28 18:40:51 UTC Comment hidden (obsolete)
Comment 39 Marc Véron 2017-04-28 18:44:14 UTC
Comment on attachment 62860 [details] [review]
Bug 17942 - Update style of the web installer with Bootstrap 3

Sorry, mistaked with bug number...
Comment 40 Michal Denar 2017-09-07 20:59:21 UTC
Can anybody rebase it on master?
Comment 41 Josef Moravec 2017-09-11 11:09:39 UTC
Created attachment 67040 [details] [review]
Bug 17492: Add a warning about age being of-limits and a button to update it

This patch adds a warning if a patron's age is out of limits if an assigned category.
It also adds a button allowing to change the category.

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q

Tested all 4 patches together, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Removed a line setting 'flagged' template parameter by mistake

Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Fixed URL generation of "Change category" button

Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Fixed forbidden patterns

Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Removed extra IFs

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 42 Josef Moravec 2017-09-11 11:09:43 UTC
Created attachment 67041 [details] [review]
Bug 17492: Followup - use bootstrap 3 button classes
Comment 43 Josef Moravec 2017-09-11 11:09:46 UTC
Created attachment 67042 [details] [review]
Bug 17492: Followup - adapt the code to current master
Comment 44 Josef Moravec 2017-09-11 11:17:39 UTC
I just rebased and added two small followup - but we probably should do it as Jonathan suggests in comment 32 : $patron->is_category_valid
Comment 45 Katrin Fischer 2017-10-08 10:45:48 UTC
I am sorry, but with the patches applied, the layout on the patron detail page is broken badly. /cgi-bin/koha/members/moremember.pl?borrowernumber=51
Comment 46 Radek Šiman (R-Bit Technology, s.r.o.) 2017-10-09 19:26:33 UTC
Created attachment 67850 [details] [review]
Bug 17492: (followup) Fixed broken layout of the patron detail page

Test plan:
1) apply this patch
2) open eg. /cgi-bin/koha/members/moremember.pl?borrowernumber=3
3) tabbed menu (checkout, details, fines, etc.) should be displayed on
the left side of the page not at the bottom
Comment 47 Radek Šiman (R-Bit Technology, s.r.o.) 2017-10-09 19:26:37 UTC
Created attachment 67851 [details] [review]
Bug 17492: (followup) Logic moved to Koha namespace

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
9) Ensure that left-side panel is always on its expected place
Comment 48 Josef Moravec 2017-10-10 07:37:06 UTC
I feel we need test for method is_category_valid
Comment 49 Radek Šiman (R-Bit Technology, s.r.o.) 2017-10-10 12:12:11 UTC
Created attachment 67867 [details] [review]
Bug 17492: (followup) Added missing tests

This patch only adds a missing condition to test defined age in
is_category_valid method. The other task of this patch is to add missing
automated db-dependent tests.

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
9) Ensure that left-side panel is always on its expected place
10) Remove patron's date of birth and test that all categories are
allowed now
11) Run automated tests: prove t/db_dependent/Koha/Patron.t
Comment 50 Radek Šiman (R-Bit Technology, s.r.o.) 2017-10-10 12:12:16 UTC
Created attachment 67868 [details] [review]
Bug 17509: Removed invalid patterns

This patch only removes invalid gender-specific patterns from messages
in automated tests.
Comment 51 Katrin Fischer 2017-10-14 20:41:28 UTC
Created attachment 68139 [details] [review]
Bug 17492: Add a warning about age being of-limits and a button to update it

This patch adds a warning if a patron's age is out of limits if an assigned category.
It also adds a button allowing to change the category.

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q

Tested all 4 patches together, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Removed a line setting 'flagged' template parameter by mistake

Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Fixed URL generation of "Change category" button

Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Fixed forbidden patterns

Signed-off-by: Marc Véron <veron@veron.ch>

Bug 17492: Removed extra IFs

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 52 Katrin Fischer 2017-10-14 20:41:33 UTC
Created attachment 68140 [details] [review]
Bug 17492: Followup - use bootstrap 3 button classes

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 53 Katrin Fischer 2017-10-14 20:41:37 UTC
Created attachment 68141 [details] [review]
Bug 17492: Followup - adapt the code to current master

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 54 Katrin Fischer 2017-10-14 20:41:42 UTC
Created attachment 68142 [details] [review]
Bug 17492: (followup) Fixed broken layout of the patron detail page

Test plan:
1) apply this patch
2) open eg. /cgi-bin/koha/members/moremember.pl?borrowernumber=3
3) tabbed menu (checkout, details, fines, etc.) should be displayed on
the left side of the page not at the bottom

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 55 Katrin Fischer 2017-10-14 20:41:47 UTC
Created attachment 68143 [details] [review]
Bug 17492: (followup) Logic moved to Koha namespace

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
9) Ensure that left-side panel is always on its expected place

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 56 Katrin Fischer 2017-10-14 20:41:52 UTC
Created attachment 68144 [details] [review]
Bug 17492: (followup) Added missing tests

This patch only adds a missing condition to test defined age in
is_category_valid method. The other task of this patch is to add missing
automated db-dependent tests.

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
9) Ensure that left-side panel is always on its expected place
10) Remove patron's date of birth and test that all categories are
allowed now
11) Run automated tests: prove t/db_dependent/Koha/Patron.t

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 57 Katrin Fischer 2017-10-14 20:41:57 UTC
Created attachment 68145 [details] [review]
Bug 17509: Removed invalid patterns

This patch only removes invalid gender-specific patterns from messages
in automated tests.

https://bugs.koha-community.org/show_bug.cgi?id=17492

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 58 Marcel de Rooy 2018-01-05 07:06:17 UTC
Applying: Bug 17492: (followup) Added missing tests
fatal: sha1 information is lacking or useless (Koha/Patron.pm).
Repository lacks necessary blobs to fall back on 3-way merge.
Comment 59 Michal Denar 2019-05-22 19:24:58 UTC
Is possible to rebase this on master?
Comment 60 Josef Moravec 2019-06-26 18:34:47 UTC
Created attachment 91028 [details] [review]
Bug 17492: Add a warning about age being of-limits and a button to update it

This patch adds a warning if a patron's age is out of limits if an assigned category.
It also adds a button allowing to change the category.

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
9) Ensure that left-side panel is always on its expected place
10) Remove patron's date of birth and test that all categories are
allowed now
11) Run automated tests: prove t/db_dependent/Koha/Patron.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 61 Josef Moravec 2019-06-26 18:34:54 UTC
Created attachment 91029 [details] [review]
Bug 17492: (follow-up) Add missing filters
Comment 62 Josef Moravec 2019-06-26 18:35:24 UTC
(In reply to Michal Denar from comment #59)
> Is possible to rebase this on master?

Rebased
Comment 63 Michal Denar 2019-06-26 19:39:22 UTC
Hi Josef,
test plan works, but test faila:

annot detect source of 'Patron.t'! at /usr/share/perl/5.24/TAP/Parser/IteratorFactory.pm line 261.
	TAP::Parser::IteratorFactory::detect_source(TAP::Parser::IteratorFactory=HASH(0x55d0ebbf2e18), TAP::Parser::Source=HASH(0x55d0ebbf2ed8)) called at /usr/share/perl/5.24/TAP/Parser/IteratorFactory.pm line 211
	TAP::Parser::IteratorFactory::make_iterator(TAP::Parser::IteratorFactory=HASH(0x55d0ebbf2e18), TAP::Parser::Source=HASH(0x55d0ebbf2ed8)) called at /usr/share/perl/5.24/TAP/Parser.pm line 472
	TAP::Parser::_initialize(TAP::Parser=HASH(0x55d0ebbf2ad0), HASH(0x55d0eb9a6b08)) called at /usr/share/perl/5.24/TAP/Object.pm line 55
	TAP::Object::new("TAP::Parser", HASH(0x55d0eb9a6b08)) called at /usr/share/perl/5.24/TAP/Object.pm line 130
	TAP::Object::_construct(TAP::Harness=HASH(0x55d0eb8c93d8), "TAP::Parser", HASH(0x55d0eb9a6b08)) called at /usr/share/perl/5.24/TAP/Harness.pm line 852
	TAP::Harness::make_parser(TAP::Harness=HASH(0x55d0eb8c93d8), TAP::Parser::Scheduler::Job=HASH(0x55d0ebbc4e40)) called at /usr/share/perl/5.24/TAP/Harness.pm line 651
	TAP::Harness::_aggregate_single(TAP::Harness=HASH(0x55d0eb8c93d8), TAP::Parser::Aggregator=HASH(0x55d0eb42d820), TAP::Parser::Scheduler=HASH(0x55d0ebbc4de0)) called at /usr/share/perl/5.24/TAP/Harness.pm line 743
	TAP::Harness::aggregate_tests(TAP::Harness=HASH(0x55d0eb8c93d8), TAP::Parser::Aggregator=HASH(0x55d0eb42d820), "Patron.t") called at /usr/share/perl/5.24/TAP/Harness.pm line 558
	TAP::Harness::__ANON__() called at /usr/share/perl/5.24/TAP/Harness.pm line 571
	TAP::Harness::runtests(TAP::Harness=HASH(0x55d0eb8c93d8), "Patron.t") called at /usr/share/perl/5.24/App/Prove.pm line 546
	App::Prove::_runtests(App::Prove=HASH(0x55d0eb41d548), HASH(0x55d0eb88e210), "Patron.t") called at /usr/share/perl/5.24/App/Prove.pm line 504
	App::Prove::run(App::Prove=HASH(0x55d0eb41d548)) called at /usr/bin/prove line 13
Comment 64 Josef Moravec 2019-06-27 06:26:09 UTC
(In reply to Michal Denar from comment #63)
> Hi Josef,
> test plan works, but test faila:
> 
> annot detect source of 'Patron.t'! at


the file name in test plan s bad, you should run:

prove t/db_dependent/Koha/Patrons.t
Comment 65 Michal Denar 2019-06-27 07:47:14 UTC
Created attachment 91044 [details] [review]
Bug 17492: Add a warning about age being of-limits and a button to update it

This patch adds a warning if a patron's age is out of limits if an assigned category.
It also adds a button allowing to change the category.

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
9) Ensure that left-side panel is always on its expected place
10) Remove patron's date of birth and test that all categories are
allowed now
11) Run automated tests: prove t/db_dependent/Koha/Patron.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 66 Michal Denar 2019-06-27 07:47:19 UTC
Created attachment 91045 [details] [review]
Bug 17492: (follow-up) Add missing filters

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 67 Nick Clemens 2019-06-28 15:16:01 UTC
Created attachment 91098 [details] [review]
Bug 17492: Add a warning about age being of-limits and a button to update it

This patch adds a warning if a patron's age is out of limits if an assigned category.
It also adds a button allowing to change the category.

Test plan:
1) Apply patch
2) Create a patron and assign him a category having age limits set
3) Change patron's age to be older/younger than category's limits
4) At "Check out" and "Details" tabs you should see a warning with a
button allowing to change the category, eg.:
 - http://prntscr.com/cz7ch3
 - http://prntscr.com/cz7em4
 - http://prntscr.com/cz7dj1
5) Set a valid category according to patron's age
6) Warning should disappear
7) Perform similar test again, but instead of changing the age change
the limits of a category
8) During tests verify "Change category" button everytime opens "Modify
patron" page:
  http://prntscr.com/cz7g5q
9) Ensure that left-side panel is always on its expected place
10) Remove patron's date of birth and test that all categories are
allowed now
11) Run automated tests: prove t/db_dependent/Koha/Patron.t

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 68 Nick Clemens 2019-06-28 15:16:05 UTC
Created attachment 91099 [details] [review]
Bug 17492: (follow-up) Add missing filters

Signed-off-by: Michal Denar <black23@gmail.com>

Signed-off-by: Michal Denar <black23@gmail.com>
Comment 69 Nick Clemens 2019-06-28 15:16:09 UTC
Created attachment 91100 [details] [review]
Bug 17492: (QA follow-up) Correct template variable
Comment 70 Martin Renvoize 2019-07-01 15:19:27 UTC
Nice work!

Pushed to master for 19.11.00
Comment 71 Fridolin Somers 2019-07-25 12:08:57 UTC
Enhancement not pushed to 19.05.x