Summary: | Don't fill in items.location with NewItemsDefaultLocation if blank | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Cataloging | Assignee: | Bugs List <koha-bugs> |
Status: | NEW --- | QA Contact: | Testopia <testopia> |
Severity: | major | ||
Priority: | P5 - low | CC: | andrew, hannah.co, m.de.rooy, nick, oleonard |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=3481 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=29308 |
||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: | ||
Circulation function: | |||
Bug Depends on: | 27545 | ||
Bug Blocks: | |||
Attachments: | Bug 28895: Don't use NewItemsDefaultLocation for items edition |
Description
Jonathan Druart
2021-08-25 10:26:00 UTC
Created attachment 124083 [details] [review] Bug 28895: Don't use NewItemsDefaultLocation for items edition This patch is adjusting the edition behaviour, not the creation. Should we allow empty location if the pref is set? When a location is explicitly set to blank, editing an item should leave it blank. I think that's the easy one. I don't think that's an inconsistency actually, as the pref should only apply to new items in the first place. So we could fix this independently of the item creation use case. If it's currently possible to set another location than the pref location, I think it would be good to also be able to set an empty one explicitly. Agreed, I would like to be able to set a blank shelving location on a new item, have it go to the location set in NewItemsDefaultLocation, then go to a blank shelving location on a later checkin. Actually, testing this on master this morning it's very much not working as intended. I have NewItemsDefaultLocation set to PROC. On additem.pl, when I create a load the new item page, location is automatically set to PROC. If I create my item without changing it, the item has both location and shelving_location set to PROC. There is no reason to set the default to PROC here, but I guess it doesn't hurt anything. On additem.pl, if I change the shelving location to any value other than PROC before I create my item, both the location and permanent location are set to whatever new value I've selected. Whatever I've selected should go into permanent_location, but location should be PROC. If I delete PROC without selecting something else on creating my item, then location and permanent_location end up as PROC anyway. This bug asserts that in this case I should get location=PROC and permanent_location=NULL. I support that change. This could debatably be a new bug, but there are a lot of old bugs around this syspref already. (In reply to Andrew Fuerste-Henry from comment #5) > Actually, testing this on master this morning it's very much not working as > intended. > > I have NewItemsDefaultLocation set to PROC. > On additem.pl, when I create a load the new item page, location is > automatically set to PROC. If I create my item without changing it, the item > has both location and shelving_location set to PROC. > There is no reason to set the default to PROC here, but I guess it doesn't > hurt anything. > > On additem.pl, if I change the shelving location to any value other than > PROC before I create my item, both the location and permanent location are > set to whatever new value I've selected. > Whatever I've selected should go into permanent_location, but location > should be PROC. > > If I delete PROC without selecting something else on creating my item, then > location and permanent_location end up as PROC anyway. > This bug asserts that in this case I should get location=PROC and > permanent_location=NULL. I support that change. > > This could debatably be a new bug, but there are a lot of old bugs around > this syspref already. In order for UpdateItemLocationOnCheckin to work correctly for items with no shelving location, the NewItemsDefaultLocation needs to stay out of the permanent_location field. If NewItemsDefaultLocation is set to PROC, and UpdateItemLocationOnCheckin has the line "PROC: _PERM_" items with no shelving location remain in processing and don't get shelving location changed to NULL on checkin, as they should. I agree that at item creation, if a shelving location is not selected then the item should have location=PROC and permanent_location=NULL. When editing an item, the NewItemsDefaultLocation currently replaces a NULL value. If the shelving location is NULL, NewItemsDefaultLocation should not replace the NULL upon editing. I've filed a separate bug (Bug 29308) for the fact that NewItemsDefaultLocation is going into permanent_location. That's absolutely a regression that breaks existing workflows. It's also a bit distinct from the question here of whether or not a selection of NULL location should mean NULL permanent_location when using NewItemsDefaultLocation. BUT in testing bug 29308, I confirmed that in 20.05 if one is using NewItemsDefaultLocation but leaves location empty at item creation, one's item is created with the NewItemsDefaultLocation value in items.location but NULL in items.permanent_location. So this is a change in behavior and the consensus here seems to be we should change it back. Moving out of In Discussion. Realized this was sitting in Assigned without an assignee. |