Bug 4959

Summary: Language inconsistencies on basket groups; skip confirmation when closing basket.
Product: Koha Reporter: Nicole C. Engard <nengard>
Component: AcquisitionsAssignee: Marcel de Rooy <m.de.rooy>
Status: CLOSED FIXED QA Contact: Bugs List <koha-bugs>
Severity: minor    
Priority: PATCH-Sent (DO NOT USE) CC: chris, colin.campbell, katrin.fischer, koha.sekjal, m.de.rooy, paul.poulain
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Attachments: Patch
Screen shot
Rebased patch
Follow up patch for install
Follow up patch for installing pref
Installation patch for 3.4.X

Description Nicole C. Engard 2010-07-07 21:05:54 UTC
When closing a basket it asks if you want to create a purchase order and then it takes you the basket grouping page.  I think that to be consistent, basket grouping language needs to be changed to say purchase order anywhere that the 'basket group' language appears.  A basket group can be only 1 basket - but a purchase order can contain 1 or more baskets - also this seems to be the language that the librarians I talked to use the most consistently.
Comment 1 Colin Campbell 2010-07-08 09:12:44 UTC
I think there is a need for a vocabulary list defining the concepts used in acquisitions. Baskets seems especially problematic as its a terminology not used by any acquisitions librarians I'm encountering and after explaining it some then say 'But why would we want that? can you hide the baskets?'. And its important to know what it means as the language is very loose in practse (how many definitions of order does your library use?) Compared to other systems I'm aware of our basket == order our order == orderline. But in modern systems orders much more often have a 1 orderline per order style.
(I'm just glad I don't have to translate this)
Comment 2 Nicole C. Engard 2010-07-08 11:13:58 UTC
I'm with you on that - but I was concerned about suggesting a change from basket since it has been that way so long and librarians using Koha have gotten used to it.

I am training today and I did have to explain that a basket is an order and that a basket group is a purchase order - as for and order == orderline - where does Koha Acq refer to orders?
Comment 3 Marcel de Rooy 2010-11-25 08:55:37 UTC
As of today, the master version still asks if I want to create an purchase order. This is really confusing. It is not evident AT ALL that this means that a basket group is created. In my interpretation, a purchase order in Koha is still not more than an order line.
If I really do not want basket groups, I would even prefer an option to get rid of this question and the possible creation of these groups when somebody unadvertently clicks the checkbox.
Comment 4 Marcel de Rooy 2010-11-25 12:53:03 UTC
Sending a proposed patch to dev list. Comments are welcome. 

This patch adds a new pref SkipBasketConfirmations. This adds the option to skip
confirmations on closing and reopening a basket. If you skip the confirm,
you do not create a new basket group.
The confusing line Create a purchase order (when closing a basket) is replaced
by Attach basket to a new basket group with the same name.
A warning for a null value on basketgroupid is fixed.

See http://lists.koha-community.org/pipermail/koha-patches/2010-November/013102.html
Comment 5 Marcel de Rooy 2010-11-29 10:40:15 UTC
Revised patch. See http://lists.koha-community.org/pipermail/koha-patches/2010-November/013115.html
Preference renamed to BasketConfirmations in order to prevent double negative (not skip...)
Comment 6 Marcel de Rooy 2011-01-17 14:02:34 UTC Comment hidden (obsolete)
Comment 7 Paul Poulain 2011-04-04 09:22:23 UTC
The patch applies, does what he said, but i've some troubles with what it does: if you don't create "basketgroups" (whatever you call them ;-) ), it means you can't create PDFs. Is it what you want to do ?
I was thinking that with this syspref being set, you had'nt the question, but the basketgroup was automatically created, without asking, to have librarians getting the PDF immediatly.

That would be handy for libraries that create one basket, close it and it's done.
Comment 8 Marcel de Rooy 2011-04-04 10:03:06 UTC
(In reply to comment #7)
> The patch applies, does what he said, but i've some troubles with what it does:
> if you don't create "basketgroups" (whatever you call them ;-) ), it means you
> can't create PDFs. Is it what you want to do ?
> I was thinking that with this syspref being set, you had'nt the question, but
> the basketgroup was automatically created, without asking, to have librarians
> getting the PDF immediatly.
> That would be handy for libraries that create one basket, close it and it's
> done.

That's why I created it. We use it that way. Actually, we print or mail a custom pdf file from the basket screen already.
By default, the confirmation question appears however. So it should not be a problem for anybody?
Comment 9 Katrin Fischer 2011-04-04 15:28:07 UTC
Hi Marcel,

Paul and I were discussing the workflow you describe. Do you use the PDF creation feature from basket groups or do you create the PDF in another way? Like using a plugin for Firefox?
Comment 10 Marcel de Rooy 2011-04-06 11:21:52 UTC
Created attachment 3735 [details]
Screen shot

In reply to comment9: We have a custom Print/Mail button on basket.pl
Comment 11 Marcel de Rooy 2011-04-11 10:38:51 UTC
Created attachment 3862 [details] [review]
Rebased patch

Rebased for TT.
Comment 12 Chris Cormack 2011-04-11 22:07:34 UTC
This is the first patch to be rebased onto Template::Toolkit, a big thank you to Marcel and please can we get this signed off as a reward :)
Comment 13 Katrin Fischer 2011-04-12 21:06:26 UTC
Sign off from Colin on the mailing list:
http://lists.koha-community.org/pipermail/koha-patches/2011-April/014801.html
Comment 14 Katrin Fischer 2011-04-12 21:09:09 UTC
The database and sql files are missing from the patch!
Comment 15 Marcel de Rooy 2011-04-27 10:53:23 UTC Comment hidden (obsolete)
Comment 16 Marcel de Rooy 2011-04-27 10:55:35 UTC
Rebased patch still applies to master (3.05.00.001). Has been signed already.
Second follow up patch installs the pref. Needs still signoff.
Comment 17 Marcel de Rooy 2011-04-27 11:05:16 UTC
Created attachment 4010 [details] [review]
Follow up patch for installing pref
Comment 18 Chris Cormack 2011-05-12 06:46:25 UTC
Follow up patch signed off and sent to the list
Comment 19 Paul Poulain 2011-08-03 10:52:13 UTC
QA comment

