Bug 10402

Summary: Add multiple contacts for vendors
Product: Koha Reporter: Jared Camins-Esakov <jcamins>
Component: AcquisitionsAssignee: Jared Camins-Esakov <jcamins>
Status: CLOSED FIXED QA Contact: Jonathan Druart <jonathan.druart>
Severity: new feature    
Priority: P5 - low CC: bgkriegel, gitbot, h.meissner.82, jonathan.druart, katrin.fischer, kyle, laurblain, paola.rossi, tomascohen, ztajoli
Version: Main   
Hardware: All   
OS: All   
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=12851
Change sponsored?: Sponsored Patch complexity: Medium patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 12953    
Attachments: Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
[SIGNED-OFF] Bug 10402: Use an object for contacts
[SIGNED-OFF] Bug 10402: Move contacts to separate table
[SIGNED-OFF] Bug 10402: Add ability to enter multiple contacts
Bug 10402 [Template follow-up] Add multiple contacts for vendors
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402 [Template follow-up] Add multiple contacts for vendors
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402 [Template follow-up] Add multiple contacts for vendors
Bug 10402 follow-up: rebase onto latest master
Bug 10402 follow-up: fix perlcritic exception
Bug 10402 follow-up: choose contacts for serials and acq claims
Bug 10402 follow-up: choose contacts for serials and acq claims
Bug 10402 follow-up: choose contacts for serials and acq claims
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402 [Template follow-up] Add multiple contacts for vendors
Bug 10402 follow-up: rebase onto latest master
Bug 10402 follow-up: fix perlcritic exception
Bug 10402 follow-up: choose contacts for serials and acq claims
Bug 10402: correct number of tests
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402 [Template follow-up] Add multiple contacts for vendors
Bug 10402 follow-up: rebase onto latest master
Bug 10402 follow-up: fix perlcritic exception
Bug 10402 follow-up: choose contacts for serials and acq claims
Bug 10402: correct number of tests
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402 [Template follow-up] Add multiple contacts for vendors
Bug 10402 follow-up: choose contacts for claims
Bug 10402 follow-up: choose contacts for claims
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402 [Template follow-up] Add multiple contacts for vendors
Bug 10402 follow-up: choose contacts for claims
Bug 10402 follow-up: choose contacts for claims
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402 [Template follow-up] Add multiple contacts for vendors
Bug 10402 follow-up: choose contacts for claims
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402 [Template follow-up] Add multiple contacts for vendors
Bug 10402 follow-up: choose contacts for claims
Bug 10402: Use an object for contacts
Bug 10402: Move contacts to separate table
Bug 10402: Add ability to enter multiple contacts
Bug 10402 [Template follow-up] Add multiple contacts for vendors
Bug 10402 follow-up: choose contacts for claims
Bug 10402: FIX conflicts in Letters.t

Description Jared Camins-Esakov 2013-06-04 16:10:18 UTC
Some vendors may have more than one contact. For example, a technical contact and a billing contact, or a contact for journals and a contact for monographs. Rather than require that each contact be either made into a separate vendor or recorded somewhere outside of Koha, it would be really useful of Koha had the ability to add multiple additional contacts to vendors i the acquisitions module.
Comment 1 Jared Camins-Esakov 2013-06-14 01:50:09 UTC Comment hidden (obsolete)
Comment 2 Jared Camins-Esakov 2013-06-14 01:50:17 UTC Comment hidden (obsolete)
Comment 3 Jared Camins-Esakov 2013-06-14 01:50:35 UTC Comment hidden (obsolete)
Comment 4 Blain Laurence 2013-06-25 15:51:55 UTC
Sandbox setup by laurblain@aol.com with database 2 and bug 10402 on Tue Jun 25 17:48:05 2013

Something went wrong !
Applying: Bug 10402: Use an object for contacts
Applying: Bug 10402: Move contacts to separate table
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging installer/data/mysql/kohastructure.sql
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Failed to merge in the changes.
Patch failed at 0001 Bug 10402: Move contacts to separate table
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 10402 - Add multiple contacts for vendors

