Bug 34617

Summary: Patron expiration dates not updated during import when there is no dateexpiry column in the file
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: ToolsAssignee: Nick Clemens (kidclamp) <nick>
Status: Pushed to oldstable --- QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: david, fridolin.somers, jbalway, jonathan.druart, katrin.fischer, m.de.rooy
Version: Main   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=34621
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.04,22.11.10
Attachments: Bug 34617: Unit tests
Bug 34617: Preserve calculated expiration dates if set to update on import whether column included or not
Bug 34617: Unit tests
Bug 34617: Preserve calculated expiration dates if set to update on import whether column included or not
Bug 34617: Unit tests
Bug 34617: Preserve calculated expiration dates if set to update on import whether column included or not

Description Nick Clemens (kidclamp) 2023-08-25 14:50:54 UTC
Normally, imports only update included columns, however, if we are asking Koha
to calculate expiration dates we should update the value even if it is not passed
Comment 1 Nick Clemens (kidclamp) 2023-08-25 14:59:33 UTC
Created attachment 154816 [details] [review]
Bug 34617: Unit tests
Comment 2 Nick Clemens (kidclamp) 2023-08-25 14:59:35 UTC
Created attachment 154817 [details] [review]
Bug 34617: Preserve calculated expiration dates if set to update on import whether column included or not

To test:
1 - Setup a csv with column headers:
    surname	firstname	branchcode	categorycode	cardnumber	dateenrolled
2 - add values
    Acosta	Edna	CPL	PT	23529001000463	02/01/2013
3 - Set PT catgeory enrollement period to a specific date in the future
4 - Import the file and overwrite patrons and update dateexpiration
5 - Check Edna, note her date is not correct
6 - Apply patch
7 - Repeat
8 - Expiration date is now set as expected
Comment 3 Nick Clemens (kidclamp) 2023-08-25 15:00:05 UTC
Note, a workaround is simply to include a dateexpiry column in the file
Comment 4 David Nind 2023-08-28 06:01:25 UTC
Created attachment 154867 [details] [review]
Bug 34617: Unit tests

Signed-off-by: David Nind <david@davidnind.com>
Comment 5 David Nind 2023-08-28 06:01:27 UTC
Created attachment 154868 [details] [review]
Bug 34617: Preserve calculated expiration dates if set to update on import whether column included or not

To test:
1 - Setup a csv with column headers:
    surname	firstname	branchcode	categorycode	cardnumber	dateenrolled
2 - add values
    Acosta	Edna	CPL	PT	23529001000463	02/01/2013
3 - Set PT catgeory enrollement period to a specific date in the future
4 - Import the file and overwrite patrons and update dateexpiration
5 - Check Edna, note her date is not correct
6 - Apply patch
7 - Repeat
8 - Expiration date is now set as expected

Signed-off-by: David Nind <david@davidnind.com>
Comment 6 David Nind 2023-08-28 12:06:01 UTC
Note that tests failed if I left step 3 of the test plan in place, if I changed back to the default then they pass.

prove t/db_dependent/Koha/Patrons/Import.t 
t/db_dependent/Koha/Patrons/Import.t .. 2/177 
#   Failed test 'Expiration date is correct with update_dateexpiry = true'
#   at t/db_dependent/Koha/Patrons/Import.t line 218.
#          got: '2024-01-31'
#     expected: '2031-11-28'
t/db_dependent/Koha/Patrons/Import.t .. 175/177 Argument "" isn't numeric in numeric eq (==) at /usr/share/perl5/DBIx/Class/Row.pm line 1018, <$handle_1> line 2.
Argument "" isn't numeric in numeric eq (==) at /usr/share/perl5/DBIx/Class/Row.pm line 1018, <$handle_1> line 2.
# Looks like you failed 1 test of 177.
t/db_dependent/Koha/Patrons/Import.t .. Dubious, test returned 1 (wstat 256, 0x100)
Failed 1/177 subtests 

Test Summary Report
-------------------
t/db_dependent/Koha/Patrons/Import.t (Wstat: 256 Tests: 177 Failed: 1)
  Failed test:  60
  Non-zero exit status: 1
Files=1, Tests=177,  2 wallclock secs ( 0.03 usr  0.00 sys +  1.75 cusr  0.26 csys =  2.04 CPU)
Result: FAIL
Comment 7 Marcel de Rooy 2023-09-01 08:26:48 UTC
Created attachment 155066 [details] [review]
Bug 34617: Unit tests

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 8 Marcel de Rooy 2023-09-01 08:26:50 UTC
Created attachment 155067 [details] [review]
Bug 34617: Preserve calculated expiration dates if set to update on import whether column included or not

To test:
1 - Setup a csv with column headers:
    surname	firstname	branchcode	categorycode	cardnumber	dateenrolled
2 - add values
    Acosta	Edna	CPL	PT	23529001000463	02/01/2013
3 - Set PT catgeory enrollement period to a specific date in the future
4 - Import the file and overwrite patrons and update dateexpiration
5 - Check Edna, note her date is not correct
6 - Apply patch
7 - Repeat
8 - Expiration date is now set as expected

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 9 Tomás Cohen Arazi 2023-09-01 15:09:21 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 10 Fridolin Somers 2023-09-08 06:10:41 UTC
Pushed to 23.05.x for 23.05.04
Comment 11 Matt Blenkinsop 2023-09-15 09:33:44 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x