Bug 23077 - Can't import patrons without cardnumber
Summary: Can't import patrons without cardnumber
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Nick Clemens
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-06-07 12:16 UTC by Nick Clemens
Modified: 2019-07-31 18:32 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.11.00,19.05.03


Attachments
Bug 23077 - Unit tests (2.43 KB, patch)
2019-06-07 12:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23077 - Don't fill cardnumber with empty string (1.18 KB, patch)
2019-06-07 12:22 UTC, Nick Clemens
Details | Diff | Splinter Review
Sample import file (848 bytes, text/csv)
2019-06-07 12:23 UTC, Nick Clemens
Details
Sample file (102 bytes, text/csv)
2019-06-07 12:23 UTC, Nick Clemens
Details
[SIGNED-OFF] Bug 23077 - Unit tests (2.49 KB, patch)
2019-06-11 07:02 UTC, Claire Gravely
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 23077 - Don't fill cardnumber with empty string (1.25 KB, patch)
2019-06-11 07:02 UTC, Claire Gravely
Details | Diff | Splinter Review
Bug 23077: Unit tests (2.49 KB, patch)
2019-07-02 10:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23077: Don't fill cardnumber with empty string (1.24 KB, patch)
2019-07-02 10:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23077: Fix import of cardnumber 0 (3.28 KB, patch)
2019-07-02 10:59 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 23077: Unit tests (2.58 KB, patch)
2019-07-05 06:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 23077: Don't fill cardnumber with empty string (1.33 KB, patch)
2019-07-05 06:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 23077: Fix import of cardnumber 0 (3.37 KB, patch)
2019-07-05 06:51 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2019-06-07 12:16:39 UTC
Note the plural, you can import 1 patron without a cardnumber, it defaults to an empty string

The next patron cannot be imported because empty string is already in use

Will attach sample file
Comment 1 Nick Clemens 2019-06-07 12:22:26 UTC
Created attachment 90415 [details] [review]
Bug 23077 - Unit tests

prove -v t/db_dependent/Koha/Patrons/Import.t
Comment 2 Nick Clemens 2019-06-07 12:22:29 UTC
Created attachment 90416 [details] [review]
Bug 23077 - Don't fill cardnumber with empty string

To test:
1 - Apply unit test patch only
2 - prove the tests, they fail
3 - Attempt to import the attached sample file
4 - First patron may be imported, second will give error (or both will error)
5 - If only one error, import agian, now both have error
6 - Apply this patch
7 - Prove tests, they pass!
8 - Import the sample file
9 - It succeeds!
Comment 3 Nick Clemens 2019-06-07 12:23:21 UTC Comment hidden (obsolete)
Comment 4 Nick Clemens 2019-06-07 12:23:57 UTC
Created attachment 90418 [details]
Sample file
Comment 5 Claire Gravely 2019-06-11 07:02:25 UTC
Created attachment 90474 [details] [review]
[SIGNED-OFF] Bug 23077 - Unit tests

prove -v t/db_dependent/Koha/Patrons/Import.t

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 6 Claire Gravely 2019-06-11 07:02:28 UTC
Created attachment 90475 [details] [review]
[SIGNED-OFF] Bug 23077 - Don't fill cardnumber with empty string

To test:
1 - Apply unit test patch only
2 - prove the tests, they fail
3 - Attempt to import the attached sample file
4 - First patron may be imported, second will give error (or both will error)
5 - If only one error, import agian, now both have error
6 - Apply this patch
7 - Prove tests, they pass!
8 - Import the sample file
9 - It succeeds!

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 7 Julian Maurice 2019-06-19 13:37:51 UTC
QA tool is not happy:

 OK     Koha/Patrons/Import.pm
 FAIL   t/db_dependent/Koha/Patrons/Import.t
   FAIL   forbidden patterns
                forbidden pattern: Data::Dumper::Dumper (line 400)


Processing additional checks

        * Commit title does not start with 'Bug XXXXX: ' - 033eec3a20
        * Commit title does not start with 'Bug XXXXX: ' - 79dd7b20d2

And the patch introduces a minor behaviour change: on master I can import a patron with cardnumber '0', with the patch I can't
Comment 8 Nick Clemens 2019-07-02 10:59:50 UTC
Created attachment 91181 [details] [review]
Bug 23077: Unit tests

prove -v t/db_dependent/Koha/Patrons/Import.t

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 9 Nick Clemens 2019-07-02 10:59:53 UTC
Created attachment 91182 [details] [review]
Bug 23077: Don't fill cardnumber with empty string

To test:
1 - Apply unit test patch only
2 - prove the tests, they fail
3 - Attempt to import the attached sample file
4 - First patron may be imported, second will give error (or both will error)
5 - If only one error, import agian, now both have error
6 - Apply this patch
7 - Prove tests, they pass!
8 - Import the sample file
9 - It succeeds!

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>
Comment 10 Nick Clemens 2019-07-02 10:59:55 UTC
Created attachment 91183 [details] [review]
Bug 23077: Fix import of cardnumber 0
Comment 11 Marcel de Rooy 2019-07-05 06:51:29 UTC
Created attachment 91315 [details] [review]
Bug 23077: Unit tests

prove -v t/db_dependent/Koha/Patrons/Import.t

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 12 Marcel de Rooy 2019-07-05 06:51:33 UTC
Created attachment 91316 [details] [review]
Bug 23077: Don't fill cardnumber with empty string

To test:
1 - Apply unit test patch only
2 - prove the tests, they fail
3 - Attempt to import the attached sample file
4 - First patron may be imported, second will give error (or both will error)
5 - If only one error, import agian, now both have error
6 - Apply this patch
7 - Prove tests, they pass!
8 - Import the sample file
9 - It succeeds!

Signed-off-by: Claire Gravely <claire.gravely@bsz-bw.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 13 Marcel de Rooy 2019-07-05 06:51:38 UTC
Created attachment 91317 [details] [review]
Bug 23077: Fix import of cardnumber 0

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 14 Martin Renvoize 2019-07-05 07:45:35 UTC
Nice work!

Pushed to master for 19.11.00
Comment 15 Fridolin SOMERS 2019-07-25 13:49:35 UTC
Pushed to 19.05.x for 19.05.03
Comment 16 Lucas Gass 2019-07-31 18:31:46 UTC
backported to 18.11.x for 18.11.09