Bug 10402: Use an object for contacts
Apply? [yn]
Bug 10402: Move contacts to separate table
Apply? [yn]
Patch left in /tmp/Bug-10402-Move-contacts-to-separate-table-KsaGBE.patch

=== translation installation ===
Comment 5 Jared Camins-Esakov 2013-06-25 15:55:13 UTC Comment hidden (obsolete)
Comment 6 Jared Camins-Esakov 2013-06-25 15:55:24 UTC Comment hidden (obsolete)
Comment 7 Jared Camins-Esakov 2013-06-25 15:55:38 UTC Comment hidden (obsolete)
Comment 8 Paola Rossi 2013-07-12 14:05:03 UTC
During step 3 of Web Installer, I received this message:
Web installer › Step 3

The following error occurred while importing the database structure:

ERROR 1064 (42000) at line 2861: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'BOOLEAN not NULL default 0, booksellerid int(11) not NULL, PRIMARY KEY (id' at line 10

Please contact your system administrator
Comment 9 Jared Camins-Esakov 2013-07-12 15:22:59 UTC Comment hidden (obsolete)
Comment 10 Jared Camins-Esakov 2013-07-12 15:23:16 UTC Comment hidden (obsolete)
Comment 11 Owen Leonard 2013-07-16 16:23:57 UTC
Auto-merging C4/Bookseller.pm
CONFLICT (content): Merge conflict in C4/Bookseller.pm
Failed to merge in the changes.
Comment 12 Jared Camins-Esakov 2013-07-19 20:36:45 UTC Comment hidden (obsolete)
Comment 13 Jared Camins-Esakov 2013-07-19 20:36:56 UTC Comment hidden (obsolete)
Comment 14 Jared Camins-Esakov 2013-07-19 20:37:14 UTC Comment hidden (obsolete)
Comment 15 Jared Camins-Esakov 2013-07-31 19:52:57 UTC Comment hidden (obsolete)
Comment 16 Jared Camins-Esakov 2013-07-31 19:53:07 UTC Comment hidden (obsolete)
Comment 17 Jared Camins-Esakov 2013-07-31 19:53:14 UTC Comment hidden (obsolete)
Comment 18 Owen Leonard 2013-08-01 18:44:36 UTC Comment hidden (obsolete)
Comment 19 Owen Leonard 2013-08-01 18:44:52 UTC Comment hidden (obsolete)
Comment 20 Owen Leonard 2013-08-01 18:45:06 UTC Comment hidden (obsolete)
Comment 21 Owen Leonard 2013-08-01 18:45:12 UTC Comment hidden (obsolete)
Comment 22 Jared Camins-Esakov 2013-08-22 14:18:31 UTC Comment hidden (obsolete)
Comment 23 Jared Camins-Esakov 2013-08-22 14:18:40 UTC Comment hidden (obsolete)
Comment 24 Jared Camins-Esakov 2013-08-22 14:18:48 UTC Comment hidden (obsolete)
Comment 25 Jared Camins-Esakov 2013-08-22 14:19:33 UTC Comment hidden (obsolete)
Comment 26 Katrin Fischer 2013-09-12 05:50:22 UTC
Hi Jared, 
can you please rebase? Thx!
#	both modified:      C4/Bookseller.pm
#	both added:         t/db_dependent/Bookseller.t
Comment 27 Jared Camins-Esakov 2013-09-12 13:31:36 UTC Comment hidden (obsolete)
Comment 28 Jared Camins-Esakov 2013-09-12 13:31:46 UTC Comment hidden (obsolete)
Comment 29 Jared Camins-Esakov 2013-09-12 13:31:58 UTC Comment hidden (obsolete)
Comment 30 Jared Camins-Esakov 2013-09-12 13:32:10 UTC Comment hidden (obsolete)
Comment 31 Jared Camins-Esakov 2013-09-12 13:34:25 UTC Comment hidden (obsolete)
Comment 32 Kyle M Hall 2013-09-12 18:28:35 UTC
FAIL   acqui/supplier.pl
   OK     pod
   OK     forbidden patterns
   FAIL   valid
                Type of arg 1 to keys must be hash (not private variable)
                acqui/supplier.pl had compilation errors.
   OK     critic
