Bug 7228 - can't order more than 1 when importing all from staged file
Summary: can't order more than 1 when importing all from staged file
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Christophe Croullebois
QA Contact: Ian Walls
URL:
Keywords:
Depends on:
Blocks: 8803 10501
  Show dependency treegraph
 
Reported: 2011-11-15 21:59 UTC by Nicole C. Engard
Modified: 2014-12-07 20:02 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
error (77.09 KB, image/png)
2011-11-15 21:59 UTC, Nicole C. Engard
Details
proposed patch (2.15 KB, patch)
2013-04-18 12:13 UTC, Christophe Croullebois
Details | Diff | Splinter Review
proposed patch (2.15 KB, patch)
2013-04-18 12:35 UTC, Christophe Croullebois
Details | Diff | Splinter Review
[PATCH 1/1][SIGNED OFF] bug 7228 : can't add items in an order with Modify (2.22 KB, patch)
2013-04-25 12:35 UTC, Mathieu Saby
Details | Diff | Splinter Review
[PASSED QA] Bug 7228: can't add items in an order with Modify (3.36 KB, patch)
2013-04-30 05:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 7228: can't add items in an order with Modify (3.36 KB, patch)
2013-04-30 05:43 UTC, Katrin Fischer
Details | Diff | Splinter Review
patch rebased for 3.10.x (3.39 KB, patch)
2013-05-14 08:28 UTC, Christophe Croullebois
Details | Diff | Splinter Review
patch rebased for 3.8.x (3.44 KB, patch)
2013-05-14 08:33 UTC, Christophe Croullebois
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2011-11-15 21:59:20 UTC
Created attachment 6314 [details]
error

If you choose to import all from a staged file you can only have 1 items per line item - you cannot at this time modify the quantity, if you try by clicking on modify it says you can't do it.
Comment 1 Christophe Croullebois 2013-04-18 12:13:27 UTC Comment hidden (obsolete)
Comment 2 Christophe Croullebois 2013-04-18 12:35:28 UTC Comment hidden (obsolete)
Comment 3 Mathieu Saby 2013-04-18 12:39:26 UTC
Patch made for Rennes 2. I will test it ;-)
Comment 4 Mathieu Saby 2013-04-18 16:20:34 UTC
Christophe, your patch is made for AcqCreateItem = on receive.
Don't you think it could also be useful for ppl using  AcqCreateItem = on cataloguing?

For me the current behavior is good only if you create items when you order...

