Bug 7294 - status "in order"
Summary: status "in order"
Status: CLOSED WONTFIX
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 5336 8229
Blocks: 7163
  Show dependency treegraph
 
Reported: 2011-12-02 09:33 UTC by claire.hernandez@biblibre.com
Modified: 2014-12-07 20:02 UTC (History)
13 users (show)

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


Attachments
proposed patch (16.04 KB, patch)
2012-02-20 13:23 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Proposed patch (14.75 KB, patch)
2012-02-23 15:10 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (14.93 KB, patch)
2012-03-08 12:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
Proposed patch (15.08 KB, patch)
2012-03-20 12:52 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (16.17 KB, patch)
2012-04-05 13:47 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (16.22 KB, patch)
2012-04-05 14:13 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Additional patch to show the ordered status in catalogue page (11.30 KB, patch)
2012-06-08 10:12 UTC, shinoy m
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (16.62 KB, patch)
2012-06-10 14:46 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (17.31 KB, patch)
2012-07-10 14:00 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (17.51 KB, patch)
2012-08-17 13:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
modified: C4/Acquisition.pm (6.14 KB, patch)
2012-09-18 12:20 UTC, Mason James
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (18.29 KB, patch)
2012-09-18 12:20 UTC, Mason James
Details | Diff | Splinter Review
perltidy of badly indented ModReceiveOrder() sub (6.16 KB, patch)
2012-09-18 12:22 UTC, Mason James
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (18.51 KB, patch)
2012-09-19 09:42 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Rebased patch (19.34 KB, patch)
2012-12-13 13:56 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Transform $$ into $-> (19.40 KB, patch)
2012-12-13 14:04 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (21.06 KB, patch)
2013-01-29 15:37 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (21.23 KB, patch)
2013-03-11 10:54 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (21.29 KB, patch)
2013-03-18 14:15 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (20.81 KB, patch)
2013-03-20 09:59 UTC, Matthias Meusburger
Details | Diff | Splinter Review
Bug 7294: Adds acquisition informations in marc record (18.43 KB, patch)
2013-04-09 15:56 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description claire.hernandez@biblibre.com 2011-12-02 09:33:44 UTC
"View status 'in order' to the OPAC and staff interface: you can map specific fields from aqorders :
aqorders.branchcode
aqorders.quantity
aqorders.listprice
aqorders.uncertainprice
aqorders.rrp
aqorders.ecost
aqorders.notes
aqorders.supplierreference
aqorders.ordernumber 
with marc field so you can keep the information on what is currently on order at biblio level until you receive everything. Once all items have been reveived, the marc field is deleted."	 (BibLibre MT5834)
Comment 1 Matthias Meusburger 2012-02-20 13:23:55 UTC Comment hidden (obsolete)
Comment 2 Katrin Fischer 2012-02-20 13:38:23 UTC
Hi Matthias, 
your patch contains kohaversion.pl - can you perhaps remove the file and resubmit?
Thank you!
Comment 3 Matthias Meusburger 2012-02-23 15:10:03 UTC Comment hidden (obsolete)
Comment 4 Matthias Meusburger 2012-02-23 15:10:30 UTC
Sorry, it was a mistake. New patch without kohaversion.pl submitted.
Comment 5 Jonathan Druart 2012-03-08 12:05:12 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2012-03-12 15:42:07 UTC
Can we get a testing scenario for this? I create a 951 field with subfields that mapped to the aqorders fields that you listed, and created an on order item, but the marc fields do not seem to have been populated with the data.
Comment 7 Katrin Fischer 2012-03-12 15:45:47 UTC
Hm, only wondering - were you using the ACQ framework in your tests?
Comment 8 Kyle M Hall 2012-03-12 19:38:24 UTC
(In reply to comment #7)
> Hm, only wondering - were you using the ACQ framework in your tests?

No, I modified and was using the default framework.
Comment 9 jmbroust 2012-03-19 11:44:35 UTC
<h1>Something went wrong !</h1>Applying: Bug 7294: Adds acquisition
informations in marc record<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/kohastructure.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/>
Comment 10 Matthias Meusburger 2012-03-20 12:52:19 UTC Comment hidden (obsolete)
Comment 11 jmbroust 2012-03-20 15:37:31 UTC
tested for unimarc on biblibre sandbox 1 : tag mapped with aqorders.supplierreference is not shown in intranet (http://pro.test1.biblibre.com/cgi-bin/koha/catalogue/MARCdetail.pl?biblionumber=407). Is supplierreference = clientnumber declared when creating a vendor, can't test this in the sandbox ?
Tag mapped with aqorders.branchcode is also empty. It appears that this enhancement might be depending on other(s) enh to come.
works for other tags listed in descritpion.
Comment 12 jmbroust 2012-03-21 10:27:00 UTC
Needs enhancements : 
contact name (supplierreference) in supplier.pl
branchcode in neworderempty.pl
Comment 13 Matthias Meusburger 2012-04-05 13:47:57 UTC Comment hidden (obsolete)
Comment 14 Matthias Meusburger 2012-04-05 13:49:23 UTC
This new patch adds branchcode in neworderempty.pl (which was missing)
Supplierreference is automatically filled with bookseller's name.
Comment 15 Matthias Meusburger 2012-04-05 14:13:52 UTC Comment hidden (obsolete)
Comment 16 Matthias Meusburger 2012-04-05 14:14:58 UTC
This patch adds the ability to select an empty branchcode in neworderempty.pl
Comment 17 shinoy m 2012-04-11 07:14:30 UTC
I tried testing this patch, I am able to see acq info captured in field 951 of the MARC record. 

But I don't see anything in opac-results or opac-detail that shows that this item is on order.

Does this change take care of showing a 'on order' status on the OPAC and staff pages?
Comment 18 Matthias Meusburger 2012-04-18 09:58:32 UTC
This patch only adds ordering information in the marc record.

Whether the ordering information will be displayed in opac or admin interface depends on your configuration.

But there are no "special" modifications of opac-display and opac-detail for displaying the "on order" status, if that's what you were wondering.
Comment 19 shinoy m 2012-06-08 10:12:12 UTC Comment hidden (obsolete)
Comment 20 MJ Ray (software.coop) 2012-06-10 11:19:04 UTC
fatal: sha1 information is lacking or useless (acqui/addorder.pl).
Repository lacks necessary blobs to fall back on 3-way merge.
Cannot fall back to three-way merge.
Patch failed at 0001 Bug 7294:Additional patch to show the ordered status in catalogue page
Comment 21 Matthias Meusburger 2012-06-10 14:46:42 UTC Comment hidden (obsolete)
Comment 22 Matthias Meusburger 2012-06-10 15:32:48 UTC
Here's how to test:

 - Create the wanted koha to marc mappings. Only ordernumber is mandatory.
 - Place an order
 - Check that the mapped fields are reported in the record
 - Place another order
 - Modify one of the orders
 - Check that the modified order applies to the matching field of the record
 - Delete an order
 - Check that the order is deleted in the record
 - Delete the other order
 - Check that the order is deleted in the record and that the field to which it was mapped is also deleted (as there are no orders left).
Comment 23 Matthias Meusburger 2012-06-11 12:55:49 UTC
Hi shinoy m,

I created a new bug that I assigned to you for your additional patch : 

http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=8230

Having two separate smaller patches will make the sign-off and QA process much easier.

Hope it's ok for you.
Comment 24 Matthias Meusburger 2012-06-11 12:56:43 UTC
Comment on attachment 9980 [details] [review]
Additional patch to show the ordered status in catalogue page

Obsoleted this patch, as it is now in 8230.
Comment 25 Matthias Meusburger 2012-06-11 13:16:15 UTC
Blocked by 8229
Comment 26 Matthias Meusburger 2012-06-11 15:39:02 UTC
Works for me, but didn't work for mveron and alex_a when adding from new record or suggestion.
I should try with the default koha community database.
Comment 27 Matthias Meusburger 2012-07-10 14:00:01 UTC Comment hidden (obsolete)
Comment 28 Matthias Meusburger 2012-07-10 14:02:23 UTC
Indeed, the previous patch did not work correctly. This new patch fixes it.
Comment 29 Kyle M Hall 2012-07-16 12:50:02 UTC
I cannot seem to get the mapped fields to show up. I created an ACQ framework, then I mapped some aqorders fields to a few 951 fields. I then created some baskets and added items to the basket. When I viewed the MARC for the record, I did not see any data for the 951 fields I had previously mapped. Can you provide a test plan?
Comment 30 delaye 2012-08-10 08:46:18 UTC
I try to understand the usefulness of the patch. Want to display order information from the search? Ok but why the OPAC? How useful for the borrowers to know the price or the reference quantity of the order?
And then from the interface professional why delete this information after receipt?

Why not limit the display to the internal interface  and do not delete the information after the receipt? ....
Comment 31 Matthias Meusburger 2012-08-10 09:51:55 UTC
Kyle, are you sure you have mapped aqorders.ordernumber? This is mandatory.

Anyway, here's my test plan:

1) Create the mappings using the ACQ framework. Here's what I have used for testing, using 930:
a	 ordernumber	 Tab:9, | Koha field: aqorders.ordernumber, Not repeatable, Mandatory,	Edit	Delete
b	 listprice	 Tab:9, | Koha field: aqorders.listprice, Not repeatable, Not mandatory,	Edit	Delete
c	 branchcode	 Tab:9, | Koha field: aqorders.branchcode, Not repeatable, Not mandatory,	Edit	Delete
d	 quantity	 Tab:9, | Koha field: aqorders.quantity, Not repeatable, Not mandatory,	Edit	Delete
e	 test2	 Tab:9, Not repeatable, Not mandatory,

2) Select a basket