Comment 33 Jared Camins-Esakov 2013-09-12 18:41:21 UTC Comment hidden (obsolete)
Comment 34 Katrin Fischer 2013-09-14 12:53:25 UTC
Hi Jared,

I think this is a great improvement and it works nicely in the UI, but there are 2 concerns:
- aqcontacts.rank is not used as far as I can see
- it seems to break use of the contact fields in serials and acquisition claim letters.

Could you please take a look?
Comment 35 Jared Camins-Esakov 2013-09-28 16:31:42 UTC Comment hidden (obsolete)
Comment 36 Jared Camins-Esakov 2013-09-28 17:44:31 UTC Comment hidden (obsolete)
Comment 37 I'm just a bot 2013-09-29 04:42:30 UTC
Patch applied cleanly, go forth and signoff
Comment 38 Bernardo Gonzalez Kriegel 2013-10-03 22:39:01 UTC
There is a problem in updatedatabase.pl
this creates the table, note selected columns (->)

    $dbh->do("CREATE TABLE aqcontacts (
        id int(11) NOT NULL auto_increment,
        name varchar(100) default NULL,
        position varchar(100) default NULL,
        phone varchar(100) default NULL,
        altphone varchar(100) default NULL,
        fax varchar(100) default NULL,
        email varchar(100) default NULL,
        notes mediumtext,
->        claimacquisition BOOLEAN NOT NULL DEFAULT 0,
->        claimissues BOOLEAN NOT NULL DEFAULT 0,
        booksellerid int(11) not NULL,
        PRIMARY KEY  (id),
        CONSTRAINT booksellerid_fk2 FOREIGN KEY (booksellerid)
            REFERENCES aqbooksellers (id) ON DELETE CASCADE ON UPDATE CASCADE
        ) ENGINE=InnoDB DEFAULT CHARSET=utf8 AUTO_INCREMENT=1;");

this try to insert into non-existent columns (->)

    $dbh->do("INSERT INTO aqcontacts (name, position, phone, altphone, fax,
            email, notes, booksellerid, -> acq_claim, -> serial_claim)

