Bug 15607 - Batch patron modification: Data loss of 'dateenrolled' and 'expirydate' fields
Summary: Batch patron modification: Data loss of 'dateenrolled' and 'expirydate' fields
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Tools (show other bugs)
Version: master
Hardware: All All
: P1 - high critical (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 15332
Blocks:
  Show dependency treegraph
 
Reported: 2016-01-19 19:59 UTC by Mason James
Modified: 2019-06-27 09:24 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
updating 'state' field (114.37 KB, image/png)
2016-01-19 20:03 UTC, Mason James
Details
'dateenrolled' and 'expirydate' fields have now been modified (92.59 KB, image/png)
2016-01-19 20:04 UTC, Mason James
Details
Bug 15607: batch patron mod - do not update dates if not given (1.25 KB, patch)
2016-01-20 08:51 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 15607: batch patron mod - do not update dates if not given (1.32 KB, patch)
2016-01-20 09:48 UTC, Karam Qubsi
Details | Diff | Splinter Review
[PASSED QA] Bug 15607: batch patron mod - do not update dates if not given (1.37 KB, patch)
2016-01-20 21:52 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Mason James 2016-01-19 19:59:42 UTC
The 'batch parton modification' tool seems to overwrite 'dateenrolled' and 'expirydate' fields with $todaysdate, if any of these fields are left unpopulated when updating

confirmed on master 8db6b0f5330f16d4e3f100987597d0ce72645d07

to test...

1/ Set 'dateenrolled' and/or 'expirydate' fields of a test user

2/ Use 'batch parton modification' tool to update any other field (eg: 'State')

3/ Confirm that user's 'dateenrolled' and/or 'expirydate' fields have been incorrectly updated to $todays date :(
Comment 1 Mason James 2016-01-19 20:03:12 UTC
Created attachment 46928 [details]
updating 'state' field
Comment 2 Mason James 2016-01-19 20:04:32 UTC
Created attachment 46930 [details]
'dateenrolled' and 'expirydate' fields have now been modified
Comment 3 Mason James 2016-01-19 20:12:24 UTC
(In reply to Mason James from comment #0)
> The 'batch parton modification' tool seems to overwrite 'dateenrolled' and
> 'expirydate' fields with $todaysdate, if any of these fields are left
> unpopulated when updating
> 
> confirmed on master 8db6b0f5330f16d4e3f100987597d0ce72645d07

..this bug will also exist in 3.22.2 and 3.20.8 releases
Comment 4 Jonathan Druart 2016-01-20 08:51:11 UTC Comment hidden (obsolete)
Comment 5 Karam Qubsi 2016-01-20 09:48:51 UTC Comment hidden (obsolete)
Comment 6 Jonathan Druart 2016-01-20 13:26:08 UTC
CCing Rmaint 3.22 and 3.20: this fix must be in the 3.22.2 and 3.20.8
Comment 7 Katrin Fischer 2016-01-20 21:52:39 UTC
Created attachment 47055 [details] [review]
[PASSED QA] Bug 15607: batch patron mod - do not update dates if not given

Bug 15332 fixed a bug but introduced a bigger one. Even if dateenrolled and
dateexpiry are not modified, they will be updated to today.
Indeed, dt_from_string returns today without parameters.

Test plan:
- Set dateenrolled and expirydate for a patron
- Using the batch patron mod tool, update any field but dateenrolled and
expirydate.
=> Without this patch, the 2 date fields should be set to today
=> With this patch applied, the 2 date fields should not have been
modified.

Signed-off-by: Karam Qubsi <karamqubsi@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 8 Jesse Weaver 2016-01-21 00:19:21 UTC
PUshed to master, thanks for your work!
Comment 9 Julian Maurice 2016-01-25 11:19:20 UTC
Patch pushed to 3.22.x, will be in 3.22.2
Comment 10 Frédéric Demians 2016-01-25 16:27:01 UTC
This patch has been pushed to 3.20.x, will be in 3.20.8.