Bug 15690

Summary: Unconstrained CardnumberLength preference conflicts with table column limit of 16
Product: Koha Reporter: Owen Leonard <oleonard>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: normal    
Priority: P5 - low CC: hector.hecaxmmx, indradg, jonathan.druart, julian.maurice, kyle, m.de.rooy, mtj, tomascohen, veron
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10267
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=15691
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 18338    
Attachments: Bug 15690: CardnumberLength should not be bigger than 16
Bug 15690: Hardcoded 16 is uncool
Bug 15690: CardnumberLength should not be bigger than 16
Bug 15690: Hardcoded 16 is uncool
Bug 15690: CardnumberLength should not be bigger than 16
Bug 15690: Hardcoded 16 is uncool
Bug 15690: CardnumberLength should not be bigger than 16
Bug 15690: Hardcoded 16 is uncool

Description Owen Leonard 2016-01-28 13:23:35 UTC
We offer the ability to define any minimum or maximum card number length, but the table column is limited to 16. At the very least the preference should be constrained to 16, but I think a better solution would be to increase the limit on borrowers.cardnumber /and/ require that the CardnumberLength preference respect it.
Comment 1 Owen Leonard 2016-01-28 13:35:26 UTC
Working on Bug 15691 makes this problem very visible. Here are the minimum/maximum length hints for various values of CardnumberLength:

"20" - "Card number can be up to 16 characters."

"20,30" - "Card number must be between 20 and 16 characters."

"40," - "Card number must be between 40 and 16 characters."
Comment 2 Owen Leonard 2016-01-28 13:46:45 UTC
> "40," - "Card number must be between 40 and 16 characters."

With the above setting you get a validation error, "Please enter at least 40 characters." Impossible because the maxlength of the field is 16.
Comment 3 Héctor Eduardo Castro Avalos 2016-01-28 15:00:28 UTC
Hi Owen

Also I think IMHO this needs to update the CardnumberLength sypref to explain the maximum lenght allowed.

Regards
Comment 4 Jonathan Druart 2016-09-13 14:46:11 UTC Comment hidden (obsolete)
Comment 5 Marc Véron 2016-09-14 14:15:07 UTC
There is a discussion on devel list about card number length proposing to extend cardnumber field in borrowers table from  VARCHAR(16) to VARCHAR(255)

See:
http://lists.koha-community.org/pipermail/koha-devel/2016-September/043049.html
Comment 6 Mark Tompsett 2016-09-14 19:36:36 UTC Comment hidden (obsolete)
Comment 7 Mark Tompsett 2016-09-14 23:12:32 UTC
Created attachment 55575 [details] [review]
Bug 15690: CardnumberLength should not be bigger than 16

borrowers.cardnumber is a varchar(16), so CardnumberLength should not
have a max > 16

Test plan:
Test different value in CardnumberLength ("20", "20,30", "40,")
Edit a patron a make sure the text display under the cardnumber input is
correct

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 8 Mark Tompsett 2016-09-14 23:12:42 UTC
Created attachment 55576 [details] [review]
Bug 15690: Hardcoded 16 is uncool

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Comment 9 Marc Véron 2016-09-16 16:20:14 UTC
Created attachment 55620 [details] [review]
Bug 15690: CardnumberLength should not be bigger than 16

borrowers.cardnumber is a varchar(16), so CardnumberLength should not
have a max > 16

Test plan:
Test different value in CardnumberLength ("20", "20,30", "40,")
Edit a patron a make sure the text display under the cardnumber input is
correct

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marc <veron@veron.ch>
Comment 10 Marc Véron 2016-09-16 16:21:39 UTC
Created attachment 55621 [details] [review]
Bug 15690: Hardcoded 16 is uncool

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Followed test plan in comment #7, works as expectd.
Signed-off-by: Marc <veron@veron.ch>
Comment 11 Marcel de Rooy 2016-11-11 08:39:07 UTC
On Jessie:

DBIx::Class::Storage::DBI::_warn_undetermined_driver(): This version of DBIC does not yet seem to supply a driver for your particular RDBMS and/or connection method ('Mock'). While we will attempt to continue anyway, the results are likely to be underwhelming. Please upgrade DBIC, and if this message does not go away, file a bugreport including the following info:
{
  DBD => "DBD::Mock",
  DBD_VER => "1.45",
  DBIC_DRIVER => "DBIx::Class::Storage::DBI",
  DBIC_DSN => "DBI:Mock:",
  DBIC_VER => "0.082810",
  DBI_VER => "1.631"
}
Comment 12 Jonathan Druart 2016-11-11 11:21:55 UTC
(In reply to Marcel de Rooy from comment #11)
> On Jessie:
> 
> DBIx::Class::Storage::DBI::_warn_undetermined_driver(): This version of DBIC
> does not yet seem to supply a driver for your particular RDBMS and/or
> connection method ('Mock'). While we will attempt to continue anyway, the
> results are likely to be underwhelming. Please upgrade DBIC, and if this
> message does not go away, file a bugreport including the following info:
> {
>   DBD => "DBD::Mock",
>   DBD_VER => "1.45",
>   DBIC_DRIVER => "DBIx::Class::Storage::DBI",
>   DBIC_DSN => "DBI:Mock:",
>   DBIC_VER => "0.082810",
>   DBI_VER => "1.631"
> }

Yes this is a known bug, see https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14778#c44
Comment 13 Marcel de Rooy 2016-11-11 15:31:14 UTC
(In reply to Jonathan Druart from comment #12)
> Yes this is a known bug, see
> https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14778#c44

Thx. Does the use of Test::DBIx::Class really outweigh its disadvantages (incl. a nasty install with a big bunch of dependencies)?
Comment 14 Tomás Cohen Arazi 2016-11-12 03:19:45 UTC
(In reply to Marcel de Rooy from comment #13)
> (In reply to Jonathan Druart from comment #12)
> > Yes this is a known bug, see
> > https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14778#c44
> 
> Thx. Does the use of Test::DBIx::Class really outweigh its disadvantages
> (incl. a nasty install with a big bunch of dependencies)?

Once we start using it, yeah.
Comment 15 Tomás Cohen Arazi 2016-11-12 03:23:18 UTC
Created attachment 57462 [details] [review]
Bug 15690: CardnumberLength should not be bigger than 16

borrowers.cardnumber is a varchar(16), so CardnumberLength should not
have a max > 16

Test plan:
Test different value in CardnumberLength ("20", "20,30", "40,")
Edit a patron a make sure the text display under the cardnumber input is
correct

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 16 Tomás Cohen Arazi 2016-11-12 03:23:24 UTC
Created attachment 57463 [details] [review]
Bug 15690: Hardcoded 16 is uncool

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

Followed test plan in comment #7, works as expectd.
Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 17 Kyle M Hall 2016-11-14 11:28:51 UTC
Pushed to master for 16.11, thanks Jonathan and Mark!
Comment 18 Julian Maurice 2016-11-30 12:29:08 UTC
Pushed to 3.22.x, will be in 3.22.13
Comment 19 Mason James 2016-12-14 12:08:41 UTC
Pushed to 16.05.x, for 16.05.06 release