perl installer/data/mysql/updatedatabase.pl
DBD::mysql::db do failed: Unknown column 'acq_claim' in 'field list' at installer/data/mysql/updatedatabase.pl line 7176.
Comment 39 Jared Camins-Esakov 2013-10-04 00:24:05 UTC Comment hidden (obsolete)
Comment 40 I'm just a bot 2013-10-28 06:24:09 UTC
Applying: Bug 10402: Use an object for contacts
Using index info to reconstruct a base tree...
M	C4/Bookseller.pm
M	acqui/uncertainprice.pl
M	koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css
M	koha-tmpl/intranet-tmpl/prog/en/modules/acqui/supplier.tt
M	t/db_dependent/Bookseller.t
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Bookseller.t
CONFLICT (content): Merge conflict in t/db_dependent/Bookseller.t
Auto-merging koha-tmpl/intranet-tmpl/prog/en/modules/acqui/supplier.tt
Auto-merging koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css
Auto-merging acqui/uncertainprice.pl
CONFLICT (content): Merge conflict in acqui/uncertainprice.pl
Auto-merging C4/Bookseller.pm
Patch failed at 0001 Bug 10402: Use an object for contacts
The copy of the patch that failed is found in:
   /home/christopher/git/koha/.git/rebase-apply/patch
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Comment 41 Jared Camins-Esakov 2013-12-10 12:39:44 UTC Comment hidden (obsolete)
Comment 42 Jared Camins-Esakov 2013-12-10 12:39:56 UTC Comment hidden (obsolete)
Comment 43 Jared Camins-Esakov 2013-12-10 12:40:10 UTC Comment hidden (obsolete)
Comment 44 Jared Camins-Esakov 2013-12-10 12:40:21 UTC Comment hidden (obsolete)
Comment 45 Jared Camins-Esakov 2013-12-10 12:40:30 UTC Comment hidden (obsolete)
Comment 46 Jared Camins-Esakov 2013-12-10 12:40:41 UTC Comment hidden (obsolete)
Comment 47 Jared Camins-Esakov 2013-12-10 12:40:51 UTC Comment hidden (obsolete)
Comment 48 Jared Camins-Esakov 2013-12-10 12:41:10 UTC Comment hidden (obsolete)
Comment 49 Jared Camins-Esakov 2014-01-20 04:45:27 UTC Comment hidden (obsolete)
Comment 50 Jared Camins-Esakov 2014-01-20 04:45:37 UTC Comment hidden (obsolete)
Comment 51 Jared Camins-Esakov 2014-01-20 04:45:49 UTC Comment hidden (obsolete)
Comment 52 Jared Camins-Esakov 2014-01-20 04:46:02 UTC Comment hidden (obsolete)
Comment 53 Jared Camins-Esakov 2014-01-20 04:46:14 UTC Comment hidden (obsolete)
Comment 54 Jared Camins-Esakov 2014-01-20 04:46:24 UTC Comment hidden (obsolete)
Comment 55 Jared Camins-Esakov 2014-01-20 04:46:35 UTC Comment hidden (obsolete)
Comment 56 Jared Camins-Esakov 2014-01-20 04:46:57 UTC Comment hidden (obsolete)
Comment 57 Jonathan Druart 2014-01-24 16:29:31 UTC
It seems I cannot deactivate the 2 values claimacquisition and claimissues (on editing a supplier). The 2 checkboxes are always checked.
I don't see (in the code) where you check the aqcontacts.claimacquisition flag before sending a claim for late orders.
It seems only the first contact is notified, is it wanted?

Marked as Failed QA.
Comment 58 Jared Camins-Esakov 2014-06-25 15:30:46 UTC Comment hidden (obsolete)
Comment 59 Jared Camins-Esakov 2014-06-25 15:30:59 UTC Comment hidden (obsolete)
Comment 60 Jared Camins-Esakov 2014-06-25 15:31:12 UTC Comment hidden (obsolete)
Comment 61 Jared Camins-Esakov 2014-06-25 15:31:24 UTC Comment hidden (obsolete)
Comment 62 Jared Camins-Esakov 2014-06-25 15:31:40 UTC Comment hidden (obsolete)
Comment 63 Paola Rossi 2014-06-27 16:25:05 UTC
I've applied against master 3.1.7.00.008.

I've tried to update database, and an error occurred:

Update report :
    Upgrade to 3.17.00.009 done (Bug 10402: Move bookseller contacts to separate table)
Update errors :
    [Fri Jun 27 14:44:13 2014] updatedatabase.pl: DBD::mysql::db do failed: Unknown column 'acquisitionprimary' in 'field list' at /var/root-koha/bug-7162/installer/data/mysql/updatedatabase.pl line 8583.

So I've passed the patch to "Failed QA" status.
----------------------------------------------------------------------
After having passed acquisitionprimary to acqprimary, another error occured:

Update errors :
    [Fri Jun 27 15:07:19 2014] updatedatabase.pl: DBD::mysql::db do failed: Unknown column 'issuesprimary' in 'field list' at /var/root-koha/bug-7162/installer/data/mysql/updatedatabase.pl line 8583.

After having passed issuesprimary to acqprimary, updating the DB was OK. 

Going on, on editing a vendor an empty contact was always listed before the filled contacts. So "Save" could add a further contact, whilest "Add another contact" didn't seem to be needed.
I have not been able to delete one contact from a vendor with multiple contacts selecting "x Delete contact".
Comment 64 Paola Rossi 2014-07-04 09:48:45 UTC
Today I've tried against master 3.17.00.009

