Bug 8307

Summary: Set a value for items when receiving
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: AcquisitionsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: enhancement    
Priority: P5 - low CC: alexandre.lengereau, chris, crohde, frederic.durand, gmcharlt, katrin.fischer, m.de.rooy, mathsabypro, paul.poulain
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9088
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10043
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 12557    
Attachments: Bug 8307: Set a value for items when ordering
Bug 8307: Set a value for items when ordering
Bug 8307: Set a value for items when ordering
Bug 8307: Set a value for subfields when receiving
Bug 8307: Set a value for subfields when receiving
Bug 8307: Set a value for subfields when receiving
Bug 8307: Set a value for subfields when receiving
Bug 8307: Set a value for subfields when receiving
Bug 8307: Set a value for subfields when receiving
Bug 8307: QA Followup for finishreceive.pl

Description Jonathan Druart 2012-06-25 12:07:12 UTC
If the system preference AcqCreateItem is "Create an item when ordering", we would like to set a value for items.
For example you could define status for the field items.notforloan (995$o for UNIMARC) with 4 = On order, 5: On treatment.
Then, it would be great to fill the item field 995$o with "5".
Comment 1 Jonathan Druart 2012-06-25 12:35:10 UTC Comment hidden (obsolete)
Comment 2 Alexandre Lengereau 2012-08-06 08:11:03 UTC
SandBox Test result:

<h1>Something went wrong !</h1>Applying: Bug 8307: Set a value for items when ordering<br/>
Using index info to reconstruct a base tree...<br/>
Falling back to patching base and 3-way merge...<br/>
Auto-merging installer/data/mysql/sysprefs.sql<br/>
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql<br/>
Auto-merging installer/data/mysql/updatedatabase.pl<br/>
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl<br/>
Failed to merge in the changes.<br/>
Patch failed at 0001 Bug 8307: Set a value for items when ordering<br/>
When you have resolved this problem run git am --resolved.<br/>
If you would prefer to skip this patch, instead run git am --skip.<br/>
To restore the original branch and stop patching run git am --abort.<br/>
Bug 8307 - Set a value for items when receiving<br/>
<br/>
Bug 8307: Set a value for items when ordering<br/>
Apply? [yn] <br/>
Patch left in /tmp/Bug-8307-Set-a-value-for-items-when-ordering-K_sCQB.patch<br/>
<br/>=== translation installation ===<br/>
If you use this system for the 1st time, you may find usefull to reach the wiki page http://wiki.koha-community.org/wiki/Sandboxes to have some information about those results, to check if everything went well & if you can test the patch properly
Comment 3 Jonathan Druart 2012-08-06 10:12:56 UTC Comment hidden (obsolete)
Comment 4 Paul Poulain 2012-08-10 08:02:12 UTC
Patch tested with a sandbox, by Delaye Stephane <stephane.delaye@biblibre.com>
Comment 5 Paul Poulain 2012-08-10 08:02:30 UTC Comment hidden (obsolete)
Comment 6 Marcel de Rooy 2012-08-30 11:32:31 UTC
QA Comment:
I am just having doubts over the form of the preference. Filling in something like 952x=9 and parsing that.
Could you please send a mail to the dev list and acquire some community consensus about this unusual form?
Thank you.
Comment 7 Jonathan Druart 2012-11-15 15:23:42 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2012-11-15 15:27:06 UTC
This new patch change the format of the syspref (subfield_1=value_1|...|subfield_n=value_n)
Like this, it is possible to set any items subfields (not specific to the status value).
Comment 9 Marcel de Rooy 2012-11-19 09:09:19 UTC
Jonathan:
Could you have a look at 9088 too. That report deals with default values for AcqCreateItem=cataloging or receiving.
Thanks
Marcel
Comment 10 Frederic Durand 2013-03-18 11:19:31 UTC
Applying: Bug 8307: Set a value for subfields when receiving
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/sysprefs.sql
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 8307: Set a value for subfields when receiving
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Bug 8307 - Set a value for items when receiving
Comment 11 Frederic Durand 2013-03-18 11:33:12 UTC
Sorry was a bit quick for copy the error message, i tried to test this patch on sandbox, it did not work correctly.
Comment 12 Jonathan Druart 2013-03-18 12:26:45 UTC Comment hidden (obsolete)
Comment 13 Paul Poulain 2013-03-18 14:32:49 UTC
Patch tested with a sandbox, by Frederic Durand <frederic.durand@unilim.fr>
Comment 14 Paul Poulain 2013-03-18 14:33:13 UTC Comment hidden (obsolete)
Comment 15 Marcel de Rooy 2013-06-03 07:58:07 UTC Comment hidden (obsolete)
Comment 16 Marcel de Rooy 2013-06-03 07:58:37 UTC
Two simple merge conflicts resolved.
Comment 17 Marcel de Rooy 2013-06-03 08:03:44 UTC Comment hidden (obsolete)
Comment 18 Marcel de Rooy 2013-06-03 08:04:03 UTC
Forgot: 3.11 -> 3.13
Comment 19 Marcel de Rooy 2013-06-03 09:56:11 UTC
QA Comment:
Tested this under AcqCreateItem=ordering. And verified that it did not change the items with AcqCreateItem=receiving.
It works (I only struggled with booksellerid 952e that was overwritten later on). [We use AcqCreateItem=cataloging and have a different use on 952e. Probably, this field should not be in the marc 952 tag, as it is now.]
No complaints from qa-tools.