M. Saby
Comment 5 Christophe Croullebois 2013-04-25 12:01:59 UTC
Hi Mathieu,
no, in fact the patch shows the warning, and prohibits to add items in one case => AcqCreateItem = placing..
So, about the other two cases there is no warning, no prohibition.
Comment 6 Mathieu Saby 2013-04-25 12:29:06 UTC
Sorry, I did not read it well. I'm going to test.
Mathieu
Comment 7 Mathieu Saby 2013-04-25 12:35:32 UTC Comment hidden (obsolete)
Comment 8 Mason James 2013-04-27 14:16:10 UTC
(In reply to comment #7)
> Created attachment 17678 [details] [review] [review]
> [PATCH 1/1][SIGNED OFF] bug 7228 : can't add items in an order with Modify

hmmm, i think we need a test plan here, so qa-team can test this properly...

can we get a test plan please, before QA?
Comment 9 Mathieu Saby 2013-04-27 15:36:24 UTC
Here is the test I made for signing off : 
after applying the patch
- syspref AcqCreateItem  : create items on receiving
- in a basket, create an order (quantity = 1)
- save the order
- reopen the order
- change the quantity (2 instead of 1)
- save the order
=> changing quantity was not possible before the patch

- syspref AcqCreateItem  : create items on cataloguing
- in a basket, create an order (quantity = 1)
- save the order
- reopen the order
- change the quantity (2 instead of 1)
- save the order
=> changing quantity was not possible before the patch

- syspref AcqCreateItem  : create items on receiving
- in a basket, create an order (click on "add" to add an item => quantity = 1)
- save the order
- reopen the order
- try to change the quantity (2 instead of 1), without clicking on "add" to create a new item => you cannot (alert message)
=> the behavior is the same as before the patch

M. Saby
Comment 10 Mathieu Saby 2013-04-27 15:37:41 UTC
wrong copy/paste...


Here is the test I made for signing off :
after applying the patch
- syspref AcqCreateItem  : create items on RECEIVING
- in a basket, create an order (quantity = 1)
- save the order
- reopen the order
- change the quantity (2 instead of 1)
- save the order
=> changing quantity was not possible before the patch

- syspref AcqCreateItem  : create items on CATALOGING
- in a basket, create an order (quantity = 1)
- save the order
- reopen the order
- change the quantity (2 instead of 1)
- save the order
=> changing quantity was not possible before the patch

- syspref AcqCreateItem  : create items on ORDERING
- in a basket, create an order (click on "add" to add an item => quantity = 1)
- save the order
- reopen the order
- try to change the quantity (2 instead of 1), without clicking on "add" to create a new item => you cannot (alert message)
=> the behavior is the same as before the patch

M. Saby
Comment 11 Katrin Fischer 2013-04-30 05:42:39 UTC Comment hidden (obsolete)
Comment 12 Katrin Fischer 2013-04-30 05:43:03 UTC
Created attachment 17832 [details] [review]
[PASSED QA] Bug 7228: can't add items in an order with Modify

We have a message if we want to add items and we can't add, substract only.
It's ok if we choose to create items on ordering, in this case koha can't add items, just substract
and in this case we have to delete manually the items(s) in the catalog.
But if via the syspref AcqCreateItem we choose to create items when receiving this limitation is not usefull
The patch just checks if the syspref AcqCreateItem is on 'ordering'
if not the message is not shown and we can add items

Signed-off-by: Mathieu Saby <mathieu.saby@univ-rennes2.fr>
Here is the test I made for signing off :
after applying the patch
- syspref AcqCreateItem  : create items on RECEIVING
- in a basket, create an order (quantity = 1)
- save the order
- reopen the order
- change the quantity (2 instead of 1)
- save the order
=> changing quantity was not possible before the patch

- syspref AcqCreateItem  : create items on CATALOGING
- in a basket, create an order (quantity = 1)
- save the order
- reopen the order
- change the quantity (2 instead of 1)
- save the order
=> changing quantity was not possible before the patch

- syspref AcqCreateItem  : create items on ORDERING
- in a basket, create an order (click on "add" to add an item => quantity = 1)
- save the order
- reopen the order
- try to change the quantity (2 instead of 1), without clicking on "add" to create a new item => you cannot (alert message)
=> the behavior is the same as before the patch

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Copied test plan from bug report.
Template only change deactivating the Javascript that blocks
you from changing the quantity when AcqCreateItem is set to
something else than 'ordering'.
Passes all tests and QA script.
Comment 13 Katrin Fischer 2013-04-30 05:45:29 UTC
The patch works and solves an existing problem, so I am passing QA on it.

What worries me a little is that there is no way to know if the AcqCreateItem setting was changed between creating the order initially and trying to modify it. Maybe a better way would be to check if there are actually items linked to this order to determine if you can add more or not.
Comment 14 Christophe Croullebois 2013-04-30 11:59:12 UTC
Hello Katrin,
you are right, my patch is just a quick way to solve only a part of the problem.
I haven't thought at the case someone change the syspref after creating a line.
Your idea about looking for items created in aqorders_items would be more safe.
But I think it would be better to add to this page the creation of each new item, the same job done when you create the order.
But it needs more time to do so.
Comment 15 Mathieu Saby 2013-04-30 12:04:25 UTC
Yes, maybe it could be improved later, but at present all libraries using AcqCreateItem = on receive and importing records from a vendor are very worried, so Christophe's patch will be a good thing for them.

Mathieu
Comment 16 Katrin Fischer 2013-04-30 12:28:55 UTC
Hi Christophe and Mathieu, 
no worries - I agree that this patch should go in as it is now.
It might be worthwile to refactor all this code sometime, but certainly not shortly before a major release :)
Comment 17 Jared Camins-Esakov 2013-05-01 12:22:55 UTC
This patch has been pushed to master and 3.12.x.
Comment 18 Chris Cormack 2013-05-09 09:02:17 UTC
Does not apply to 3.10.x
Comment 19 Christophe Croullebois 2013-05-14 08:28:40 UTC
Created attachment 18129 [details] [review]
patch rebased for 3.10.x
Comment 20 Christophe Croullebois 2013-05-14 08:33:13 UTC
Created attachment 18130 [details] [review]
patch rebased for 3.8.x

Hi Chris,
I have made two patches for 3.8 and 3.10.
I have added a line in the comment to distinguish each one
thx
Comment 21 Chris Cormack 2013-05-14 08:34:21 UTC
Thanks, will test and push them now
Comment 22 Chris Cormack 2013-05-14 08:44:16 UTC
Pushed to 3.10.x and 3.8.x will be in 3.10.6 and 3.8.13
Comment 23 Nicole C. Engard 2013-06-21 12:55:41 UTC
The problem I reported was the one thing that this patch doesn't fix:

- syspref AcqCreateItem  : create items on ORDERING
- in a basket, create an order (click on "add" to add an item => quantity = 1)
- save the order
- reopen the order
- try to change the quantity (2 instead of 1), without clicking on "add" to create a new item => you cannot (alert message)
=> the behavior is the same as before the patch


If I want to order from a staged file I should be able to add multiple copies even if I'm doing the item at the time of ordering. Otherwise what's the benefit?

Nicole
Comment 24 Katrin Fischer 2013-06-24 08:22:23 UTC
Hi Nicole, maybe you could open a new bug for this and link them? I know that this was your bug and the fix didn't fix your problem, but it might be less confusing. Bugs with lots of comments are hard to work with and we could state clearly that the next development will be for AcqCreateItem = on order.
Comment 25 Nicole C. Engard 2013-06-24 13:15:03 UTC
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=10501 created