After I've replaced [in th 5-th patch to apply, line 274]:

"acquisitionprimary" with "acqprimary", 
"issuesprimary" with "serialsprimary", 

updating the DB was OK.

I've kept the "Failed QA" status.
Comment 65 Jared Camins-Esakov 2014-07-04 15:25:20 UTC
(In reply to Paola Rossi from comment #63)
> Going on, on editing a vendor an empty contact was always listed before the
> filled contacts. So "Save" could add a further contact, whilest "Add another
> contact" didn't seem to be needed.
> I have not been able to delete one contact from a vendor with multiple
> contacts selecting "x Delete contact".

I have a patch which fixes the update and the contact deleting. The empty contact is intentional- there should always be one contact visible when you open up the form, because otherwise you might forget to enter any contact information you might have.
Comment 66 Jared Camins-Esakov 2014-07-04 15:42:57 UTC Comment hidden (obsolete)
Comment 67 Paola Rossi 2014-07-08 09:16:23 UTC
I've tried to apply against master 3.17.00.009

Applying: Bug 10402 follow-up: choose contacts for claims
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Letters.t
CONFLICT (content): Merge conflict in t/db_dependent/Letters.t
Auto-merging t/db_dependent/Bookseller.t
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Failed to merge in the changes.

So I've passed the patch to "Failed QA" status.
Comment 68 Paola Rossi 2014-07-08 10:07:28 UTC
[I beg your pardon.]

I've tried to apply against master 3.17.00.009

Applying: Bug 10402 follow-up: choose contacts for claims
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging t/db_dependent/Letters.t
CONFLICT (content): Merge conflict in t/db_dependent/Letters.t
Auto-merging t/db_dependent/Bookseller.t
Auto-merging installer/data/mysql/updatedatabase.pl
CONFLICT (content): Merge conflict in installer/data/mysql/updatedatabase.pl
Failed to merge in the changes.

I've passed the patch to "Patch doesn't apply" status.
Comment 69 Jared Camins-Esakov 2014-07-10 12:47:37 UTC Comment hidden (obsolete)
Comment 70 Jared Camins-Esakov 2014-07-10 12:47:49 UTC Comment hidden (obsolete)
Comment 71 Jared Camins-Esakov 2014-07-10 12:48:00 UTC Comment hidden (obsolete)
Comment 72 Jared Camins-Esakov 2014-07-10 12:48:12 UTC Comment hidden (obsolete)
Comment 73 Jared Camins-Esakov 2014-07-10 12:48:29 UTC Comment hidden (obsolete)
Comment 74 Paola Rossi 2014-07-11 16:32:03 UTC
I've applied the patches against master 3.17.00.009

I've added a vendor.
I've added a contact [filling the "Notes" field].
I've saved the vendor.

1) I've edited the vendor. The "Notes" field of the contact is shown empty.

2) I've filled the "Email" field of the contact with such a string :
XXXXX
I've selected "Save". The system has given no error, instead of sending "Please enter a valid email address" error.

3) I've tried to send late orders. The system has shown:
"Email has been sent"
but I (I'm the vendor...) have received no mail.
On apache' log:
[Fri Jul 11 17:09:29] lateorders.pl at line 107: Bad or missing From address: ''

It seems SendAlerts line 366 has no mail in $userenv->{emailaddress}.
[The "Primary email", "Secondary email" and "Alternate address' email" fields of the patron/login-user are set well.]

4) I've tried to send serials claims. 
I've "sent a notification". 
The system has given me no feedback, the subscription has become in "Claimed" status, but I've received no mail.
On apache' log:
[Fri Jul 11 17:29:43] claims.pl at line 78: Bad or missing From address: ''
----------------------------------------------------------------------------
So I've passed the patch to "Failed QA" status.
---------------------------------------------------------------------------