But I do have my doubts over this feature.
First, AcqItemSetSubfieldsWhenReceived only works for AcqCreateItem=ordering. This should be noted more clearly in the pref description.
What about setting an item value for AcqCreateItem=receiving or cataloging btw?

Second, it looks like you want to update some status in the acquisition process and you implemented it in a perhaps somewhat unusual way that makes completely different applications possible. I am afraid that it could make code maintenance more difficult in the long run.

Added a small followup for the split statement.

Setting the status to Passed QA. I am not blocking this patch, but leave the final comment to RM.
Comment 20 Marcel de Rooy 2013-06-03 09:57:14 UTC
Created attachment 18607 [details] [review]
Bug 8307: Set a value for subfields when receiving

If items are created when ordering, this patch allows to add a value for
some items subfields.

Test plan:
Define status for items.notforloan (mapping 995$o in unimarc), for
example 4:On order, 5:On treatment

Set the Syspref AcqCreateItem on "ordering".

ACQ framework : set default value = 4 for 995$o (in unimarc)

Syspref AcqItemSetSubfieldsWhenReceived : set "o=5|b='foo bar'"

When ordering the item, default status will be 4 ; when receiving the
item, status will be changed from 4 to 5. The subfield b have to contain
'foo bar'

Signed-off-by: Frederic Durand <frederic.durand@unilim.fr>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 21 Marcel de Rooy 2013-06-03 09:57:58 UTC
Created attachment 18608 [details] [review]
Bug 8307: QA Followup for finishreceive.pl

Only split the expression a=b on the first = sign.
Tested with the very unlikely z=x=1 :)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 22 Katrin Fischer 2013-06-03 10:09:00 UTC
Hi Marcel, 

I have been following this bug as it's an interesting feature I get asked about a lot. I am not sure I understand your concerns:

>First, AcqItemSetSubfieldsWhenReceived only works for AcqCreateItem=ordering. >This should be noted more clearly in the pref description.
>What about setting an item value for AcqCreateItem=receiving or cataloging btw?

Not sure how it could work for those settings - it seems to me that in these cases we either have no item to change or no defined event to trigger the change?
Comment 23 Marcel de Rooy 2013-06-03 10:15:10 UTC
(In reply to comment #22)
> >First, AcqItemSetSubfieldsWhenReceived only works for AcqCreateItem=ordering. >This should be noted more clearly in the pref description.
> >What about setting an item value for AcqCreateItem=receiving or cataloging btw?
> 
> Not sure how it could work for those settings - it seems to me that in these
> cases we either have no item to change or no defined event to trigger the
> change?

When you set it to receiving, you do have a new item. In that case it would function as a sort of default. So perhaps we should not do that? Trigger would be the same (confirm receive).
In another way for cataloging, I opened another report once for default values for fund and itemtype (does not apply anymore).
Just curious if we can get a broader application for this pref.
Comment 24 Katrin Fischer 2013-06-03 10:26:48 UTC
Hm, I think dealing with defaults is maybe better done in a different place as this is focused on changing existing values from one value to another. For the use case you gave maybe it would work to use default values in the frameworks?
Comment 25 Marcel de Rooy 2013-06-03 10:29:24 UTC
(In reply to comment #24)
> Hm, I think dealing with defaults is maybe better done in a different place
> as this is focused on changing existing values from one value to another.
> For the use case you gave maybe it would work to use default values in the
> frameworks?

Yes for the item values that probably would be enough. (Not for the fund and itemtype.) So it may be hard to widen the scope.
Comment 26 Galen Charlton 2013-10-31 23:50:31 UTC
Pushed to master.  Thanks, Jonathan!

Being able to set item record values during receiving will be well received, but I must say that I am not a fan of how things have drifted:

- It would be better to refer to item fields by their names, not how they happen to be mapped in a MARC framework.  (By the way, I have an itch to stamp out the use of AddItemFromMarc and ModItemFromMarc, replacing it with direct calls to AddItem / ModItem, or their eventual DBIC equivalents.  A helper routine to map a MARC field to an item should be used only when necessary -- during record import, primarily, and not item record editing).
- It's not great that depending on what stage of the process you're dealing with, the configuration interface is either a syspref or the MARC frameworks.
Comment 27 Katrin Fischer 2013-11-08 09:59:47 UTC
I think the syspref description is a bit confusing:

AcqItemSetSubfieldsWhenReceived
Set subfields for item when items are created when receiving (e.g. o=5|a="foo bar")

Shouldn't it be something like: 
Update subfields in items on receive when items are created when ordering (e.g.  o=5|a="foo bar")
Comment 28 Jonathan Druart 2013-11-12 09:29:15 UTC
(In reply to Katrin Fischer from comment #27)
> I think the syspref description is a bit confusing:
> 
> AcqItemSetSubfieldsWhenReceived
> Set subfields for item when items are created when receiving (e.g.
> o=5|a="foo bar")
> 
> Shouldn't it be something like: 
> Update subfields in items on receive when items are created when ordering
> (e.g.  o=5|a="foo bar")

Yes, you are right. I proposed a patch on bug 11237.