Bug 17512 - Improve handling dates in C4::Items
Summary: Improve handling dates in C4::Items
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Cataloging (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-10-27 12:55 UTC by Marcel de Rooy
Modified: 2017-12-07 22:20 UTC (History)
6 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
Bug 17512: Improve handling dates in C4::Items (5.69 KB, patch)
2016-10-27 13:18 UTC, Marcel de Rooy
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17512: Improve handling dates in C4::Items (5.71 KB, patch)
2017-02-01 22:37 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17512: Improve handling dates in C4::Items (5.77 KB, patch)
2017-02-02 11:11 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2016-10-27 12:55:39 UTC
This is a follow-up on the internal server error on 0000-00-00 in the items
column onloan. This patch deals with preventing to have such dates at all in the date fields of items.
Comment 1 Marcel de Rooy 2016-10-27 13:18:47 UTC
Created attachment 56920 [details] [review]
Bug 17512: Improve handling dates in C4::Items

This is a follow-up on the internal server error on 0000-00-00 in the items
column onloan. This patch deals with preventing to have such dates at all
in the date fields of items.

It is accomplished by:
[1] Adding a (private) subroutine _mod_item_dates. It takes an item hash
    and replaces date values if needed.
[2] AddItem and ModItem call _koha_new_item resp. koha_modify_item. In these
    routines a call to the new _mod_item_dates is inserted.
[3] Although the routine is actually private, I have added some unit tests
    to Items.t.

Test plan:
[1] Add a new item. Fill a correct date in dateaccessioned and an invalid
    date in Price effective from (=replacementpricedate).
[2] Verify that dateaccessioned is saved correctly and replacementpricedate
    is still null (does not contain 0000-00-00).
[3] Edit the item again. Fill some text in dateaccessioned and put a correct
    date in replacementpricedate. Verify the results.
[4] Run t/db_dependent/Items.t
Comment 2 Mika 2016-11-30 21:26:15 UTC
Passed step 4 of test plan, however invalid dates still show as 0000-00-00 rather than null when saved for both dateaccessioned and replacementpricedate.
Comment 3 Marcel de Rooy 2016-12-01 11:42:52 UTC
(In reply to Mika from comment #2)
> Passed step 4 of test plan, however invalid dates still show as 0000-00-00
> rather than null when saved for both dateaccessioned and
> replacementpricedate.

Thanks for testing !
I cannot reproduce your test results.
If am entering e.g. some text "aa" in replacementpricedate, the date will become null in the record, and shows blank on the form.

If you test under Plack, please make sure to restart Plack after applying the patches. Could that be a cause ?
Comment 4 Josef Moravec 2017-02-01 22:37:30 UTC
Created attachment 59759 [details] [review]
[SIGNED-OFF] Bug 17512: Improve handling dates in C4::Items

This is a follow-up on the internal server error on 0000-00-00 in the items
column onloan. This patch deals with preventing to have such dates at all
in the date fields of items.

It is accomplished by:
[1] Adding a (private) subroutine _mod_item_dates. It takes an item hash
    and replaces date values if needed.
[2] AddItem and ModItem call _koha_new_item resp. koha_modify_item. In these
    routines a call to the new _mod_item_dates is inserted.
[3] Although the routine is actually private, I have added some unit tests
    to Items.t.

Test plan:
[1] Add a new item. Fill a correct date in dateaccessioned and an invalid
    date in Price effective from (=replacementpricedate).
[2] Verify that dateaccessioned is saved correctly and replacementpricedate
    is still null (does not contain 0000-00-00).
[3] Edit the item again. Fill some text in dateaccessioned and put a correct
    date in replacementpricedate. Verify the results.
[4] Run t/db_dependent/Items.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 5 Marcel de Rooy 2017-02-02 07:25:06 UTC
Thanks Josef.
Comment 6 Jonathan Druart 2017-02-02 11:11:19 UTC
Created attachment 59789 [details] [review]
Bug 17512: Improve handling dates in C4::Items

This is a follow-up on the internal server error on 0000-00-00 in the items
column onloan. This patch deals with preventing to have such dates at all
in the date fields of items.

It is accomplished by:
[1] Adding a (private) subroutine _mod_item_dates. It takes an item hash
    and replaces date values if needed.
[2] AddItem and ModItem call _koha_new_item resp. koha_modify_item. In these
    routines a call to the new _mod_item_dates is inserted.
[3] Although the routine is actually private, I have added some unit tests
    to Items.t.

Test plan:
[1] Add a new item. Fill a correct date in dateaccessioned and an invalid
    date in Price effective from (=replacementpricedate).
[2] Verify that dateaccessioned is saved correctly and replacementpricedate
    is still null (does not contain 0000-00-00).
[3] Edit the item again. Fill some text in dateaccessioned and put a correct
    date in replacementpricedate. Verify the results.
[4] Run t/db_dependent/Items.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Kyle M Hall 2017-02-14 13:57:44 UTC
Pushed to master for 17.05, thanks Marcel!
Comment 8 Katrin Fischer 2017-02-14 19:48:46 UTC
This patch has been pushed to 16.11.x and will be in 16.11.04.
Comment 9 Julian Maurice 2017-02-20 09:27:31 UTC
Pushed to 3.22.x for 3.22.17
Comment 10 Mason James 2017-02-23 20:20:27 UTC
Pushed to 16.05.x, for 16.05.10 release