Bug 15690 - Unconstrained CardnumberLength preference conflicts with table column limit of 16
Summary: Unconstrained CardnumberLength preference conflicts with table column limit o...
Status: Pushed to Master
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-01-28 13:23 UTC by Owen Leonard
Modified: 2016-12-14 12:08 UTC (History)
9 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 15690: CardnumberLength should not be bigger than 16 (3.11 KB, patch)
2016-09-13 14:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 15690: Hardcoded 16 is uncool (4.33 KB, patch)
2016-09-14 19:36 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 15690: CardnumberLength should not be bigger than 16 (3.16 KB, patch)
2016-09-14 23:12 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 15690: Hardcoded 16 is uncool (4.38 KB, patch)
2016-09-14 23:12 UTC, M. Tompsett
Details | Diff | Splinter Review
Bug 15690: CardnumberLength should not be bigger than 16 (3.20 KB, patch)
2016-09-16 16:20 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15690: Hardcoded 16 is uncool (4.47 KB, patch)
2016-09-16 16:21 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 15690: CardnumberLength should not be bigger than 16 (3.25 KB, patch)
2016-11-12 03:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 15690: Hardcoded 16 is uncool (4.52 KB, patch)
2016-11-12 03:23 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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 M. Tompsett 2016-09-14 19:36:36 UTC Comment hidden (obsolete)
Comment 7 M. 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 M. 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