Mark, you're the author of the patch, so I've assigned the bug to you.
Plus the updated patch is "application/octet-stream", don't you have forgotten to set "patch" to the attachment ?

(In reply to comment #4)
> Created attachment 8635 [details] [review]
> Replacing previous application/octet-stream patch
> 
> Replacing previous application/octet-stream patch

Thanks Mark,

note that you could have updated the initial patch:
click on "Details" on the right of the patch
click on "(edit details)" just after the patch title

you'll see a form where you can change patch informations (including obsoleting one without uploading another)

This says 3.6 does that mean it won't work in 3.8? I haven't tried to test or apply this yet, just wondering about the versioning.

(In reply to comment #6)
> This says 3.6 does that mean it won't work in 3.8? I haven't tried to test
> or apply this yet, just wondering about the versioning.

I hope it doesn't. All patches for ENH must be submitted against master. Changing version

I was able to install this patch on current master 3.09.00.004 (pulled right before trying)..  So the patch does work, I'm working on finding a librarian that needs EDI to do some testing for me.  Once I can get a full test done - I will sign off on this patch.

-Brendan

Hi Mark, 

thank you for providing those patches and the documentation! :)

I was reading through your patch trying to figure out how it all works and found some small issues:

For the new permission to be available on new installations, it has to be added to the sample sql files. There is a test that you can run to make sure you got all necessary files - xt/permissions.t.

The new tables have to be added to kohastructure.sql for the same reason.

Thanks Collin - 

Again no problem installing the code on master.  And everything works great for the examples that you gave.  I have since worked on creating an new edi_message to test with a US company Baker and Taylor.  It's currently only outgoing and I'm waiting on an example incoming from them.  Once I have that I will forward it along to include with this - and hopefully some patches too..

My comment.

I had to hard code in the Rebus/EDI.pm to be able to successfully create a outgoing .CEP (to a non-ptfs-europe example)...  Which I did and I've forward that onto the vendor.  I also notice that it's hardcoded for GBP.

So I'm wondering - are those changes that we should make to the code? - so it's not so "hard" coded.  Or is that part of the scope that you are developing for and we're going to see some more patches?  (Just want an idea on where I should send our development team) 

Anyways - code works.  I can sign off on this as is.  But I really want to have a discussion with some more developers, before I sign-off - since this would have to be hard coded for anyone to use it with there choice of vendor  - and it doesn't take advantage of information that is readily available through koha (currency, etc. or even the EAN/SAN accounts created in the database).  Hope that makes sense.

-Brendan

Errors on apply the patch:
error: patch failed: installer/data/mysql/sysprefs.sql:382
error: installer/data/mysql/sysprefs.sql: patch does not apply
error: patch failed: installer/data/mysql/updatedatabase.pl:5966
error: installer/data/mysql/updatedatabase.pl: patch does not apply

In fact the problem is trivial: those files (sysprefs.sql and updatedatabase.pl)
are changing quite quickly those days

(In reply to comment #14)
> Errors on apply the patch:
> error: patch failed: installer/data/mysql/sysprefs.sql:382
> error: installer/data/mysql/sysprefs.sql: patch does not apply
> error: patch failed: installer/data/mysql/updatedatabase.pl:5966
> error: installer/data/mysql/updatedatabase.pl: patch does not apply
> 
> In fact the problem is trivial: those files (sysprefs.sql and
> updatedatabase.pl)
> are changing quite quickly those days

If you use git am -3 you'll be able to resolve the conflict yourself.

Hi to all.
I can't sign-off the patch.
In kohastructure.sql and updatedatabase.pl I see:

CREATE TABLE IF NOT EXISTS edifact_messages (
  key int(11) NOT NULL auto_increment,
  message_type text NOT NULL,
  date_sent date default NULL,
  provider int(11) default NULL,
  status text,
  basketno int(11) NOT NULL default '0',
  PRIMARY KEY  (key)
) ENGINE=InnoDB DEFAULT CHARSET=utf8;

The string key is a reserved word in Mysql 5.5, http://dev.mysql.com/doc/refman/5.5/en/reserved-words.html

I don't know if it is better to use `key` or to use a different name

> I don't know if it is better to use `key` or to use a different name

Using a keyword is always as field name a bad practise, so I would say use a different name !

I find those problems in the patch:
a)It doesn't create the dir misc/edi_files
b)It doesn't create the file edi_quote_cron.pl (same function with a different name?)

I find the test file, ptfs-europe-koha-community.CEQ,  in an obsolete attach.

I don't have an EDI vendor, only a FTP site so I strictly depend on  the test "Instructions for setting up a test environment for the patch" to do tests

The test of this patch is failed.
The instructions say:
4. Add EANs to your local ordering branches
Go to Administration > Additional Parameters > EDI EANs

This operation is not possible because the .pl files
admin/edi_ean_accounts.pl
admin/edi_ean_edit.pl  
and the connect table 'edifact_ean' are not more present in the patch

The .pl files and the definiton of the table were present in the version of the patch dated 2011-06-11

It not clear if are wrong instructions or code.
If instructions are correct probably a merge of the two patch is possible.

The instructions accompanied the obsoleted patch which was of 3.4 vintage, the table files are mot present in aites using this in production

Ok, but can you rewrite the instructions ?
I can do the test but I don't know EDI, so I need instructions to do ti

Section 4 of Mark's original instructions is incorrect  the Edifact EAN for the library is now stored in the syspreference EDIfactEAN.

Dear Colin,

can you check the instruction for testing the patch now ?
I have delete step 4 but I have doubts about those lines:

"On the command line cd to misc/cronjobs and run 'perl edi_quote_cron.pl' (ordinarily this would be automated at intervals using the cron)"

Now the script edi_quote_cron.pl is not present but are 3 different script are avaible for EDI in misc/cronjobs.

Can you update the istructions about them ?

Hi Elliot,
 could check the instructions to test the patch ?
I will the the commit but I need instructions.
I don't know EDIfact

Planning on rewriting the testing plan for this.
I will also provide an FTP server for testing that portion of the code.
I should have this completed Monday March 18th.  And then we can test and sign-off that week in time for QA to review before the freeze.