3) Add an order to basket (in my case: from an existing record)

4) Fill item informations (syspref AcqCreateItem's value is: Create an item when placing an order)

5) Add the item

6) Quantity is now at 1. Select a budget. Fill order informations. Save.

7) You're back at the basket details screen. Click on the newly ordered title. Click on "Marc". Click on "9". Here's what i've got:

930 ## - order
  a ordernumber 38
  c branchcode FPL
  d quantity 1
  b listprice 12.00
Comment 32 Nicole C. Engard 2012-08-16 23:39:13 UTC
Matthias,

How do I add the linking to the Koha fields? The only fields in the pull down are biblio and items and I don't see  away to add fields in the Koha to Marc Mapping so I'm not sure where to go to add aqorders.ordernumber etc. so that I can put them as the Koha link in the framework.

Nicole
Comment 33 Kyle M Hall 2012-08-17 13:35:15 UTC Comment hidden (obsolete)
Comment 34 Kyle M Hall 2012-08-17 13:35:55 UTC
Followed the test plan. Feature works as advertised.
Comment 35 Matthias Meusburger 2012-08-23 10:30:18 UTC
(En réponse au commentaire 32)
> Matthias,
> 
> How do I add the linking to the Koha fields? The only fields in the pull
> down are biblio and items and I don't see  away to add fields in the Koha to
> Marc Mapping so I'm not sure where to go to add aqorders.ordernumber etc. so
> that I can put them as the Koha link in the framework.
> 
> Nicole

