Bug 27430 - Use minimum length for patron category on password hint
Summary: Use minimum length for patron category on password hint
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Templates (show other bugs)
Version: master
Hardware: All All
: P5 - low minor (vote)
Assignee: Emmi Takkinen
QA Contact: Testopia
URL:
Keywords:
Depends on: 23816
Blocks:
  Show dependency treegraph
 
Reported: 2021-01-14 12:21 UTC by Emmi Takkinen
Modified: 2021-03-03 15:16 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
This corrects the hint on the patron add/edit form to take into account that the minimum password length can now also be set on patron category level.
Version(s) released in:
21.05.00,20.11.03


Attachments
Bug 27430 - Use password minimum length for patron category on password hint (1.80 KB, patch)
2021-01-14 12:41 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 27430: Use password minimum length for patron category on password hint (1.85 KB, patch)
2021-01-14 13:20 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 27430: Use password minimum length for patron category on password hint (1.91 KB, patch)
2021-01-17 17:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27430: Change password minimum value hint when patron category is changed (2.37 KB, patch)
2021-01-18 09:22 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 27430: Use password minimum length for patron category on password hint (1.97 KB, patch)
2021-01-19 12:31 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 27430: Change password minimum value hint when patron category is changed (2.44 KB, patch)
2021-01-19 12:31 UTC, PTFS Europe Sandboxes
Details | Diff | Splinter Review
Bug 27430: Change password minimum value hint when patron category is changed (2.50 KB, patch)
2021-01-31 13:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27430: Use password minimum length for patron category on password hint (2.03 KB, patch)
2021-01-31 13:45 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 27430: Change password minimum value hint when patron category is changed (2.50 KB, patch)
2021-01-31 13:45 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 Emmi Takkinen 2021-01-14 12:21:00 UTC
Hint for password minimum length should use minimum length set for patron category  not "minPasswordLength" value. 

To test:

1. Set "minPasswordLength" e.g. 6
2. Set "Patron" category minimum password length e.g. 4
3. Create new a patron.
4. Note that hint after password input field reads "Minimum password length: 6"
Comment 1 Emmi Takkinen 2021-01-14 12:41:18 UTC Comment hidden (obsolete)
Comment 2 Owen Leonard 2021-01-14 13:20:39 UTC
Created attachment 115157 [details] [review]
Bug 27430: Use password minimum length for patron category on password hint

Hint for password minimum length should use minimum length
set for patron category  not "minPasswordLength" value.

To test:
1. Set "minPasswordLength" e.g. 6
2. Set "Patron" category minimum password length e.g. 4
3. Create new a patron.
4. Note that hint after password input field reads "Minimum
   password length: 6"