NB. Soon after having saved the vendor "XX", I see:
Home > Acquisition > Search for vendor Y,
where Y is the id of the aqbookseller "XX".
IMO Y is instead of the vendor's name, which can be searched.
Comment 75 Jared Camins-Esakov 2014-07-16 15:55:51 UTC
(In reply to Paola Rossi from comment #74)
> 1) I've edited the vendor. The "Notes" field of the contact is shown empty.

Good catch!

Issues 2-4 and the breadcrumbs you mention are outside the scope of this development. The following are true of Koha going back to 3.8 at least (so far as I can tell):
2) you can save an invalid e-mail address
3-4) notices will fail if you don't have an e-mail address set up for your account
5) you will see the vendor ID number rather than the vendor name in breadcrumbs after saving a vendor.
Comment 76 Jared Camins-Esakov 2014-07-16 15:56:08 UTC Comment hidden (obsolete)
Comment 77 Paola Rossi 2014-07-28 14:49:51 UTC
Thanks, Jared.

I've tried to apply the patches against master 3.17.00.010.

Applying: Bug 10402: Use an object for contacts
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging koha-tmpl/intranet-tmpl/prog/en/css/staff-global.css
Auto-merging acqui/uncertainprice.pl
Auto-merging acqui/supplier.pl

So I pass the patch to "Patch doesn't apply" status.
Comment 78 Jared Camins-Esakov 2014-07-28 15:03:27 UTC Comment hidden (obsolete)
Comment 79 Jared Camins-Esakov 2014-07-28 15:03:41 UTC Comment hidden (obsolete)
Comment 80 Jared Camins-Esakov 2014-07-28 15:03:52 UTC Comment hidden (obsolete)
Comment 81 Jared Camins-Esakov 2014-07-28 15:04:03 UTC Comment hidden (obsolete)
Comment 82 Jared Camins-Esakov 2014-07-28 15:04:28 UTC Comment hidden (obsolete)
Comment 83 Paola Rossi 2014-07-31 09:07:51 UTC
Today I've tried to apply against 3.17.00.011

Applying: Bug 10402: Use an object for contacts
Using index info to reconstruct a base tree...
Falling back to patching base and 3-way merge...
Auto-merging acqui/supplier.pl

So I pass the patch to "Patch doesn't apply" status
Comment 84 Jared Camins-Esakov 2014-07-31 12:14:27 UTC
(In reply to Paola Rossi from comment #83)
> Today I've tried to apply against 3.17.00.011
> 
> Applying: Bug 10402: Use an object for contacts
> Using index info to reconstruct a base tree...
> Falling back to patching base and 3-way merge...
> Auto-merging acqui/supplier.pl
> 
> So I pass the patch to "Patch doesn't apply" status

This message doesn't mean that the patch doesn't apply. It's just telling you that there were other changes made to acqui/supplier.pl.
Comment 85 Paola Rossi 2014-08-01 15:45:40 UTC
Thanks, Jared.

I've applied against 3.17.00.011.

Everything's ok.

So I pass the patches to "Signed Off" status.
Comment 86 Paola Rossi 2014-08-01 15:48:37 UTC Comment hidden (obsolete)
Comment 87 Paola Rossi 2014-08-01 15:49:45 UTC Comment hidden (obsolete)
Comment 88 Paola Rossi 2014-08-01 15:50:23 UTC Comment hidden (obsolete)
Comment 89 Paola Rossi 2014-08-01 15:50:57 UTC Comment hidden (obsolete)
Comment 90 Paola Rossi 2014-08-01 15:51:39 UTC Comment hidden (obsolete)
Comment 91 Jonathan Druart 2014-08-22 13:37:37 UTC
Created attachment 31091 [details] [review]
Bug 10402: Use an object for contacts

In preparation for adding the ability to handle multiple contacts, this
patch moves booksellers' contacts into their own class,
C4::Bookseller::Contact.

To test:
1) Apply patch.
2) Run database update.
3) Edit a bookseller, making sure to add a contact.
4) View the bookseller's information, making sure the contact
   information is there.