Hi Nicole,

When applying this patch, you should have an "aqorders" value in the dropdown list in the "Koha to MARC mapping" page (koha2marclinks.pl). When selecting it and pressing ok, you should be able to do the mapping. It's not the case?

Another way to do it is to edit the ACQ framework MARC Structure (marctagstructure.pl?frameworkcode=ACQ), edit the 930 subfields, and proceed to the mapping from there (you should also have the aqorder.* fields listed in "Display More Constraints -> Koha link"). Does this work for you?
Comment 36 Nicole C. Engard 2012-08-23 13:05:42 UTC
I might have done something wrong when testing - Kyle has signed off and I trust his test :)
Comment 37 Mason James 2012-09-18 12:20:10 UTC Comment hidden (obsolete)
Comment 38 Mason James 2012-09-18 12:20:33 UTC Comment hidden (obsolete)
Comment 39 Mason James 2012-09-18 12:22:26 UTC Comment hidden (obsolete)
Comment 40 Mason James 2012-09-18 12:25:24 UTC
(In reply to comment #38)
> Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

QA comment - there is a small error in the neworderempty.tt template
causing ./xt/tt_valid.t to fail

failing QA on this


$ perl  ./xt/tt_valid.t 1..2
not ok 1 - TT syntax: not using TT directive within HTML tag
#   Failed test 'TT syntax: not using TT directive within HTML tag'
#   at ./xt/tt_valid.t line 55.
# Files list: 
# intranet-tmpl/prog/en/modules/acqui/neworderempty.tt: 341
ok 2 - Token received same as original put on stack
# Looks like you failed 1 test of 2.
Comment 41 Matthias Meusburger 2012-09-19 09:42:35 UTC Comment hidden (obsolete)
Comment 42 Matthias Meusburger 2012-09-19 09:43:15 UTC
This new patch fixes TT validation issue.
Comment 43 Paul Poulain 2012-10-09 12:52:11 UTC
QA comments:
 * this patch touches core features, I won't push it for 3.10 (Feature Freeze)
 * There are many $$x{Y} that should be written $x->{Y}
 * I think/fear that the aqorders.branchcode feature is a bad idea: how will it work when syspref AcqCreateItem=ordering ? How will you deal, when you don't have AcqCreadItem=ordering, with creation of items, will this value be used for defining the branch ? How will you deal with cases where you order 3 items for different branches ? The items table has some fields that are already related to acquisition: items.booksellerid, items.price, items.replacementprice.
 * I don't see why the "aqorders.uncertainprice" should be ported to MARC, but maybe there's a use-case I don't see

As a conclusion, what is requested from QA point of view:
 * split the patch in 2, to separate branchcode feature from the link2marc feature, that's 2 different things
 * fix $${}
 * argue how this feature will integrate smoothly with various acqcreateitem possibilities
Comment 44 Matthias Meusburger 2012-12-13 13:56:11 UTC Comment hidden (obsolete)
Comment 45 Matthias Meusburger 2012-12-13 14:04:15 UTC Comment hidden (obsolete)
Comment 46 Matthias Meusburger 2012-12-13 15:33:57 UTC
* There are many $$x{Y} that should be written $x->{Y} 

=> Done.


* I think/fear that the aqorders.branchcode feature is a bad idea: how will it work when syspref AcqCreateItem=ordering ? How will you deal, when you don't have AcqCreadItem=ordering, with creation of items, will this value be used for defining the branch ? How will you deal with cases where you order 3 items for different branches ? The items table has some fields that are already related to acquisition: items.booksellerid, items.price, items.replacementprice.

=> I don't see the problem here, because the branchcode that is used is the one in the aqorders table (added by this patch), so it doesn't matter which value has AcqCreateItem syspref. The branchcode is asked when creating a new order and that's it. Moreover, the values inserted in the MARC Record are here for information, they're not reused afterwards. 


* split the patch in 2, to separate branchcode feature from the link2marc feature, that's 2 different things

=> Same as before, branchcode is an aqorders field, just like the others. I don't see how it is different and why it should be handled separately.
Comment 47 Jonathan Druart 2012-12-20 16:01:22 UTC
testing 1 commit(s) (applied to commit 33e95ea)
 * adc69c0 Bug 7294: Adds acquisition informations in marc record
      C4/Acquisition.pm
      acqui/addorder.pl
      admin/koha2marclinks.pl
      admin/marc_subfields_structure.pl
      installer/data/mysql/kohastructure.sql
      installer/data/mysql/updatedatabase.pl
      koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt

* C4/Acquisition.pm                                                        FAIL
	pod                         FAIL
		*** WARNING: No items in =over  / =back list  in file C4/Acquisition.pm
	forbidden patterns          FAIL
		The patch introduces a forbidden pattern: tabulation character (1662)
		The patch introduces a forbidden pattern: tabulation character (1371)
		The patch introduces a forbidden pattern: tabulation character (1369)
		The patch introduces a forbidden pattern: tabulation character (1667)
		The patch introduces a forbidden pattern: tabulation character (1381)
		The patch introduces a forbidden pattern: tabulation character (1665)
		The patch introduces a forbidden pattern: tabulation character (1661)
		The patch introduces a forbidden pattern: tabulation character (1370)
		The patch introduces a forbidden pattern: tabulation character (1660)
		The patch introduces a forbidden pattern: tabulation character (1385)
		The patch introduces a forbidden pattern: tabulation character (1383)
		The patch introduces a forbidden pattern: tabulation character (1658)
		The patch introduces a forbidden pattern: tabulation character (1363)
		The patch introduces a forbidden pattern: tabulation character (1382)
		The patch introduces a forbidden pattern: tabulation character (1377)
		The patch introduces a forbidden pattern: tabulation character (1668)
		The patch introduces a forbidden pattern: tabulation character (1402)
		The patch introduces a forbidden pattern: tabulation character (1366)
		The patch introduces a forbidden pattern: tabulation character (1364)
		The patch introduces a forbidden pattern: tabulation character (1664)
		The patch introduces a forbidden pattern: tabulation character (1367)
		The patch introduces a forbidden pattern: withespace character  (1389)
		The patch introduces a forbidden pattern: tabulation character (1659)
		The patch introduces a forbidden pattern: withespace character  (1667)
		The patch introduces a forbidden pattern: tabulation character (1374)
		The patch introduces a forbidden pattern: tabulation character (1389)
		The patch introduces a forbidden pattern: tabulation character (1373)
		The patch introduces a forbidden pattern: tabulation character (1657)
		The patch introduces a forbidden pattern: tabulation character (1372)
		The patch introduces a forbidden pattern: tabulation character (1386)
		The patch introduces a forbidden pattern: tabulation character (1403)
		The patch introduces a forbidden pattern: tabulation character (1666)
		The patch introduces a forbidden pattern: tabulation character (1379)
		The patch introduces a forbidden pattern: tabulation character (1375)
		The patch introduces a forbidden pattern: tabulation character (1390)
		The patch introduces a forbidden pattern: tabulation character (1384)
		The patch introduces a forbidden pattern: tabulation character (1380)
		The patch introduces a forbidden pattern: tabulation character (1376)
		The patch introduces a forbidden pattern: tabulation character (1391)
		The patch introduces a forbidden pattern: tabulation character (1663)
		The patch introduces a forbidden pattern: tabulation character (1388)
		The patch introduces a forbidden pattern: withespace character  (1663)
		The patch introduces a forbidden pattern: withespace character  (1377)
		The patch introduces a forbidden pattern: tabulation character (1368)
	valid                       OK
	critic                      OK
* acqui/addorder.pl                                                        FAIL
	pod                         OK
	forbidden patterns          FAIL
		The patch introduces a forbidden pattern: tabulation character (239)
		The patch introduces a forbidden pattern: tabulation character (229)
		The patch introduces a forbidden pattern: tabulation character (236)
		The patch introduces a forbidden pattern: withespace character  (197)
		The patch introduces a forbidden pattern: tabulation character (235)
		The patch introduces a forbidden pattern: tabulation character (202)
		The patch introduces a forbidden pattern: withespace character  (210)
		The patch introduces a forbidden pattern: tabulation character (242)
		The patch introduces a forbidden pattern: withespace character  (175)
		The patch introduces a forbidden pattern: tabulation character (240)
		The patch introduces a forbidden pattern: tabulation character (224)
		The patch introduces a forbidden pattern: tabulation character (249)
		The patch introduces a forbidden pattern: tabulation character (208)
		The patch introduces a forbidden pattern: withespace character  (232)
		The patch introduces a forbidden pattern: tabulation character (244)
		The patch introduces a forbidden pattern: tabulation character (241)
		The patch introduces a forbidden pattern: tabulation character (237)
		The patch introduces a forbidden pattern: tabulation character (232)
		The patch introduces a forbidden pattern: tabulation character (203)
		The patch introduces a forbidden pattern: tabulation character (243)
		The patch introduces a forbidden pattern: tabulation character (245)
		The patch introduces a forbidden pattern: tabulation character (233)
		The patch introduces a forbidden pattern: tabulation character (247)
		The patch introduces a forbidden pattern: tabulation character (246)
		The patch introduces a forbidden pattern: tabulation character (194)
		The patch introduces a forbidden pattern: tabulation character (230)
	valid                       OK
	critic                      OK
* admin/koha2marclinks.pl                                                  OK
	pod                         OK
	forbidden patterns          OK
	valid                       OK
	critic                      OK
* admin/marc_subfields_structure.pl                                        OK
	pod                         OK
	forbidden patterns          OK
	valid                       OK
	critic                      OK
* installer/data/mysql/updatedatabase.pl                                   FAIL
	pod                         FAIL
		*** ERROR: =item without previous =over  in file installer/data/mysql/updatedatabase.pl
	forbidden patterns          FAIL
		The patch introduces a forbidden pattern: merge marker (=======) (6309)
	valid                       OK
	critic                      OK
* koha-tmpl/intranet-tmpl/prog/en/modules/acqui/neworderempty.tt           OK
	forbidden patterns          OK
	tt_valid                    OK
Comment 48 Kyle M Hall 2013-01-28 15:35:15 UTC
Ping! Matthias, are you going to write a followup for this? It looks like it only failed QA due to new tab characters.
Comment 49 Matthias Meusburger 2013-01-29 15:37:53 UTC Comment hidden (obsolete)
Comment 50 Matthias Meusburger 2013-01-29 15:38:28 UTC
Tabs and pods are now fixed.
Comment 51 Jonathan Druart 2013-03-11 10:54:16 UTC Comment hidden (obsolete)
Comment 52 Jonathan Druart 2013-03-11 10:54:58 UTC
Last patch fixes an issues in updatedatabase.pl
Comment 53 Jonathan Druart 2013-03-18 14:15:59 UTC Comment hidden (obsolete)
Comment 54 Katrin Fischer 2013-03-19 17:29:54 UTC
Taking a look at this now.
Comment 55 Katrin Fischer 2013-03-20 09:14:14 UTC
Hi Matthias,

let's get started with QA on that feature :)

1) The QA script does not pass, becasue of Perlcritic and a POD error, please fix.