Reading the patch and thinking of ergonomy rules we try to follow in Koha, I think this patch addresses a problem, but in a wrong way.

Ergonomic rule = don't ask for confirmation if you can cancel the operation. For example, when you add something, you can delete it after, so don't ask "do you want to add this". Usually, confirmations are added only for deletions.

So I would say :
* remove the confirmation for reopening the basket, it's useless (you can always close it again)
* For the close with creating a basket group or no form, it's still needed (as it's not only a confirmation), so i'm OK with the 
- <label for="createbasketgroup">create a purchase order now?</label>
+ <label for="createbasketgroup">Attach this basket to a new basket group with the same name</label>

benefit = no syspref added. (I'm strongly against multiplicating sysprefs, we already have too many !)

IAN = I propose QA failed (but won't set it)
Comment 20 Marcel de Rooy 2011-08-03 11:03:13 UTC
(In reply to comment #19)
> QA comment
> 
> Reading the patch and thinking of ergonomy rules we try to follow in Koha, I
> think this patch addresses a problem, but in a wrong way.
> 
> Ergonomic rule = don't ask for confirmation if you can cancel the operation.
> For example, when you add something, you can delete it after, so don't ask "do
> you want to add this". Usually, confirmations are added only for deletions.
> 
> So I would say :
> * remove the confirmation for reopening the basket, it's useless (you can
> always close it again)
> * For the close with creating a basket group or no form, it's still needed (as
> it's not only a confirmation), so i'm OK with the 
> - <label for="createbasketgroup">create a purchase order now?</label>
> + <label for="createbasketgroup">Attach this basket to a new basket group with
> the same name</label>
> 
> benefit = no syspref added. (I'm strongly against multiplicating sysprefs, we
> already have too many !)
> 
> IAN = I propose QA failed (but won't set it)

This patch just came forth out of Koha not being so ergonomic in this area! I can understand that you do not want unnecessary sysprefs. If it now fails QA after being rebased a number of times and signoff, I suggest to close the patch.
Too bad for the time being spent on it.
Comment 21 Chris Cormack 2011-08-03 11:09:11 UTC
I dont share Pauls aversion to system preferences, I think the configurability of Koha is it's strength not a weakness. So not adding a system preference doesn't add any weight to the argument for me.

I see no harm in pushing this patch, but if Ian sees a QA problem with it, I would reconsider.
Comment 22 Paul Poulain 2011-08-03 11:31:19 UTC
Chris = What I tried to say is that the syspref is useless: it solves the -real- issue by adding a syspref everybody should/will set to "don't confirm".
That's why i'm proposing just to change the wording of the message, and remove the confirmation for reopening the basket.

The change on the wording will fix one of the problem, removing the message will fix the other one, and no one will complain about the "do you confirm reopening the basket"

The patch should be done in a few minutes, and I'm OK to write it if you want.

Marcel = sorry for the time spent on it.
Comment 23 Marcel de Rooy 2011-08-03 11:33:15 UTC
(In reply to comment #22)
> Chris = What I tried to say is that the syspref is useless: it solves the
> -real- issue by adding a syspref everybody should/will set to "don't confirm".
> That's why i'm proposing just to change the wording of the message, and remove
> the confirmation for reopening the basket.
> 
> The change on the wording will fix one of the problem, removing the message
> will fix the other one, and no one will complain about the "do you confirm
> reopening the basket"
> 
> The patch should be done in a few minutes, and I'm OK to write it if you want.
> 
> Marcel = sorry for the time spent on it.

Will sign your patch if it solves this issue ;)
Comment 24 Ian Walls 2011-08-03 21:09:41 UTC
I believe the system preference is a good way of addressing the second half of this bug, the "skip confirmation when closing" part.  Let users determine if adding the closed basket to a basketgroup is part of their Acquisitions workflow or not.

I would agree that the confirmation for the reopening of a basket should be removed, but that is outside the scope of this bug report.  At least with this patch confirmations going both ways are consistent.

I'm marking this patch as Passed QA.  For the removal of the basket reopen confirmation, please open another bug report.
Comment 25 Ian Walls 2011-08-03 21:10:35 UTC
Please note that BOTH patches, the code and the syspref installation, have Passed QA; forgot to mention that before clicking 'Save changes'.
Comment 26 Chris Cormack 2011-08-03 21:55:51 UTC
Pushinated, please test
Comment 27 Marcel de Rooy 2011-08-11 06:25:11 UTC
Created attachment 4955 [details] [review]
Installation patch for 3.4.X

Rebased for 3.4.X
Comment 28 Marcel de Rooy 2011-10-13 08:20:25 UTC
Two months passed: closing it myself..