Summary: | Item editor handles clearing dateaccessioned and replacementpricedate inconsistently | ||
---|---|---|---|
Product: | Koha | Reporter: | Marcel de Rooy <m.de.rooy> |
Component: | Cataloging | Assignee: | Marcel de Rooy <m.de.rooy> |
Status: | CLOSED WONTFIX | QA Contact: | Testopia <testopia> |
Severity: | minor | ||
Priority: | P5 - low | CC: | jonathan.druart, m.de.rooy, veron |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: | https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=17512 | ||
Change sponsored?: | --- | Patch complexity: | Trivial patch |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Attachments: |
Bug 17519: Item editor handles clearing dateaccessioned and replacementpricedate inconsistently
Bug 17519: Item editor handles clearing dateaccessioned and replacementpricedate inconsistently |
Description
Marcel de Rooy
2016-10-31 09:46:41 UTC
Created attachment 56984 [details] [review] Bug 17519: Item editor handles clearing dateaccessioned and replacementpricedate inconsistently This patch adds dateaccessioned to the hash built by Items routine _build_default_values_for_mod_marc, used in ModItemFromMarc when modifying an item. Normally, dateaccessioned will already have a value, and this change has no effect. The only exception is: when you modify an item and for some reason want to clear the dateaccessioned field, the item editor did not respond to that change. This change makes it possible to do so. This is consistent with the other date in the item editor (replacementpricedate) where you can already clear the date. Test plan: [1] Do not yet apply this patch. [2] Try to clear date acquired and replacementpricedate in the Item editor. Save and check results. You should still find the original date acquired in the saved record. [3] Apply the patch (restart Plack?), restore replacementpricedate, and repeat step 2. Both dates should be cleared. Created attachment 61013 [details] [review] Bug 17519: Item editor handles clearing dateaccessioned and replacementpricedate inconsistently This patch adds dateaccessioned to the hash built by Items routine _build_default_values_for_mod_marc, used in ModItemFromMarc when modifying an item. Normally, dateaccessioned will already have a value, and this change has no effect. The only exception is: when you modify an item and for some reason want to clear the dateaccessioned field, the item editor did not respond to that change. This change makes it possible to do so. This is consistent with the other date in the item editor (replacementpricedate) where you can already clear the date. Test plan: [1] Do not yet apply this patch. [2] Try to clear date acquired and replacementpricedate in the Item editor. Save and check results. You should still find the original date acquired in the saved record. [3] Apply the patch (restart Plack?), restore replacementpricedate, and repeat step 2. Both dates should be cleared. Followed test plan, date acquired can be deleted as expected. Signed-off-by: Marc Véron <veron@veron.ch> (In reply to Marc Véron from comment #2) > Signed-off-by: Marc Véron <veron@veron.ch> Thanks! It seems that replacement price is ok before this patch, and the patch does not affect it. Should we rename the bug report and the commit msg? (In reply to Jonathan Druart from comment #4) > It seems that replacement price is ok before this patch, and the patch does > not affect it. Should we rename the bug report and the commit msg? No problem. What would you suggest? The inconsistency is in the comparison between how both fields behave. So maybe: Handle clearing dateaccessioned just like replacementpricedate? Or just Fix clearing dateaccessioned? Marcel, sorry but I do not understand your answer. I am just saying that replacementpricedate is working correctly without your patch, so we should remove it from the bug title and commit message. Do you agree? (In reply to Jonathan Druart from comment #6) > Marcel, sorry but I do not understand your answer. > I am just saying that replacementpricedate is working correctly without your > patch, so we should remove it from the bug title and commit message. > Do you agree? Yes, no problem. Will this effect the automatic setting of dateaccessioned on leaving it empty when saving? We rely on this behaviour at the moment. (In reply to Katrin Fischer from comment #8) > Will this effect the automatic setting of dateaccessioned on leaving it > empty when saving? We rely on this behaviour at the moment. Please explain what you mean. Where do you rely on exactly? We have trained libraries that they don't have to fill out the field because Koha does it in the background automatically. I was wondering, because this patch talks about clearing the field. At the moment it's simply not possible to have an item without a dateaccessioned which is often used as creation data in various reports. (In reply to Katrin Fischer from comment #10) > We have trained libraries that they don't have to fill out the field because > Koha does it in the background automatically. I was wondering, because this > patch talks about clearing the field. At the moment it's simply not possible > to have an item without a dateaccessioned which is often used as creation > data in various reports. No, this does not affect adding an item with an empty date. The sub _set_defaults_for_add will default dateaccessioned to today. The change is in ModItemFromMarc. If the user cleared the date when modifying an item, this change was not picked up. Now it is possible just as it already was for replacementpricedate. Note that we are still not 100% consistent imo. Only a little bit more ;) If we want a default in dateaccessioned, we should better take care of that in the interface rather than doing it invisibly in a set_defaults sub. Why should a user understand that we fill dateaccessioned but we do not fill replacementpricedate? But that is a topic for another report.. Ok, so when adding an item the behaviour will be the same. But it will be possible to empty out the date on changing an item, correct? I am not sure how replacementpricedate was intended to work. I have heard somewhere that it was used to decrease the replacement price over time as the item aged, but not entirely sure. I am not sure we have any functionality tied to it at the moment in Koha itself. (In reply to Katrin Fischer from comment #12) > Ok, so when adding an item the behaviour will be the same. > But it will be possible to empty out the date on changing an item, correct? Correct. The discussion seems to be finished ;) Actually no - I just didn't have the time to write another answer. I think this patch makes the behaviour inconsistent. If you can empty out on editing, but not on adding the item - that will look like a bug. The behaviour right now is consistent as the field is forced to be mandatory and as I said, we rely on this in some contexts - for example in context of inventory books. Maybe we should discuss use cases first? Things that come to my mind: - new books list - inventory book (electronical, sometimes printed out) - sorting by accession date in the OPAC - weeding reports/workflows (In reply to Katrin Fischer from comment #15) > Actually no - I just didn't have the time to write another answer. > > I think this patch makes the behaviour inconsistent. If you can empty out on > editing, but not on adding the item - that will look like a bug. > > The behaviour right now is consistent as the field is forced to be mandatory > and as I said, we rely on this in some contexts - for example in context of > inventory books. > > Maybe we should discuss use cases first? > > Things that come to my mind: > - new books list > - inventory book (electronical, sometimes printed out) > - sorting by accession date in the OPAC > - weeding reports/workflows OK. The current behavior is not really consistent (if you compare with the other date). But in view of its low importance, I would rather close this report now if it disturbs other users too much.. No problem! Sorry Marcel, didn't want to upset you - it just felt not 'quite right' yet to me. Maybe having a separate 'created_on' column on items would be a good idea long term. |