2) I think you accidentally deleted a line from an earlier update in updatedatabase: 

+++ b/installer/data/mysql/updatedatabase.pl
@@ -5806,7 +5806,6 @@ if ( C4::Context->preference("Version") < TransformToNum($DBversion) ) {
     $dbh->do("INSERT INTO systempreferences (variable,value,explanation,options,type) VALUES('AllowReturnToBranch', '$prefvalue', 'Where an item may be returned', 'anywhere|homebranch|holdingbranch|homeorholdingbranch', 'Choice');");
 
     print "Upgrade to $DBversion done: adding AllowReturnToBranch syspref (bug 6151)";
-    SetVersion($DBversion);
 }

3) Can you explain how the addition of branch to the aquisition detail works?
Right now one library can easily order items for multiple branches, will this still be possible?
Comment 56 Katrin Fischer 2013-03-20 09:24:11 UTC
4) QA script also complains about tabs in neworderempty.tt 

Please fix and switch back to signed off!
Comment 57 Matthias Meusburger 2013-03-20 09:59:24 UTC
Created attachment 16464 [details] [review]
Bug 7294: Adds acquisition informations in marc record

"View status 'in order' to the OPAC and staff interface. Specific
fields from aqorders can be mapped to marc fields (using ACQ bibliographic
framework):
aqorders.branchcode
aqorders.quantity
aqorders.listprice
aqorders.uncertainprice
aqorders.rrp
aqorders.ecost
aqorders.notes
aqorders.supplierreference
aqorders.ordernumber

