In C4/Letters.pm, sub _parseletter changes values that are passed by reference. E.g. patron's expiry date can change from iso format to syspref format,leading to strange behaviour in the calling routines (see Bug 15423).
Created attachment 45994 [details] [review] Bug 15429 - sub _parseletter should not change referenced values In C4/Letters.pm, sub _parseletter changes values that are passed by reference. E.g. patron's expiry date can change from iso format to syspref format, leading to strange behaviour in the calling routines (see Bug 15423). This patch makes sub _parseletter work on a copy of the referenced values. (Submitted to get feedback - is this the way to go?)
To reproduce an issue with strange value changes (patron's expiry date) see Bug 15423.
*** Bug 15423 has been marked as a duplicate of this bug. ***
Switching to "Needs Signoff" Test plan: Reproduce without this patch: - Try to repruduce issue in Bug 15423 (comment #2) - make sure that patron with batch checkout permissionhas nothing checked out - Go to batch checkout - enter 2 valid barcodes - Then enter a line with one space character - Then enter one more empty line or a barcode Example: e.g. 1 2 3 ('empty' line contains one space character) Expected result: Software error The given date (13/05/2022 00:00) does not match the date format (iso) at /usr/share/kohaclone/Koha/DateUtils.pm line 153. - If not yet done, apply patch 15432 - prove -v t/db_dependent/Letters.t Expected Result: OK Test with patch: - apply patch - try to reproduce software error above Expected result: No software error, checkouts OK - prove -v t/db_dependent/Letters.t Expected result: OK (Note: I did not manage to create a test to reproduce Bug 15423 in Letters.t)
*** Bug 15433 has been marked as a duplicate of this bug. ***
Setting to 'critical' because Bug 15433 (resolved by this patch) has status 'critical'
Created attachment 46072 [details] [review] Bug 15429 - sub _parseletter should not change referenced values In C4/Letters.pm, sub _parseletter changes values that are passed by reference. E.g. patron's expiry date can change from iso format to syspref format, leading to strange behaviour in the calling routines (see Bug 15423). This patch makes sub _parseletter work on a copy of the referenced values. (Submitted to get feedback - is this the way to go?) Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Good solution to real time bomb.
Created attachment 46073 [details] [review] Bug 15429 UT for _parseletter modifying its parameter Before/after applying the related patch, do: prove -v t/db_dependent/Letters.t
Created attachment 46076 [details] [review] Bug 15429 - sub _parseletter should not change referenced values In C4/Letters.pm, sub _parseletter changes values that are passed by reference. E.g. patron's expiry date can change from iso format to syspref format, leading to strange behaviour in the calling routines (see Bug 15423). This patch makes sub _parseletter work on a copy of the referenced values. (Submitted to get feedback - is this the way to go?) Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Good solution to real time bomb. Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Created attachment 46077 [details] [review] Bug 15429 UT for _parseletter modifying its parameter Before/after applying the related patch, do: prove -v t/db_dependent/Letters.t Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
We should not do that, but it is definitely the safest and easiest way to go.
Seems to have been introduced by 6810.
(In reply to Jonathan Druart from comment #12) > Seems to have been introduced by 6810. waitingdate was already modified before bug 6810.
Pushed to master, thanks Marc!
Patch pushed to 3.22.x, will be in 3.22.2
This patch has been pushed to 3.20.x, will be in 3.20.8.
Created attachment 46160 [details] [review] Bug 15429 Followup to avoid test failing