Bug 7449 - billingplace and freedeliveryplace not saved when creating a new basketgroup
Summary: billingplace and freedeliveryplace not saved when creating a new basketgroup
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Target Milestone: ---
Assignee: Fridolin SOMERS
QA Contact: Katrin Fischer
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-01-16 20:49 UTC by Frédérick Capovilla
Modified: 2014-05-26 21:04 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Proposed patch (3.28 KB, patch)
2013-02-22 09:39 UTC, Fridolin SOMERS
Details | Diff | Splinter Review
Attachment to Bug 7449 - billingplace and freedeliveryplace not saved when creating a new basketgroup (3.38 KB, patch)
2013-03-09 00:36 UTC, mathieu saby
Details | Diff | Splinter Review
[PASSED QA] Bug 7449: billingplace and freedeliveryplace not saved when creating a new basketgroup (3.54 KB, patch)
2013-04-07 08:02 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Frédérick Capovilla 2012-01-16 20:49:17 UTC
When creating a new basketgroup, the billingplace isn't saved.

Because of that, the selected branch's informations aren't added to the pdf when printing the basketgroup.

If the basket if modified and saved again, the billingplace then gets saved correctly.

The problem seems to be around lines 463-472 of acqui/basketgroup.pl and in the NewBasketgroup subroutine : $billingplace isn't saved there.
Comment 1 Fridolin SOMERS 2013-02-22 09:39:36 UTC Comment hidden (obsolete)
Comment 2 Fridolin SOMERS 2013-02-22 09:40:33 UTC
I changed bug name because freedeliveryplace is also concerned.
Comment 3 Fridolin SOMERS 2013-02-22 09:43:26 UTC
Patch changes in C4/Acquisition.pm NewBasketgroup() so that it looks more like ModBasketgroup() witch is correct.
Comment 4 mathieu saby 2013-03-09 00:36:45 UTC Comment hidden (obsolete)
Comment 5 mathieu saby 2013-03-09 00:38:53 UTC
It works, I sign off.

M. Saby
Comment 6 Katrin Fischer 2013-03-20 11:01:24 UTC
Hi Fridolyn, 

this is certainly an improvement, but I think there is still a little problem with the setting for delivery place:

1) Created a new basket group, set delivery place to "-"
2) Added a delivery address in the text box.
3) Saved.
4) Edited basket group - delivery place is set to my current branch instead
   of "-"

Could you take a look please?
Comment 7 mathieu saby 2013-03-27 16:33:10 UTC
Hello
We consider it is a very strange behavior. When creating a basketgroup from a basket, billing and deliveryplace should be the same.

But I think this was the behavior for deliveryplace before Fridolyn's patch.
So maybe it can be changed in other patch?

Mathieu
Comment 8 Katrin Fischer 2013-03-27 16:35:37 UTC
Hm, I am not sure. But wouldn't it be possible that delivery could go to one branch while bills are handled centrally at the main library? The package would still have a list of delivered items I guess.
Comment 9 mathieu saby 2013-03-27 18:06:05 UTC
So let's say the current behavior is correct -)
Before changing it, we may ask the lists, and maybe made a syspref if there are different opinions on that question...

Mathieu
Comment 10 Katrin Fischer 2013-03-27 18:31:11 UTC
Hm, not sure I understand :) What would the syspref change?
Comment 11 Katrin Fischer 2013-03-27 18:31:34 UTC
Also, shouldn't the bug that you can't save the information correctly be fixed?
Comment 12 mathieu saby 2013-03-27 19:23:05 UTC
I was thinking of a syspref for Using either the librairian's connecting site, or the site used in basket.
But maybe not a good idea.
What is sure, is that 7449 is already an improvement. For my colleagues it will change life! (currently we have to reopen each basketgroup to change the billingplace...)

M. Saby
Comment 13 Fridolin SOMERS 2013-04-03 15:33:27 UTC
(In reply to comment #6)
> Hi Fridolyn, 
> 
> this is certainly an improvement, but I think there is still a little
> problem with the setting for delivery place:
> 
> 1) Created a new basket group, set delivery place to "-"
> 2) Added a delivery address in the text box.
> 3) Saved.
> 4) Edited basket group - delivery place is set to my current branch instead
>    of "-"
> 
> Could you take a look please?

This is another problem :
Creating a new basketgroup and editing one uses the same page and operation : acqui/basketgroup.pl?op=add&booksellerid=x&basketgroupid=x
So when you edit an existing basketgroup, selected branches are librarian branch.

Can you create a new bug ?

This patch does not change any behavior, it is only a correction in SQL query building.
Comment 14 Katrin Fischer 2013-04-07 07:45:20 UTC
Hi Fridolyn, I agree this can go on a separate bug. Taking another look now.
Comment 15 Katrin Fischer 2013-04-07 08:02:46 UTC
Created attachment 17230 [details] [review]
[PASSED QA] Bug 7449: billingplace and freedeliveryplace not saved when creating a new basketgroup

billingplace and freedeliveryplace are missing in C4::Acquisition::NewBasketgroup.

Test plan :
- Go to a vendor basket groups
- Create a new basket group
- Enter a name
- Choose a billing place
- Do not choose a delivery place in combobox but enter a text in delivery place textarea
- Enter a comment
- Save
- Edit created basket group
=> Check that billing place and free delivery place are ok

Signed-off-by: Mathieu Saby <mathieu.saby@univ-rennes2.fr>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works according to test plan, delivery place is now
correctly saved into the databas and was before lost.
All tests and QA script pass.
Comment 16 Jared Camins-Esakov 2013-04-07 17:08:41 UTC
This patch has been pushed to master and 3.12.x.
Comment 17 Chris Cormack 2013-04-14 07:39:42 UTC
Pushed to 3.10.x will be in 3.10.5