This way, you can keep track on what is currently on order
at biblio level until you receive everything. Once all items have been
received, the marc field is deleted.

Please note that mapping the ordernumber is mandatory for this feature to work

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 58 Matthias Meusburger 2013-03-20 10:07:40 UTC
1, 2, 4 : All green now :)

As for the 3), even if the branchcode has been added to the aqorders table, it is possible to leave it blank, so acquisitions for multiple branches is still possible.
Comment 59 Katrin Fischer 2013-03-20 14:17:44 UTC
I found a problem, my test plan is:

1) Create a new order
2) Create a new order line (from empty record) with 2 items (AcqCreateItem = on order)
3) Close basket
3) Receive only 1 of the items

Perl error message shown:
Mandatory parameter quantity missing at /home/katrin/kohaclone/acqui/finishreceive.pl line 88

Patch has been applied and I am using a newly created ACQ framework based on Default (MARC21). No other confiuration/mapping has been done yet.

Error only occurs for partial receives.
Comment 60 Katrin Fischer 2013-03-22 05:58:40 UTC
Matthias, did you find the problem yet?
Comment 61 Matthias Meusburger 2013-03-22 10:13:39 UTC
Yes, I did. But I also did some more testing, and it doesn't behave correctly when canceling an order, so I have fix that.
Comment 62 Katrin Fischer 2013-03-22 10:23:30 UTC
Ah - good testing!
Comment 63 mathieu saby 2013-04-06 09:55:38 UTC
I have just realized the aim of this patch is partially the same as BZ9780 I made, and which is waiting for QA.
But in my patch, in order status is not display in tabs, but under the record.
And my patch also prevent librarians to suppress a record if it is used in active order.