Thank you,
Brendan

I confir that tomorrow i will start to test it

Test Plan - Koha EDIfact module - PTFS Europe (Updated by Brendan Gallagher - ByWater)

Setup and testing
-----

1. Allocate permissions to a staff user
Find a staff user and click the Details tab.
Select More > Set permissions
Expand the Tools options, then check '(edi_manage) Manage EDIFACT transmissions', then click the Set flags button
This user should now be able to see an EDIfact messages option in Tools > Additional Tools

2. Set default values for testing the included quote message
Add two test branches with the branchcodes 'BRANCH1' and 'BRANCH2'
Add two funds with the fundcodes 'FUND1' and 'FUND2'
Add three itemtypes with the codes 'BKREF', 'BK1WK', and 'BK4WK'
Add three authorised values in the LOC category - 'REFERENCE', 'STORE', and 'SHELVES'

3. Add the test bookseller and EDIfact account details
Create a new bookseller with the name 'PTFS-Europe'
Go to Administration > Additional Parameters > EDI Accounts
Add a new account:
        Vendor: PTFS-Europe
        Description: Test EDI FTP account
        Server hostname: <hostname of your FTP account> **ByWater is offering one for testing - 166.78.149.34
        Server username: <user with read and write permissions> **ByWater is offering one for testing - edifact2443
        Server password: <password of the above user> **ByWater is offering one for testing - order2443
        Server remote directory: <path to the remote directory holding your edi quotes and orders> **Bywater is offering one for testing - /home/edifact2443
        Vendor SAN/EAN: 5099876543210
Click save (these details can be edited or removed using the options in Administration > Additional Parameters > EDI Accounts)
Upload the supplied quotes file to the Server remote directory on your FTP server (supplied quotes file is in misc/edi_files/ptfs-europe-koha-community.CEQ)  ***(Brendan has uploaded that file to ByWater temp FTP and renamed it edifact_test_brendan.CEQ)

5. Import the quote file from the Vendor FTP account
Before running the import, ensure that the user running the web server has read/write access to misc/edi_files and the files in it (drw-rw-r-- koha www-data)
On the command line cd to misc/cronjobs and run 'perl edifact_order_ftp_transfer.pl ' (ordinarily this would be automated at intervals using the cron)
The remote file will be renamed following EDI convention with a .EEQ extension. You can also move the file to another directory (see later details)
The last activity date will be updated in the vendor EDI account
FTP transactions are logged in misc/edi_files/edi_ftp.logged
Any errors in processing the quote will be logged in edi_quote_error.log
Downloaded EDI messages are stored in misc/edi_files and also in the edi_messages MySQL table

6. View the EDI message
Go to Tools > EDIfact messages
Messages are listed in date order descending. You should have a QUOTES type message with a status of processed, and a link to the basket holding the order
Click View basket
Three items should be in the basket, linked to the correct funds, branches, etc. that you specified earlier
When the quote message is imported, existing bib records are checked for a match on ISBN. If a match is found, items are attached to that bib record. If no matches are found, a bib record is created using the detail from the quote message and items are attached to them.
The name of the created basket is the same as the originating quote message filename

7. Sending an order message back to the vendor
Click the EDIfact order button (this is only shown for vendors with an associated EDI account)
If you only have more than one branch with an EAN, you will be prompted to select an ordering branch. If only one branch has an EAN, this is used by default and this step is skipped.
The EDI order message is generated and delivered to the vendor FTP site. Filenames take the format 'ediorder_xxx.CEP' (where xxx is the basket number)
The edi_messages table is updated with an entry for your edi order message with a status of sent, transactions are logged to misc/edi_files/edi_ftp.log, and the last activity date is updated for the vendor edi account.

8. Sending an order message without a quote
Any vendor that has an edi account can accept order messages without a quote. When you have created a basket from scratch, follow the instructions in step 7 to send the order message.

Applying: Bug 7736: EDI Edifact quote and order functionality
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/de-DE/mandatory/userpermissions.sql
Auto-merging installer/data/mysql/en/mandatory/userpermissions.sql
Auto-merging installer/data/mysql/es-ES/mandatory/userpermissions.sql
Auto-merging installer/data/mysql/fr-FR/1-Obligatoire/userpermissions.sql
Auto-merging installer/data/mysql/it-IT/necessari/userpermissions.sql
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging installer/data/mysql/nb-NO/1-Obligatorisk/userpermissions.sql
Auto-merging installer/data/mysql/pl-PL/mandatory/userpermissions.sql
Auto-merging installer/data/mysql/ru-RU/mandatory/permissions_and_user_flags.sql
Auto-merging installer/data/mysql/sysprefs.sql
CONFLICT (content): Merge conflict in installer/data/mysql/sysprefs.sql
Auto-merging installer/data/mysql/uk-UA/mandatory/permissions_and_user_flags.sql
Auto-merging installer/data/mysql/updatedatabase.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 7736: EDI Edifact quote and order functionality
When you have resolved this problem run "git am --resolved".
If you would prefer to skip this patch, instead run "git am --skip".
To restore the original branch and stop patching run "git am --abort".
Bug 7736 - Edifact QUOTE and ORDER functionality

Bug 7736: EDI Edifact quote and order functionality
Apply? [yn] 
Patch left in /tmp/Bug-7736-EDI-Edifact-quote-and-order-functionality-VE0gZq.patch

=== translation installation ===
Posté le 01/01/2

Patch applied for me - but would not apply in the sandbox.

I rebase the patch on last master git tree

I found problems on sending order,
routine SendEDIOrder in C4/EDI.pm

I receive the error:

Could not transfer the file /home/test7736/kohaclone/misc/edi_files/ediorder_1.CEP to 166.78.149.34:

The problem is in the lines 594-596