5) Run the unit test:
    > prove t/db_dependent/Bookseller.t

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 92 Jonathan Druart 2014-08-22 13:37:43 UTC
Created attachment 31092 [details] [review]
Bug 10402: Move contacts to separate table

This patch normalizes the data structures used for bookseller
contacts.

To test:
1) Repeat tests described on previous patch.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 93 Jonathan Druart 2014-08-22 13:37:52 UTC
Created attachment 31093 [details] [review]
Bug 10402: Add ability to enter multiple contacts

Some vendors may have more than one contact. For example, a technical
contact and a billing contact, or a contact for journals and a contact
for monographs. Rather than require that each contact be either made
into a separate vendor or recorded somewhere outside of Koha, it would
be really useful of Koha had the ability to add multiple additional
contacts to vendors in the Acquisitions module.

To test:
1) Apply patch.
2) Edit a bookseller, making sure to add a contact.
3) View the bookseller's information, making sure the contact
   information is there.
4) Run the unit test:
    > prove t/db_dependent/Bookseller.t
5) Add multiple contacts to a vendor, see that they show up.
6) Delete one contact from a vendor with multiple contacts,
   see that the result is correct.
7) Sign off.

Note: This test plan can supersede that on the previous two patches,
as all functionality of the previous two patches is required by this
one.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 94 Jonathan Druart 2014-08-22 13:38:01 UTC
Created attachment 31094 [details] [review]
Bug 10402 [Template follow-up] Add multiple contacts for vendors

This template-only follow-up tweaks the button labels and restructures
the vendor view layout a little bit to make it more amenable to multiple
contacts:

- Add spaces after button icons
- Change "Add contact" to "Add another contact" in hopes of making it
  less ambiguous whether clicking it will submit the whole form.
- Eliminate duplicate headers on the vendor view page by making the
  contact name the subheading for each individual contact.

To test, view details for vendors with one or more contacts, and try
editing to add additional contacts. Everything should look good and work
correctly.

Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 95 Jonathan Druart 2014-08-22 13:38:11 UTC
Created attachment 31095 [details] [review]
Bug 10402 follow-up: choose contacts for claims

This patch makes it possible to choose a particular contact for
acquisitions and serials claims. To test:

1) Select a contact to use for claiming late orders and a contact
   to use for claiming late issues.
2) Send a claim for a late order and a claim for a late issue.
3) Note that the claims went out to the proper people.
4) Run the unit test with:
    > prove t/db_dependent/Letters.t
5) Sign off.

Note: the claim messages are recorded in the logs in the *Acquisitions*
module, not the Letters module as you might expect

This patch also fixes several perlcritic violations and centralizes
contact-related unit testing in Bookseller.t.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 96 Jonathan Druart 2014-08-22 13:38:18 UTC
Created attachment 31096 [details] [review]
Bug 10402: FIX conflicts in Letters.t

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Comment 97 Jonathan Druart 2014-08-22 13:39:03 UTC
QA comment:

not blocker: On a editing a supplier with 1 contact, the "Delete contact" button will remove the contact form. It is not blocker since you can click on the "add contact" to add another contact.

Still in master:
1/ The letter generated contains the <order></order> tags.
2/ The validation on claiming serials does not work (no warning/error/ok message on sending the emails). This should be done with bug 5342 (pending for 2 years...).

No regression found, all works as expected.

Marked as Passed QA.
Comment 98 Tomás Cohen Arazi 2014-08-26 17:30:31 UTC
Patches pushed to master.

Thanks Jared!

Note: some small template glitches I've seen vanished after I did ctrl+shift+r to reload CSS.
Comment 99 Jonathan Druart 2014-08-29 12:56:46 UTC
(In reply to Jonathan Druart from comment #97)
> 1/ The letter generated contains the <order></order> tags.

I am going to provide a patch on bug 12851.