Maybe the logic of BZ9780 and BZ7294 are not compatible : I did not put order information into MARC record, but create a new function in C4/Acquisition to check if a record is used by an order.

So, is it really necessary to put order information into MARC record? 

M. Saby
Comment 64 Katrin Fischer 2013-04-06 10:13:00 UTC
I am not sure every library will want to have the information in the record, so maybe we need both approaches?
Comment 65 mathieu saby 2013-04-06 12:21:50 UTC
My patch was only make to display the number of orders using a bib record, but the function I created (Acquisition::GetOrdersByBiblionumber) return all fields from   aqorders and aqbudgets, so I suppose it could be used for displaying more information.
If the link between biblio and aqorders tables is kept (and that s the aim of my pacth, by preventing the suppression of the record...), I don't see the interest of putting order information within MARC.

M. Saby
Comment 66 Jonathan Druart 2013-04-09 15:56:44 UTC
Created attachment 17313 [details] [review]
Bug 7294: Adds acquisition informations in marc record
Comment 67 mathieu saby 2013-04-14 11:56:09 UTC
Hello Jonathan
Could you take a look at my last comment under BZ 9780 and give me your opinion please?

Mathieu
Comment 68 mathieu saby 2013-04-23 09:44:55 UTC
Somebody create a new bug for displaying orders under records (that is the 4th!) : BZ 10013