if ( !$newerr ) {
 $newerr = 0;
 $ftp->put("${edi_files}ediorder_$basketno.CEP")
  or $newerr = 1;

The file ediorder_1.CEP is present in the dir /home/test7736/kohaclone/misc/edi_files

With a normal ftp client I don't have problems to upload files in the site 166.78.149.34.

Hi Zeno -

Thanks for comments.

I do see that you were successful in downloading the CEQ file from the FTP server and that should have created a basket which was step #5

I think maybe you are having trouble with step #7 - is that correct?

-Brendan

(In reply to comment #36)
> I found problems on sending order,
> routine SendEDIOrder in C4/EDI.pm
> 
> I receive the error:
> 
> Could not transfer the file
> /home/test7736/kohaclone/misc/edi_files/ediorder_1.CEP to 166.78.149.34:
> 
> The problem is in the lines 594-596
> 
> if ( !$newerr ) {
>  $newerr = 0;
>  $ftp->put("${edi_files}ediorder_$basketno.CEP")
>   or $newerr = 1;
> 
> The file ediorder_1.CEP is present in the dir
> /home/test7736/kohaclone/misc/edi_files
> 
> With a normal ftp client I don't have problems to upload files in the site
> 166.78.149.34.

Yes, the problem is on step #7.
I have also found the solution.

New line 578:
  my $ftp = Net::FTP->new( $ftpaccount->{host}, Passive=> 1, Timeout => 10 )
instead of
  my $ftp = Net::FTP->new( $ftpaccount->{host}, Timeout => 10 )

I don't know if we could insert "Passive=> 1" for every site or we need to change the edi ftp site definition with a new parameter 'Type of connection'.

I have found an two other little problems:

A)file acqui/basket.pl
New line 31
use C4::EDI qw( CreateEDIOrder SendEDIOrder  CheckVendorFTPAccountExists);
instead of 
use C4::EDI qw( CreateEDIOrder SendEDIOrder );

B)file /installer/data/mysql/kohastructure.sql
The integration of the new tables for edit start around line 3045.
But git doesn't do a perfet merge.
The defintion of the previous tables [borrower_modifications] is cutted.
So the file kohastructure.sql needs to be checked

The rebase of  Fridolyn SOMERS [comment 38] doesn't fix the errors that I describe in comment 39

ok looking at changes and rebasing.

Hi all, taking a quick look at this before breakfast and I feel like there is another big issue we have to fix before this can go in. Patches have been squashed resulting in the fact, that Mark Gavillet will not get any credit for all the lines of code he wrote here. I think as we can not split up the patches again, we probably need to reinstate Mark as author of the first patch.

Thanks for this Katrin - I really appreciate it.

I've been working on an improved version of the EDI code which allows for separate processing modules for booksellers, local customisation of order messages for individual customers, multiple SAN/EANs for different library branches, and other improvements.
I'm currently working on adding EDI invoice message processing which is progressing very well.
Obviously, I'd feel very reluctant to submit code in the future if I'm not going to be credited for any of the work.

(In reply to comment #45)
> Hi all, taking a quick look at this before breakfast and I feel like there
> is another big issue we have to fix before this can go in. Patches have been
> squashed resulting in the fact, that Mark Gavillet will not get any credit
> for all the lines of code he wrote here. I think as we can not split up the
> patches again, we probably need to reinstate Mark as author of the first
> patch.

(In reply to comment #46)
> Thanks for this Katrin - I really appreciate it.
> 
> I've been working on an improved version of the EDI code which allows for
> separate processing modules for booksellers, local customisation of order
> messages for individual customers, multiple SAN/EANs for different library
> branches, and other improvements.
> I'm currently working on adding EDI invoice message processing which is
> progressing very well.
> Obviously, I'd feel very reluctant to submit code in the future if I'm not
> going to be credited for any of the work.

Hi Mark, 

You probably need to have a chat with Colin, as it was he who merge yours, martin and his patches together

http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=12398

So maybe since you are all from the same organisation that is ok? If not you might need to work together to split them back out

(In reply to comment #47)
> (In reply to comment #46)
> > Thanks for this Katrin - I really appreciate it.
> > 
> > I've been working on an improved version of the EDI code which allows for
> > separate processing modules for booksellers, local customisation of order
> > messages for individual customers, multiple SAN/EANs for different library
> > branches, and other improvements.
> > I'm currently working on adding EDI invoice message processing which is
> > progressing very well.
> > Obviously, I'd feel very reluctant to submit code in the future if I'm not
> > going to be credited for any of the work.
> 
> Hi Mark, 
> 
> You probably need to have a chat with Colin, as it was he who merge yours,
> martin and his patches together
> 
> http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=12398
> 
> So maybe since you are all from the same organisation that is ok? If not you
> might need to work together to split them back out

Once those are split up, we can split out the little bits from Elliott and Zeno too. But those are small changes and easy to split out

Thx Chris, for taking a closer look at this. Quite hard to figure out what's going on here. So I guess if we fixed authorship for the big patch to whatever Mark/Colin/Martin agree it should be doable?

With the last follow-up I also sign off this enhacement.
In fact to install the patch you need 4 different files.
The points to do:
1)Install the big rebase of Brendan
2)Install the first follow-up of Brendan, http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16652
3)Install the second rebase of Brendan, http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16667
4)Install my follow-up, http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=16761

For QA:
to testi you need to have the dir misc/edi_files/ usable in read/write from the command line user of Koha [standard name: koha] and the apache user [standard name and group in debian: www-data].
A good idea is also to create the file misc/edi_files/edi_ftp_error.log and to give the red/write permissions on it.

So, usign the default users:

sudo chown koha:www-data edi_files
sudo chmod 770 edit_files
sudo touch edi_files/edi_ftp_error.log
sudo chown koha:www-data edi_files/edi_ftp_error.log
sudo chmod 770 edi_files/edi_ftp_error.log

I have received an email from Mark that the first patch should have the authorship:
Mark Gavillet [mark.gavillet@ptfs-europe.com]

Is this agreeable?

For me yes, no problem if I don't be present in the patch.
Clearly I speak only for myself (and CINECA).

I agree!  This is Agreeable!

(In reply to comment #54)
> I have received an email from Mark that the first patch should have the
> authorship:
> Mark Gavillet [mark.gavillet@ptfs-europe.com]
> 
> Is this agreeable?

Comment on attachment 16779 [details]
Instruction to test the patch and how-to use the feature



Perl dependencies
-----
Edifact::Interchange
Business::ISBN
Net::FTP
Net::FTP::File


(This should be at the top - Removed somewhere along the line)

>Test Plan - Koha EDIfact module - PTFS Europe 
>
>(Updated by Brendan Gallagher - ByWater)
>(with little extra frm Zeno tajoli
>
>Setup, testing and how-to
>-----
>
>1. Allocate permissions to a staff user
>Find a staff user and click the Details tab.
>Select More > Set permissions
>Expand the Tools options, then check '(edi_manage) Manage EDIFACT transmissions', 
>then click the Set flags button.
>This user should now be able to see an EDIfact messages option in Tools > Additional Tools
>
>2. Set default values for testing the included quote message
>Add two test branches with the branchcodes 'BRANCH1' and 'BRANCH2'
>Add two funds with the fundcodes 'FUND1' and 'FUND2'
>Add three itemtypes with the codes 'BKREF', 'BK1WK', and 'BK4WK'
>Add three authorised values in the LOC category - 'REFERENCE', 'STORE', and 'SHELVES'
>
>3. Add the test bookseller and EDIfact account details
>Create a new bookseller with the name 'PTFS-Europe'
>Go to Administration > Additional Parameters > EDI Accounts
>Add a new account:
>        Vendor: PTFS-Europe
>        Description: Test EDI FTP account
>        Server hostname: <hostname of your FTP account> 
>			**ByWater for testing - 166.78.149.34
>        Server username: <user with read and write permissions> 
>			**ByWater for testing - edifact2443
>        Server password: <password of the above user> **ByWater for testing - order2443
>        Server remote directory: <path to the remote directory holding edi quotes and orders> 
>			**Bywater is offering one for testing - /home/edifact2443
>        Vendor SAN/EAN: 5099876543210
>Click save (these details can be edited or removed using the options in Administration > 
>Additional Parameters > EDI Accounts)
>Upload the supplied quotes file to the Server remote directory on your FTP server
>(supplied quotes file is in misc/edi_files/ptfs-europe-koha-community.CEQ)  
>***(Brendan has uploaded that file to ByWater temp FTP and renamed it edifact_test_brendan.CEQ)
>
>
>4. Check the mode of FTP
>The test FTP site works in a 'passive' mode. Now (2013) passive mode is standard for FTP sites.
>Ask to your vendor if his site support FTP in passive mode.
> 
>
>5. Setup the  misc/edi_files dir
>Before running the import, ensure that the user running the web server has read/write 
>access to  misc/edi_files and the files in it (drwxrwx--- koha www-data)
>The defaults users are 'koha' [standar command line users] and www-data [standard debian 
>apache user].
>With those users you the instructions are:
>
>cd <your install patch>/misc
>sudo chown koha:www-data edi_files
>sudo chmod 770 edit_files
>sudo touch edi_files/edi_ftp_error.log
>sudo chown koha:www-data edi_files/edi_ftp_error.log
>sudo chmod 770 edi_files/edi_ftp_error.log
>
>It is important that you create an empty file misc/edi_files/edi_ftp_error.log and that
>koha and apache could togheter use it. The file misc/edi_files/edi_ftp_error.log will be
>used as log file for errors about EDI.
> 
>
>6. Import the quote file from the Vendor FTP account
>On the command line cd to misc/cronjobs and run 'perl edifact_order_ftp_transfer.pl' 
>ordinarily this would be automated at intervals using the cron)
>The remote file will be renamed following EDI convention with a .EEQ extension. You 
>can also move the file to another directory (see later details)
>The last activity date will be updated in the vendor EDI account
>FTP transactions are logged in misc/edi_files/edi_ftp.logged
>Any errors in processing the quote will be logged in edi_quote_error.log
>Downloaded EDI messages are stored in misc/edi_files and also in the 
>edi_messages MySQL table
>
>
>
>7. View the EDI message
>Go to Tools > EDIfact messages
>Messages are listed in date order descending. You should have a QUOTES
>type message with a status of processed, and a link to the basket
>holding the order
>
>Click View basket
>Three items should be in the basket, linked to the correct funds, branches,
>etc. that you specified earlier.
>When the quote message is imported, existing bib records are checked for a match
>on ISBN. If a match is found, items are attached to that bib record. If no
>matches are found, a bib record is created using the detail from the quote 
>message and items are attached to them. The name of the created basket is the
>same as the originating quote message filename
>
>
>
>8. Sending an order message back to the vendor
>Click the EDIfact order button (this is only shown for vendors with an associated
>EDI account). Now the EAN used for identify the library is the code in 
>the system prefernce 'EDIfactEAN'. So, for now, all libraries share the same EAN
>identifier
>
>The EDI order message is generated and delivered to the 
>vendor FTP site. Filenames take the format 'ediorder_xxx.CEP' 
>(where xxx is the basket number).
>The edi_messages table is updated with an entry for your edi order message with
>a status of sent, transactions are logged to misc/edi_files/edi_ftp.log, and the
>last activity date is updated for the vendor edi account.
>
>
>9. Sending an order message without a quote
>Any vendor that has an edi account can accept order messages without a quote. 
>When you have created a basket from scratch, follow the instructions in step 7 
>to send the order message.

QA comments:
 1- SQL reserved words must be in capital letters (see coding guidelines)
 2- MANY tabulation errors: 
* C4/EDI.pm                                                                FAIL
	forbidden patterns          FAIL
		The patch introduces a forbidden pattern: tabulation character (778) 
 3- other koha-qa.pl errors:

 * installer/data/mysql/updatedatabase.pl                                   FAIL

	valid                       FAIL
		Missing right curly or square bracket 
		syntax error 
		installer/data/mysql/updatedatabase.pl had compilation errors.
 * koha-tmpl/intranet-tmpl/prog/en/modules/admin/edi-edit.tt                FAIL
	tt_valid                    FAIL

(untested yet, maybe other comments once follow-up provided)

(In reply to comment #58)
> QA comments:
>  1- SQL reserved words must be in capital letters (see coding guidelines)
>  2- MANY tabulation errors: 
> * C4/EDI.pm                                                               
> FAIL
> 	forbidden patterns          FAIL
> 		The patch introduces a forbidden pattern: tabulation character (778) 
>  3- other koha-qa.pl errors:
> 
>  * installer/data/mysql/updatedatabase.pl                                  
> FAIL
> 
> 	valid                       FAIL
> 		Missing right curly or square bracket 
> 		syntax error 
> 		installer/data/mysql/updatedatabase.pl had compilation errors.
>  * koha-tmpl/intranet-tmpl/prog/en/modules/admin/edi-edit.tt               
> FAIL
> 	tt_valid                    FAIL
> 
> (untested yet, maybe other comments once follow-up provided)

Paul Are these all from the qa-test-tools git branch?  Or did you run something else to test?  Just curious - cause I can rerun those and fix these up in the next few months after the 3.12 release.  Thanks,
Brendan

(In reply to comment #60)
> Paul Are these all from the qa-test-tools git branch?
Yes they are

> Or did you run
> something else to test? 

nope, that's just the output from koha-qa.pl

(In reply to comment #61)
> (In reply to comment #60)
> > Paul Are these all from the qa-test-tools git branch?
> Yes they are
> 
> > Or did you run
> > something else to test? 
> 
> nope, that's just the output from koha-qa.pl

AH thank you very much Paul - that was what I meant - I guess I am still speaking and typing in broken English - product of traveling in France - but a good one...  
Thanks much,
Brendan

The field san in vendor_edi_accounts table is varchar(10).
In the template, field "Vendor SAN" is longer.
In the documentation the length of SAN/EAN is 13.
So I don't signed off.

I'm working on generating a usable patch set for people to test from this and some custom scripts. The edifact parsing is effectively done (for a very limited number of vendors). It needs to be made into something that fits cleanly with Koha.

In practice for those Koha sites using EDI in the UK the process is largely one of transferring incoming quotes and invoices and sending orders by ftp/sftp. (one gets the impression that the vendors edi side is not very automated).

The impact on Koha's work flow is minimal we need to store vendor edi account details (a vendor may have multiple edi 'accounts' ) and the corresponding library side accounts.

In the normal acq workflow the addition would be an option to send an order_basket by edi if the vendor is edi_enabled Otherwise there need to be batch scripts to handle transferring data from the vendor and loading that into Koha.

I dont know if people are envisioning other workflows, if so please post details against this bug.

Hope to post with tests and documentation as a working beast soon

Only a little remark:
To test the code we need a test sftp/ftp site.

Hello Colin - 

I was wondering on the status of this and if you all have had a chance to post some newer code up here soon?

Thanks much,
Brendan

(In reply to Colin Campbell from comment #85)
> I'm working on generating a usable patch set for people to test from this
> and some custom scripts. The edifact parsing is effectively done (for a very
> limited number of vendors). It needs to be made into something that fits
> cleanly with Koha.
> 
> In practice for those Koha sites using EDI in the UK the process is largely
> one of transferring incoming quotes and invoices and sending orders by
> ftp/sftp. (one gets the impression that the vendors edi side is not very
> automated).
> 
> The impact on Koha's work flow is minimal we need to store vendor edi
> account details (a vendor may have multiple edi 'accounts' ) and the
> corresponding library side accounts.
> 
> In the normal acq workflow the addition would be an option to send an
> order_basket by edi if the vendor is edi_enabled Otherwise there need to be
> batch scripts to handle transferring data from the vendor and loading that
> into Koha.
> 
> I dont know if people are envisioning other workflows, if so please post
> details against this bug.
> 
> Hope to post with tests and documentation as a working beast soon

Just a note that looking at the existing patches, there are a number of files there with no documentation at all. This makes maintaining it into the future difficult. Additionally, the functions that have documentation just have a one-liner. Nothing that says what arguments they take, or what results they're expected to produce, or what happens in failure cases.

It'd also be good to have a bit of an explanation of how EDI messages work in the parsing module for it.

(In reply to Brendan Gallagher from comment #87)
> Hello Colin - 
> 
> I was wondering on the status of this and if you all have had a chance to
> post some newer code up here soon?
> 
> Thanks much,
> Brendan
> 
> (In reply to Colin Campbell from comment #85)
> > I'm working on generating a usable patch set for people to test from this
> > and some custom scripts. The edifact parsing is effectively done (for a very
> > limited number of vendors). It needs to be made into something that fits
> > cleanly with Koha.
> > 
> > In practice for those Koha sites using EDI in the UK the process is largely
> > one of transferring incoming quotes and invoices and sending orders by
> > ftp/sftp. (one gets the impression that the vendors edi side is not very
> > automated).
> > 
> > The impact on Koha's work flow is minimal we need to store vendor edi
> > account details (a vendor may have multiple edi 'accounts' ) and the
> > corresponding library side accounts.
> > 
> > In the normal acq workflow the addition would be an option to send an
> > order_basket by edi if the vendor is edi_enabled Otherwise there need to be
> > batch scripts to handle transferring data from the vendor and loading that
> > into Koha.
> > 
> > I dont know if people are envisioning other workflows, if so please post
> > details against this bug.
> > 
> > Hope to post with tests and documentation as a working beast soon

Should be posting a patch in the next few days

(In reply to Robin Sheat from comment #88)
> Just a note that looking at the existing patches, there are a number of
> files there with no documentation at all. This makes maintaining it into the
> future difficult. Additionally, the functions that have documentation just
> have a one-liner. Nothing that says what arguments they take, or what
> results they're expected to produce, or what happens in failure cases.
> 
> It'd also be good to have a bit of an explanation of how EDI messages work
> in the parsing module for it.

Agreed. the rewitten routines should be more useable

Branch edi_master on git@github.com:colinsc/koha.git is the version I've deployed on our customers. I'll be creating a patch to submit against this bug but its available there for interested parties that may want to take a look

(In reply to Colin Campbell from comment #91)
> Branch edi_master on git@github.com:colinsc/koha.git is the version I've
> deployed on our customers. I'll be creating a patch to submit against this
> bug but its available there for interested parties that may want to take a
> look

I took a quick look. Thanks.  Once the patches are up here - I will do some more testing and hopefully a sign off.  Can't wait :)

Patch based on what we're running at a number of uk sites.

Couple of future developments which have been requested: handling credit notes in invoice records which requires some changes in invoice handling. And handling order status updates

Kyle updated the patch so it applies cleanly on master.  I am testing this today and should have a signoff if it follows the plans.  Testing going on ;)

Everything works great.

Questions for Colin or Martin - "EDIfact messages Manage EDIfact transmissions" That is found in the Tools section of koha.  But I think you've moved that functionality into ACQ, so I think that can just be ignored and removed now.  Answer that question for me and I'll sign-off.  I think that a QA follow-up of just removing that from the Koha Tools and we are golden.  

Great Job.  Thanks

(In reply to Brendan Gallagher from comment #98)
> Everything works great.
> 
> Questions for Colin or Martin - "EDIfact messages Manage EDIfact
> transmissions" That is found in the Tools section of koha.  But I think
> you've moved that functionality into ACQ, so I think that can just be
> ignored and removed now.  Answer that question for me and I'll sign-off.  I
> think that a QA follow-up of just removing that from the Koha Tools and we
> are golden.  
> 
> Great Job.  Thanks

Yes edifact message handling should all be under ACQ now

I setup acq to test this patch.
I use a 'FILE' edi account.

When I try to close the basket and create the request I see

Software error:
Can't call method "branchcode" on an undefined value at /home/koha/test_edi/Koha/Edifact/Order.pm line 374.

The link used:
http://192.168.216.128:8080/cgi-bin/koha/acqui/basket.pl?basketno=1&op=ediorder&ean=111222&booksellerid=1&confirm=1&basketgroupname=bask1

Generaly speaking I think we need more details on how to do test of this feature.
For example:
-- EAN/SAN, names for budgets, funds and vendors
-- files of different types
-- ect.

I run those tests, but I think it is not enough to sign-off:

prove Ediorder.t: OK
prove EdiInvoice.t: OK
prove Edifact.t: OK

(In reply to Zeno Tajoli from comment #102)
> I setup acq to test this patch.
> I use a 'FILE' edi account.
> 
> When I try to close the basket and create the request I see
> 
> Software error:
> Can't call method "branchcode" on an undefined value at
> /home/koha/test_edi/Koha/Edifact/Order.pm line 374.
> 
> The link used:
> http://192.168.216.128:8080/cgi-bin/koha/acqui/basket.
> pl?basketno=1&op=ediorder&ean=111222&booksellerid=1&confirm=1&basketgroupname
> =bask1
> 
> Generaly speaking I think we need more details on how to do test of this
> feature.
> For example:
> -- EAN/SAN, names for budgets, funds and vendors
> -- files of different types
> -- ect.
> 
> I run those tests, but I think it is not enough to sign-off:
> 
> prove Ediorder.t: OK
> prove EdiInvoice.t: OK
> prove Edifact.t: OK

Addeda supplementary patch handling derivation of branch from the basket

(In reply to Brendan Gallagher from comment #106)
> Created attachment 35717 [details] [review] [review]
> Bug 7904: (QA followup) fix tests
> 
> The package name for SIP wasn't fixed in the tests by the original patches.
> 
> This patch fixes it.
> 
> To test:
> - Run
>   $ prove t/db_dependent/SIP_ILS.t
> - Tests should pass with the patch.
> 
> Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
> 
> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>

Sorry grabbed the wrong patch (too overzealous)

> 
> Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
> Following test plan this works great.  Doesn't break ACQ.
> There are a few little spots QA can followup with but should 
> go through.  Thanks!

Can you let us know what the spots are? To help QA?

(In reply to Chris Cormack from comment #109)
> > 
> > Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
> > Following test plan this works great.  Doesn't break ACQ.
> > There are a few little spots QA can followup with but should 
> > go through.  Thanks!
> 
> Can you let us know what the spots are? To help QA?

Sure thing!

Running the QA script I see that is having some trouble with the license (that's an easy QA follow-up IMO).

If you spot this while signing off, please note and fail - it will save the time of a QA team member - and easy to sign off on the follow up then.

Hey Collin or Martin - 

Can you run these through the QA script and fix up the errors, then I'll be happy to mark it as signed off again.

Thanks,
Brendan

I'm really itching to get this fixed up and tested this week - while we have everyone hacking.  We can get some great testing done here and get you the feedback.  Really important for this week.

Switched status back to needs signoff - issues with old gplv2 licenses causing QA fail are fixed in the patch

Moved to status to In Disussion to hold back on testing - am reworking the cron job logic to ensure it deals with some fields being passed in some suppliers invoices and aware of some changes in Acq - also needs a rebase against current master. Will revise patch in a couple of days but worth holding back tests until then

Any update on this?  Now is the time to really get a move on this so we have enough months left to test and fix for getting into 3.22

Would it make sense to work out the remaining patches for the tax rewrite first before we tackle this? I think they will effect this.

(In reply to Katrin Fischer from comment #118)
> Would it make sense to work out the remaining patches for the tax rewrite
> first before we tackle this? I think they will effect this.

That's fine - I still want to see the code though and they are both going to take months of time to work on.  The important thing coming up is the KohaNA where we'd like to have librarians testing both (they are having hacking days).

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

I've put the latest code up on my github repo git@github.com:colinsc/koha.git as the branch edifact_snapshot_150911 If anyone wants to look at this. I'm hoping to get order responses working end to end then will generate a patch from this

I insert the patch rebasing this branch of Colin [https://github.com/colinsc/koha/tree/edifact_snapshot_150911] with origin/master of today (27/10/2015].

Patch 0001-Rebase-on-master: the main rebase on master done from Colin's work.

Patch 0002-Patch-to-update-DBIx: Update DBIX files from Colin's work

Patch 0003-Modification-in-Acquisition.t: Update of Acquisition.t with new DB structure.

Patch 0004-Fixed-the-definition-of-table-edifact_ean: My proposal to fix problem of defintion in edifact_ean table

Patch 0005-EdifactEan_DBIx-update: The update of DBIx file for table edifact_ean 

Patch 0006-Fix-differences-between-kohastructure.sql-and-edifact.sql:
There are same differences between kohastructure.sql and atomicupdate/edifact.sql
about the structure of edifact_ean. With this patch I insert also in kohastructure.sql the creation of the coloum  ee_id.

With all patch installed there are still problem on using edi.

I try to add the EAN/SAN to the library using 
  administration/acquisitions/EDI EANs 
[...:8080/cgi-bin/koha/admin/edi_ean_accounts.pl]

The result is:
Software error:

DBIx::Class::ResultSet::search(): No such relationship branch on EdifactEan at /production/develops/admin/edi_ean_accounts.pl line 93

Functionality question: is there an EDIFACT way for a vendor to cancel an order line (ie: the vendor cannot deliver an item that was ordered) ?

(In reply to Paul Poulain from comment #129)
> Functionality question: is there an EDIFACT way for a vendor to cancel an
> order line (ie: the vendor cannot deliver an item that was ordered) ?

Yes its a separate message an Order Response it can cancel an order also report on orders that cannot be immediately fulfilled (e.g. reprinting ) I have the code which reads them I just need to add the resulting updates to the koha files

Patch with consolidated modifications including ordrsps (and order cancellation as a result)
  Will update Zeno's patch to Acquisition.t

Hi all,

I know this isn't strictly a sign-off, but we wanted to note that we've been using these patches (thanks Colin) on our live - and rather large - system over the past year.

We're extremely happy with how the module operates - everything works correctly with no known issues.

Hi all
Sorry haven't done a formal sign off yet but completely agree with Sally's previous comment. 
Here at Staffordshire University we've been using EDI since 2011 and testing the new EDI patches during the recent updates to the module and can confirm that everything is working as planned with no known issues.
thanks.

Apply? [(y)es, (n)o, (i)nteractive] y
Applying: Bug 7736: Support Ordering via Edifact EDI messages
Using index info to reconstruct a base tree...
M	C4/Acquisition.pm
M	C4/Installer/PerlDependencies.pm
M	installer/data/mysql/kohastructure.sql
M	installer/data/mysql/sysprefs.sql
M	koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc
M	koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt
M	koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt
<stdin>:6102: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/tools/tools-home.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/admin/admin-home.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/basket.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/includes/admin-menu.inc
Auto-merging installer/data/mysql/sysprefs.sql
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging C4/Installer/PerlDependencies.pm
CONFLICT (content): Merge conflict in C4/Installer/PerlDependencies.pm
Auto-merging C4/Acquisition.pm
Failed to merge in the changes.
Patch failed at 0001 Bug 7736: Support Ordering via Edifact EDI messages
The copy of the patch that failed is found in:
   /home/vagrant/kohaclone/.git/rebase-apply/patch
When you have resolved this problem run "git bz apply --continue".
If you would prefer to skip this patch, instead run "git bz apply --skip".
To restore the original branch and stop patching run "git bz apply --abort".
Patch left in /tmp/Amended-patch-hYhFhR.patch

Sally and Paul, I put your sign offs on these patches. Thanks for the update!

(In reply to Nicole C. Engard from comment #141)
> Sally and Paul, I put your sign offs on these patches. Thanks for the update!

Excellent, I will see what I can do to qa this asap!

Created attachment 49037 [details] [review]
Bug 7736: Support Ordering via Edifact EDI messages

Add support for processing incoming Edifact Quotes, Invoices
and order responses and generating and transmission of
Edifact Orders.
Basic workflow is that an incoming quote generates an aquisition
basket in Koha, with each line corresponding to an order record

The user can then generate an edifact order from this (or another)
basket, which is transferred to the vendor's site

The supplier generates an invoice on despatch and this will
result in corresponding invoices being generated in Koha
The orderlines on the invoice are receipted automatically.

We also support order response messages. This may include
simple order acknowledgements, supplier reports/amendments
on availability. Cancellation messages cause the koha order
to be cancelled, other messages are recorded against the order

Which messages are to be supported/processed is specifiable on a
vendor by vendor basis via the admin screens

You can also specify auto order i.e. to generate orders from quotes
without user intervention - This reflects existing
workflows where most work is done on the suppliers website
then generating a dummy quote

Received messages are stored in the edifact_messages table
and the original can be viewed via the online

Database changes are in installer/data/mysql/atomicchanges/edifact.sql
Note new perl dependencies:
    Net::SFTP:Foreign
    Text::Unidecode

Signed-off-by: Paul Johnson <p.johnson@staffs.ac.uk>

Signed-off-by: Sally Healey <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 49038 [details] [review]
Bug 7736 Add Edifact order fields to Acquisition.t

Modification in Acquisition.t to support in test the new fields
of the table aqorders:
line_item_id, suppliers_reference_number,
suppliers_reference_qualifier, suppliers_report

Original patch by Zeno Tajoli
I amended to include the report field (ColinC)

Signed-off-by: Paul Johnson <p.johnson@staffs.ac.uk>

Signed-off-by: Sally Healey <sally.healey@cheshiresharedservices.gov.uk>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 49039 [details] [review]
Bug 7736 [QA Followup] - Fix collation

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 49040 [details] [review]
Bug 7736 [QA Followup] - Fix column headers, values and spelling

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 49041 [details] [review]
Bug 7736 [QA Followup] - Fix SQL, Add description field for Library EANs

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 49042 [details] [review]
Bug 7736 [QA Followup] - Fix spelling and pod errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Created attachment 49307 [details] [review]
patch to get logdir correctly

Change to call to C4::Context to use the syntax required since bug 9006 applied

Pushed to Master - Should be in the May 2016 release.  Thanks.

RM note - still need to run dbix schema !

Created attachment 49850 [details] [review]
Bug 7736: (QA followup) Add missing body id and class

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Brendan, please push the last followup so xt/tt_valid.t gets fixed.

(In reply to Tomás Cohen Arazi from comment #158)
> Brendan, please push the last followup so xt/tt_valid.t gets fixed.

Cool I'm on it!

Last patch Pushed to Master - Should be in the May 2016 release.

Thanks and good eye!

TestBuilder.t fails because of this patch:

t/db_dependent/TestBuilder.t .. 1/41 DBD::mysql::st execute failed: Table 'koha_empty.msg_invoice' doesn't exist [for Statement "SELECT COUNT( * ) FROM `msg_invoice` `me` WHERE ( `mi_id` IS NULL )"] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834.

What is this msg_invoice table??

(In reply to Jonathan Druart from comment #161)
> TestBuilder.t fails because of this patch:
> 
> t/db_dependent/TestBuilder.t .. 1/41 DBD::mysql::st execute failed: Table
> 'koha_empty.msg_invoice' doesn't exist [for Statement "SELECT COUNT( * )
> FROM `msg_invoice` `me` WHERE ( `mi_id` IS NULL )"] at
> /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834.
> 
> What is this msg_invoice table??

Looks like an obsolete table was merged in in error - posting patch to remove it

(In reply to Colin Campbell from comment #162)
> (In reply to Jonathan Druart from comment #161)
> > TestBuilder.t fails because of this patch:
> > 
> > t/db_dependent/TestBuilder.t .. 1/41 DBD::mysql::st execute failed: Table
> > 'koha_empty.msg_invoice' doesn't exist [for Statement "SELECT COUNT( * )
> > FROM `msg_invoice` `me` WHERE ( `mi_id` IS NULL )"] at
> > /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1834.
> > 
> > What is this msg_invoice table??
> 
> Looks like an obsolete table was merged in in error - posting patch to
> remove it

Thanks!

Created attachment 50008 [details] [review]
Bug 7736: (QA followup) Remove obsolete Schema module

msg_invoice is not used merged into patch in error

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

(In reply to Jesse Weaver from comment #165)
> Created attachment 50008 [details] [review] [review]
> Bug 7736: (QA followup) Remove obsolete Schema module
> 
> msg_invoice is not used merged into patch in error
> 
> Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

pushed

I have an error when trying to add a new EDI account:
Software error:

Can't call method "GetPlugins" on an undefined value at /home/katrin/kohaclone/admin/edi_accounts.pl line 56.

(In reply to Katrin Fischer from comment #167)
> I have an error when trying to add a new EDI account:
> Software error:
> 
> Can't call method "GetPlugins" on an undefined value at
> /home/katrin/kohaclone/admin/edi_accounts.pl line 56.

Same problem here.

 
> > Can't call method "GetPlugins" on an undefined value at
> > /home/katrin/kohaclone/admin/edi_accounts.pl line 56.
> 
> Same problem here.

Bug 16237 fixed it for me - it's PQA now.

This patchset introduces some uses of CGI->param in list context, please fix.

(In reply to Jonathan Druart from comment #170)
> This patchset introduces some uses of CGI->param in list context, please fix.

Any clue as to where?

(In reply to Colin Campbell from comment #171)
> (In reply to Jonathan Druart from comment #170)
> > This patchset introduces some uses of CGI->param in list context, please fix.
> 
> Any clue as to where?

  grep '=>.*->param' admin/edi_*

and see commit f3e4b5bbb6b02d7bb6e89d84c0379666eb1e704f
    Bug 16154: CGI->multi_param - Force scalar context
should give you some tricks.

Basically you need to force the scalar context if called in a list context (when passed to the template).

(In reply to Jonathan Druart from comment #172)
> (In reply to Colin Campbell from comment #171)
> > (In reply to Jonathan Druart from comment #170)
> > > This patchset introduces some uses of CGI->param in list context, please fix.
> > 
> > Any clue as to where?
> 
>   grep '=>.*->param' admin/edi_*
> 
> and see commit f3e4b5bbb6b02d7bb6e89d84c0379666eb1e704f
>     Bug 16154: CGI->multi_param - Force scalar context
> should give you some tricks.
> 
> Basically you need to force the scalar context if called in a list context
> (when passed to the template).

Exactly these are scalar context calls

(In reply to Colin Campbell from comment #173)
> (In reply to Jonathan Druart from comment #172)
> > (In reply to Colin Campbell from comment #171)
> > > (In reply to Jonathan Druart from comment #170)
> > > > This patchset introduces some uses of CGI->param in list context, please fix.
> > > 
> > > Any clue as to where?
> > 
> >   grep '=>.*->param' admin/edi_*
> > 
> > and see commit f3e4b5bbb6b02d7bb6e89d84c0379666eb1e704f
> >     Bug 16154: CGI->multi_param - Force scalar context
> > should give you some tricks.
> > 
> > Basically you need to force the scalar context if called in a list context
> > (when passed to the template).
> 
> Exactly these are scalar context calls

Please have a look at bug 16154 and bug 15809 if you want more information on this issue.

(In reply to Jonathan Druart from comment #174)
> (In reply to Colin Campbell from comment #173)
> > (In reply to Jonathan Druart from comment #172)
> > > (In reply to Colin Campbell from comment #171)
> > > > (In reply to Jonathan Druart from comment #170)
> > > > > This patchset introduces some uses of CGI->param in list context, please fix.
> > > > 
> > > > Any clue as to where?
> > > 
> > >   grep '=>.*->param' admin/edi_*
> > > 
> > > and see commit f3e4b5bbb6b02d7bb6e89d84c0379666eb1e704f
> > >     Bug 16154: CGI->multi_param - Force scalar context
> > > should give you some tricks.
> > > 
> > > Basically you need to force the scalar context if called in a list context
> > > (when passed to the template).
> > 
> > Exactly these are scalar context calls
> 
> Please have a look at bug 16154 and bug 15809 if you want more information
> on this issue.

Patch submitted on separate bug 16514

Under Plack, from the logs:
Variable "$query" is not available at /home/koha/src/acqui/basket.pl line 495.
Variable "$confirm_pref" is not available at /home/koha/src/acqui/basket.pl line 495.
Variable "$ean" is not available at /home/koha/src/acqui/basket.pl line 499.
Variable "$basket" is not available at /home/koha/src/acqui/basket.pl line 501.
Variable "$booksellerid" is not available at /home/koha/src/acqui/basket.pl line 521.
Variable "$template" is not available at /home/koha/src/acqui/basket.pl line 545.