Hello all, We are seeing issues in our import_patrons.pl which is causing issue for our academic customer base. In versions prior to 22.11, running import_patrons.pl would cause the dateexpiry to either: — get set based on the contents of the dateexpiry in the CSV file; or — get left as the value already in Koha This worked well for our academics, because they could opt when to update the dateexpiry rather than sending them over every night (e.g. if a student exited a course early, they could be expired in the LMS to prevent them from accessing the services). In 22.11, the update-expiration and expiration-from-today flags have been added to the script. I believe the logic behind these flags is to update the dateexpiry based on enrolment plus user category duration, or today plus user category duration. This probably works great for most libraries, but it isn't necessarily desirable for Colleges. The option still exists to set dateexpiry from the CSV, by not passing the aforementioned flags. However, the dateexpiry now gets set to the following, when using this: — get set based on the contents of the dateexpiry in the CSV file; or — get left as the value already in Koha As you can see, this is a clear regression. Could we please look to remediate this, so I can pass on some good news to our affected customers? Many thanks, Jake Deery PTFS Europe
> The option still exists to set dateexpiry from the CSV, by not passing the > aforementioned flags. However, the dateexpiry now gets set to the following, > when using this: > — get set based on the contents of the dateexpiry in the CSV file; or > — get left as the value already in Koha Apologies, I meant to say that the regressed behaviour is as follows: — get set based on the contents of the dateexpiry in the CSV file; or — gets nulled out, essentially setting a patron as never expired
This will undoubtedly be a side effect of bug 27920.
Created attachment 156149 [details] [review] Bug 34883: Stop patron expiry date being set to NULL during import A regression has been identified whereby an empty field in the dateexpiry field in a patron import file will cause the patron's expiry date to be set to NULL. This patch addresses this by checking for an empty field and using the existing expiry date if one is found. Test plan: 1) Setup a csv with column headers: surname firstname branchcode categorycode cardnumber dateenrolled dateexpiry 2) Add values: Acosta Edna CPL PT 23529001000463 02/01/2013 3) Leave the dateexpiry column blank 4) Check Edna and make a note of her patron expiry date 5) Run the import_patrons.pl script with the following flags: a) --file <filepath_for_your_csv_file> b) --matchpoint cardnumber c) --confirm d) --overwrite 6) Check Edna, note her expiry date is now set to NULL 7) Manually edit Edna's expiry date to be reset to what it was before you ran the script 8) Apply the patch and restart_all 9) Repeat step 5 10) Check Edna, this time her expiry date should be the same as the value you set it to in step 7 11) Sign off!
Created attachment 156196 [details] [review] Bug 34883: Stop patron expiry date being set to NULL during import A regression has been identified whereby an empty field in the dateexpiry field in a patron import file will cause the patron's expiry date to be set to NULL. This patch addresses this by checking for an empty field and using the existing expiry date if one is found. Test plan: 1) Setup a csv with column headers: surname firstname branchcode categorycode cardnumber dateenrolled dateexpiry 2) Add values: Acosta Edna CPL PT 23529001000463 02/01/2013 3) Leave the dateexpiry column blank 4) Check Edna and make a note of her patron expiry date 5) Run the import_patrons.pl script with the following flags: a) --file <filepath_for_your_csv_file> b) --matchpoint cardnumber c) --confirm d) --overwrite 6) Check Edna, note her expiry date is now set to NULL 7) Manually edit Edna's expiry date to be reset to what it was before you ran the script 8) Apply the patch and restart_all 9) Repeat step 5 10) Check Edna, this time her expiry date should be the same as the value you set it to in step 7 11) Sign off! Signed-off-by: David Nind <david@davidnind.com>
Created attachment 156197 [details] Sample patron import file - Bug 34883 Testing notes (using KTD): 1. I've added the CSV file I used for testing (rename and save into /kohadevbox/koha using whatever file name you want). 2. Command: misc/import_patrons.pl --file /kohadevbox/koha/whatever-you-called-me.csv --matchpoint cardnumber --confirm --overwrite -v
Looks good to me. But could you add a small test that will stop future regressions on this topic?
Created attachment 156368 [details] [review] Bug 34883: Add unit test prove -v t/db_dependent/Koha/Patrons/Import.t
Good call, have added a new test to try and prevent regressions
Created attachment 156369 [details] [review] Bug 34883: Stop patron expiry date being set to NULL during import A regression has been identified whereby an empty field in the dateexpiry field in a patron import file will cause the patron's expiry date to be set to NULL. This patch addresses this by checking for an empty field and using the existing expiry date if one is found. Test plan: 1) Setup a csv with column headers: surname firstname branchcode categorycode cardnumber dateenrolled dateexpiry 2) Add values: Acosta Edna CPL PT 23529001000463 02/01/2013 3) Leave the dateexpiry column blank 4) Check Edna and make a note of her patron expiry date 5) Run the import_patrons.pl script with the following flags: a) --file <filepath_for_your_csv_file> b) --matchpoint cardnumber c) --confirm d) --overwrite 6) Check Edna, note her expiry date is now set to NULL 7) Manually edit Edna's expiry date to be reset to what it was before you ran the script 8) Apply the patch and restart_all 9) Repeat step 5 10) Check Edna, this time her expiry date should be the same as the value you set it to in step 7 11) Sign off! Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 156370 [details] [review] Bug 34883: Add unit test prove -v t/db_dependent/Koha/Patrons/Import.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 23.11. Nice work everyone, thanks!
Pushed to 23.05.x for 23.05.05
Nice work everyone! Pushed to oldstable for 22.11.x