And we made a local dev in jquery in my library. I joined a screenshot under 10013.

M. Saby
Comment 69 Katrin Fischer 2013-07-03 20:57:31 UTC
I have given this feature some more thought and I worry a bit about the consequences of storing acquisition related information in the MARC record.

Libraries might easily overwrite the information in the record using z39.50 or importing and matching records. This would make this feature probably not usable for our libraries, as the records are updated automatically with updated records from the union catalog.

Also there might be information you don't want the patron to see, so the fields would have to be excluded from the MARC views and MARC export options.

Overall I'd like this to be strictly optional and not tied to important functionality. I think the new patch from Jonathan for 5336 is no longer dependenton this feature?
Comment 70 Jonathan Druart 2013-07-04 07:26:35 UTC
(In reply to Katrin Fischer from comment #69)
> I have given this feature some more thought and I worry a bit about the
> consequences of storing acquisition related information in the MARC record.
> 
> Libraries might easily overwrite the information in the record using z39.50
> or importing and matching records. This would make this feature probably not
> usable for our libraries, as the records are updated automatically with
> updated records from the union catalog.
> 
> Also there might be information you don't want the patron to see, so the
> fields would have to be excluded from the MARC views and MARC export options.

We will certainly drop this feature.

> Overall I'd like this to be strictly optional and not tied to important
> functionality. I think the new patch from Jonathan for 5336 is no longer
> dependenton this feature?

No, it is the opposite, this one is dependent on bug 5336.
Comment 71 Katrin Fischer 2013-07-04 07:52:07 UTC
If the feature will be dropped, should this be closed?
Comment 72 Jonathan Druart 2013-07-04 07:58:42 UTC
(In reply to Katrin Fischer from comment #71)
> If the feature will be dropped, should this be closed?

Not sure... :)
Comment 73 Jonathan Druart 2013-10-04 14:16:40 UTC
The status of orders is managed by bug 5336.
We don't want order status information in the marc record.