Bug 11237

To be honest, I'm confused by the description. I'm a non-librarian. So there is order entry and order receiving, right? When do these fields get set? I didn't go read the code.

When items are created on order (AcqCreateItem = on order) the subfields of those items will be updated as set in this preference automatically during the receive process. For example this will allow to remove the 'on order' status.

Could I ask that the second when be changed? having two whens in the sentence makes it hard to read.

Neither Jonathan nor I are native speakers - maybe just make a suggestion what would sound better to you?

How about?

Upon receiving items, update their subfields if they were created when placing an order (e.g. 995\$o=5)

Does this make sense? I intentionally chose "placing an order", because that is the value in the drop down for the AcqCreateItem system preference above it.

Two things I noticed when I tested.
1) The order of the system preference text box was changed. I don't think that is problem, but felt I should note that.
2) I noticed the default was 0. Should it be 0, or should it be blank? Since this is really outside the scope of updating the explanation, I'm ignoring it, even if it is a potential problem.

(In reply to M. Tompsett from comment #8)
> Two things I noticed when I tested.
> 1) The order of the system preference text box was changed. I don't think
> that is problem, but felt I should note that.

Just looking at the patch, I do not understand your remark.

> 2) I noticed the default was 0. Should it be 0, or should it be blank? Since
> this is really outside the scope of updating the explanation, I'm ignoring
> it, even if it is a potential problem.

I agree that it should be blank. In the code I see a split on this pref without further testing. (May not harm, but correcting it is better.) Although you formally could say that it is actually outside report scope, I would like to ask Jonathan to just handle this tiny 'bug' within the same db revision (in the same update statement). No need to touch the same code in two reports.
For this reason setting it to Failed QA.

(In reply to M. de Rooy from comment #10)
> (In reply to M. Tompsett from comment #8)
> > Two things I noticed when I tested.
> > 1) The order of the system preference text box was changed. I don't think
> > that is problem, but felt I should note that.
> 
> Just looking at the patch, I do not understand your remark.

Before the patch, it was <input> pref description
Now it is: pref description <input>
I thought it is better but maybe i am wrong. Feel free to change the order if you disagree.

> I agree that it should be blank. In the code I see a split on this pref
> without further testing. (May not harm, but correcting it is better.)
> Although you formally could say that it is actually outside report scope, I
> would like to ask Jonathan to just handle this tiny 'bug' within the same db
> revision (in the same update statement). No need to touch the same code in
> two reports.

Done!

(In reply to Jonathan Druart from comment #12)
> Before the patch, it was <input> pref description
> Now it is: pref description <input>
> I thought it is better but maybe i am wrong. Feel free to change the order
> if you disagree.
I agree.


> 
> > I agree that it should be blank. In the code I see a split on this pref
> > without further testing. (May not harm, but correcting it is better.)
> > Although you formally could say that it is actually outside report scope, I
> > would like to ask Jonathan to just handle this tiny 'bug' within the same db
> > revision (in the same update statement). No need to touch the same code in
> > two reports.
> 
> Done!
Thanks.

If Mark would be willing to sign off on the follow-up, I will finish the QA step.

(In reply to M. de Rooy from comment #10)
> (In reply to M. Tompsett from comment #8)
> > Two things I noticed when I tested.
> > 1) The order of the system preference text box was changed. I don't think
> > that is problem, but felt I should note that.
> 
> Just looking at the patch, I do not understand your remark.

In the koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref diff the "pref" line comes before the description initially, and after in the patch. This does not break functionality, but it does change the way it displays in the Acquisition system preferences.

(In reply to M. Tompsett from comment #14)
> (In reply to M. de Rooy from comment #10)
> > (In reply to M. Tompsett from comment #8)
> > > Two things I noticed when I tested.
> > > 1) The order of the system preference text box was changed. I don't think
> > > that is problem, but felt I should note that.
> > 
> > Just looking at the patch, I do not understand your remark.
> 
> In the
> koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/acquisitions.pref
> diff the "pref" line comes before the description initially, and after in
> the patch. This does not break functionality, but it does change the way it
> displays in the Acquisition system preferences.

Thanks for clarification, Mark. Could you sign the follow-up?

Created attachment 24493 [details] [review]
Bug 11237: Update explanation for pref AcqItemSetSubfieldsWhenReceived

Test plan:
Update the updatedb entry and search the pref in the admin module.
The explanation should have been updated.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 24494 [details] [review]
Bug 11237: Update default value for pref AcqItemSetSubfieldsWhenReceived

The default value for this pref should be an empty string, not "0".

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Pushed to master.  Thanks, Jonathan!

*** Bug 11323 has been marked as a duplicate of this bug. ***