5. Apply patch.
6. Hint should now read "Minimum password length: 4".

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 3 Owen Leonard 2021-01-14 13:21:45 UTC
I would suggest we make this change in member-password.tt too.
Comment 4 Katrin Fischer 2021-01-17 17:03:45 UTC
(In reply to Owen Leonard from comment #3)
> I would suggest we make this change in member-password.tt too.

I've tried to see where this is needed it it looked ok to me - can you explaina bit more?
Comment 5 Katrin Fischer 2021-01-17 17:04:47 UTC
Created attachment 115226 [details] [review]
Bug 27430: Use password minimum length for patron category on password hint

Hint for password minimum length should use minimum length
set for patron category  not "minPasswordLength" value.

To test:
1. Set "minPasswordLength" e.g. 6
2. Set "Patron" category minimum password length e.g. 4
3. Create new a patron.
4. Note that hint after password input field reads "Minimum
   password length: 6"
5. Apply patch.
6. Hint should now read "Minimum password length: 4".

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2021-01-17 17:07:07 UTC
While this improves the situation, there is still an issue: When the patron category is changed, the hint might be wrong. Do we need an ajax call here to adjust the hint to the patron category selected?
Comment 7 Emmi Takkinen 2021-01-18 07:19:16 UTC

(In reply to Owen Leonard from comment #3)
> I would suggest we make this change in member-password.tt too.

I don't see where in member-password.tt this would be necessary. Did you mean some other page?  

(In reply to Katrin Fischer from comment #6)
> While this improves the situation, there is still an issue: When the patron
> category is changed, the hint might be wrong. Do we need an ajax call here
> to adjust the hint to the patron category selected?

Good point. This might be confusing for users especially when error message is adjusted by the selected patron category.
Comment 8 Emmi Takkinen 2021-01-18 09:22:38 UTC
Created attachment 115233 [details] [review]
Bug 27430: Change password minimum value hint when patron category is changed

If patrons category is changed during add or edit password
minimum value doesn't change. This patch adds ajax call to
fix this.

To test:
1. Add password minimum length for patron
categories A and B.
2. Create a new patron with category A.
3. Change patrons category to B.
4. Note that password minimum value doesn't change
on hint text.
5. Apply patch.
6. Repeat patron creation, password minimum value
should now change correctly.
Comment 9 PTFS Europe Sandboxes 2021-01-19 12:31:49 UTC
Created attachment 115316 [details] [review]
Bug 27430: Use password minimum length for patron category on password hint

Hint for password minimum length should use minimum length
set for patron category  not "minPasswordLength" value.

To test:
1. Set "minPasswordLength" e.g. 6
2. Set "Patron" category minimum password length e.g. 4
3. Create new a patron.
4. Note that hint after password input field reads "Minimum
   password length: 6"
5. Apply patch.
6. Hint should now read "Minimum password length: 4".

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 10 PTFS Europe Sandboxes 2021-01-19 12:31:52 UTC
Created attachment 115317 [details] [review]
Bug 27430: Change password minimum value hint when patron category is changed

If patrons category is changed during add or edit password
minimum value doesn't change. This patch adds ajax call to
fix this.

To test:
1. Add password minimum length for patron
categories A and B.
2. Create a new patron with category A.
3. Change patrons category to B.
4. Note that password minimum value doesn't change
on hint text.
5. Apply patch.
6. Repeat patron creation, password minimum value
should now change correctly.

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>
Comment 11 Sally 2021-01-19 12:32:10 UTC
Works as described.
Comment 12 Owen Leonard 2021-01-19 18:48:42 UTC
(In reply to Emmi Takkinen from comment #7)
> 
> (In reply to Owen Leonard from comment #3)
> > I would suggest we make this change in member-password.tt too.
> 
> I don't see where in member-password.tt this would be necessary. Did you
> mean some other page?  

Wouldn't we want to show the required password length on the page for changing the patron's username and password?

/cgi-bin/koha/members/member-password.pl?member=XXX
Comment 13 Katrin Fischer 2021-01-19 19:15:10 UTC
(In reply to Owen Leonard from comment #12)
> (In reply to Emmi Takkinen from comment #7)
> > 
> > (In reply to Owen Leonard from comment #3)
> > > I would suggest we make this change in member-password.tt too.
> > 
> > I don't see where in member-password.tt this would be necessary. Did you
> > mean some other page?  
> 
> Wouldn't we want to show the required password length on the page for
> changing the patron's username and password?
> 
> /cgi-bin/koha/members/member-password.pl?member=XXX

Hm it did show for me in testing, but only after I had tried to save a too short one - mean to show the hint always?
Comment 14 Katrin Fischer 2021-01-31 13:42:36 UTC
Created attachment 116105 [details] [review]
Bug 27430: Change password minimum value hint when patron category is changed

If patrons category is changed during add or edit password
minimum value doesn't change. This patch adds ajax call to
fix this.

To test:
1. Add password minimum length for patron
categories A and B.
2. Create a new patron with category A.
3. Change patrons category to B.
4. Note that password minimum value doesn't change
on hint text.
5. Apply patch.
6. Repeat patron creation, password minimum value
should now change correctly.

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2021-01-31 13:44:49 UTC
Thx Emmi for fixing the "category change"! I believe this is a nice bug fix fixing the wrong information shown on the edit patron screen.

I verified that the hint you get on using the "change password" feature is already correct. Showing a hint there always and not only when entering a wrong length one might be nice, but I think we can treat it as a separate enhancement.
Comment 16 Katrin Fischer 2021-01-31 13:45:18 UTC
Created attachment 116106 [details] [review]
Bug 27430: Use password minimum length for patron category on password hint

Hint for password minimum length should use minimum length
set for patron category  not "minPasswordLength" value.

To test:
1. Set "minPasswordLength" e.g. 6
2. Set "Patron" category minimum password length e.g. 4
3. Create new a patron.
4. Note that hint after password input field reads "Minimum
   password length: 6"
5. Apply patch.
6. Hint should now read "Minimum password length: 4".

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 17 Katrin Fischer 2021-01-31 13:45:23 UTC
Created attachment 116107 [details] [review]
Bug 27430: Change password minimum value hint when patron category is changed

If patrons category is changed during add or edit password
minimum value doesn't change. This patch adds ajax call to
fix this.

To test:
1. Add password minimum length for patron
categories A and B.
2. Create a new patron with category A.
3. Change patrons category to B.
4. Note that password minimum value doesn't change
on hint text.
5. Apply patch.
6. Repeat patron creation, password minimum value
should now change correctly.

Signed-off-by: Sally <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 18 Jonathan Druart 2021-02-01 16:13:10 UTC
In my understanding we never see 
    <div class="hint">Minimum password length: [% patron.category.effective_min_password_length | html %]</div>

as it's replaced by the JS code.

Shouldn't we remove it then?
Comment 19 Jonathan Druart 2021-02-01 16:14:33 UTC
1573             [% IF categorycode %]
1574                 update_category_code( "[% categorycode | html %]" );
1575             [% ELSE %]
1576                 if ( $("#categorycode_entry").length > 0 ){
1577                     var category_code = $("#categorycode_entry").find("option:selected").val();
1578                     update_category_code( category_code );
1579                 }
1580             [% END %]


update_category_code COULD not be called on document.ready. But I am not sure we display the form without the category dropdown list.

I will push as it, don't want to mess with this form.
Comment 20 Jonathan Druart 2021-02-01 16:26:49 UTC
Pushed to master for 21.05, thanks to everybody involved!
Comment 21 Fridolin Somers 2021-02-05 15:49:27 UTC
Pushed to 20.11.x for 20.11.03
Comment 22 Andrew Fuerste-Henry 2021-02-09 21:01:02 UTC
Missing dependency, not backported to 20.05.x