Bug 18596

Summary: Quick add form duplicating password confirm
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: PatronsAssignee: Nick Clemens (kidclamp) <nick>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: gmcharlt, jonathan.druart, kyle.m.hall, kyle
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 3534    
Bug Blocks:    
Attachments: Bug 18596 - Quick add form duplicating password confirm
Bug 18596 - Quick add form duplicating password confirm
[SIGNED-OFF] Bug 18596 - Quick add form duplicating password confirm
[SIGNED-OFF] Bug 18596 - Quick add form duplicating password confirm
Bug 18596 (QA Followup)
Bug 18596 - Quick add form duplicating password confirm

Description Nick Clemens (kidclamp) 2017-05-12 13:07:33 UTC
If password is in BorrowerMandatoryField we end up with two confirm fields - the test for matching and copying the field is unneccessary since we copy all required fields anyways.

To recreate:
1 - Add password to BorrowerMandatoryField
2 - View quick add form
3 - Confirm password twice
Comment 1 Nick Clemens (kidclamp) 2017-05-12 13:10:23 UTC Comment hidden (obsolete)
Comment 2 Nick Clemens (kidclamp) 2017-05-12 13:19:23 UTC
Created attachment 63426 [details] [review]
Bug 18596 - Quick add form duplicating password confirm

To test:
1 - Add password to BorrowerMandatoryField
2 - View quick add form
3 - See confirm password twice
4 - Apply patch
5 - See confirm password once
6 - Add password to QuickAddFields
7 - Confirm one confirm field
8 - Remove password form BorrowerMandatory field
9 - Confirm there is one confirm field and password fields are not
required
Comment 3 Nick Clemens (kidclamp) 2017-05-12 14:45:25 UTC
Created attachment 63447 [details] [review]
[SIGNED-OFF] Bug 18596 - Quick add form duplicating password confirm

To test:
1 - Add password to BorrowerMandatoryField
2 - View quick add form
3 - See confirm password twice
4 - Apply patch
5 - See confirm password once
6 - Add password to QuickAddFields
7 - Confirm one confirm field
8 - Remove password form BorrowerMandatory field
9 - Confirm there is one confirm field and password fields are not
required

Signed-off-by: Peggy Thrasher <p.thrasher@dover.nh.gov>
Comment 4 Owen Leonard 2017-05-12 14:53:18 UTC
Created attachment 63449 [details] [review]
[SIGNED-OFF] Bug 18596 - Quick add form duplicating password confirm

To test:
1 - Add password to BorrowerMandatoryField
2 - View quick add form
3 - See confirm password twice
4 - Apply patch
5 - See confirm password once
6 - Add password to QuickAddFields
7 - Confirm one confirm field
8 - Remove password form BorrowerMandatory field
9 - Confirm there is one confirm field and password fields are not
required

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 5 Jonathan Druart 2017-05-18 18:49:12 UTC
My brain scratched just after the first line:
  var passwd_mandatory = [% IF mandatorypassword %]0[% ELSE %]1[% END %];

I'd suggest to not use a JS var and simply:

  [% UNLESS mandatorypassword %]
      if( input_label == 'password' ) $("#entryform label[for='password2']").parent().clone().appendTo("#quick_add_list");
  [% END %]
Comment 6 Nick Clemens (kidclamp) 2017-05-19 02:25:31 UTC
Created attachment 63555 [details] [review]
Bug 18596 (QA Followup)

Use TT in a less awkward way
Comment 7 Jonathan Druart 2017-05-22 13:42:41 UTC
Created attachment 63616 [details] [review]
Bug 18596 - Quick add form duplicating password confirm

To test:
1 - Add password to BorrowerMandatoryField
2 - View quick add form
3 - See confirm password twice
4 - Apply patch
5 - See confirm password once
6 - Add password to QuickAddFields
7 - Confirm one confirm field
8 - Remove password form BorrowerMandatory field
9 - Confirm there is one confirm field and password fields are not
required

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Kyle M Hall (khall) 2017-05-24 14:21:33 UTC
Pushed to master for 17.05, thanks Nick!
Comment 9 Katrin Fischer 2017-06-05 12:16:15 UTC
This patch has been pushed to 16.11.x and will be in 16.11.09.