Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2 fields
Summary: Allow extended patron attributes to be sent in arbitrary SIP2 fields
Status: Failed QA
Alias: None
Product: Koha
Classification: Unclassified
Component: SIP2 (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on: 17792
Blocks:
  Show dependency treegraph
 
Reported: 2016-12-29 14:36 UTC by Kyle M Hall
Modified: 2017-10-27 14:30 UTC (History)
4 users (show)

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


Attachments
Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2 fields (4.13 KB, patch)
2016-12-29 15:54 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2 fields (4.05 KB, patch)
2016-12-29 16:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2 fields (4.17 KB, patch)
2017-02-09 12:04 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2 fields (6.63 KB, patch)
2017-10-26 15:14 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2 fields (6.80 KB, patch)
2017-10-27 06:31 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall 2016-12-29 14:36:52 UTC
Some libraries need to be able to send additional patron data from the extended patron attributes in made up SIP2 fields for the patron information and patron status responses.
Comment 1 Kyle M Hall 2016-12-29 15:54:12 UTC
Created attachment 58499 [details] [review]
Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2 fields

Some libraries need to be able to send additional patron data from the
extended patron attributes in made up SIP2 fields for the patron
information and patron status responses.

Test Plan:
1) Apply this patch
2) Create 3 new patron attributes with the codes CODE1, CODE2, CODE3.
   Make a least one repeatable.
3) Create a patron, add those attibutes for the patron, make sure there
   are at least two instances of the repeatable code
4) Edit your SIP2 config file, add the following within the login stanza:
   <patron_attribute field="XX" code="CODE1" />
   <patron_attribute field="XY" code="CODE2" />
   <patron_attribute field="XZ" code="CODE3" />
5) Using the sip cli emulator, run patron_information and
   patron_status_request messages for the patron
6) Note the values you set for the patron attributes are sent in the
   corrosponding fields!
Comment 2 Kyle M Hall 2016-12-29 16:00:42 UTC
Created attachment 58500 [details] [review]
Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2 fields

Some libraries need to be able to send additional patron data from the
extended patron attributes in made up SIP2 fields for the patron
information and patron status responses.

Test Plan:
1) Apply this patch
2) Create 3 new patron attributes with the codes CODE1, CODE2, CODE3.
   Make a least one repeatable.
3) Create a patron, add those attibutes for the patron, make sure there
   are at least two instances of the repeatable code
4) Edit your SIP2 config file, add the following within the login stanza:
   <patron_attribute field="XX" code="CODE1" />
   <patron_attribute field="XY" code="CODE2" />
   <patron_attribute field="XZ" code="CODE3" />
5) Using the sip cli emulator, run patron_information and
   patron_status_request messages for the patron
6) Note the values you set for the patron attributes are sent in the
   corrosponding fields!
Comment 3 Chris Kirby 2016-12-29 20:40:59 UTC
(In reply to Kyle M Hall from comment #2)
> Created attachment 58500 [details] [review] [review]
> Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2
> fields
> 
> Some libraries need to be able to send additional patron data from the
> extended patron attributes in made up SIP2 fields for the patron
> information and patron status responses.
> 
> Test Plan:
> 1) Apply this patch
> 2) Create 3 new patron attributes with the codes CODE1, CODE2, CODE3.
>    Make a least one repeatable.
> 3) Create a patron, add those attibutes for the patron, make sure there
>    are at least two instances of the repeatable code
> 4) Edit your SIP2 config file, add the following within the login stanza:
>    <patron_attribute field="XX" code="CODE1" />
>    <patron_attribute field="XY" code="CODE2" />
>    <patron_attribute field="XZ" code="CODE3" />
> 5) Using the sip cli emulator, run patron_information and
>    patron_status_request messages for the patron
> 6) Note the values you set for the patron attributes are sent in the
>    corrosponding fields!

At step 3, I added a patron (and added values to CODE1, CODE2, and CODE3). When I subsequently clicked on the cgi-bin/koha/admin/categories.pl,
I got the following error:

"Software error:

Template process failed: undef error - round() overflow. Try smaller precision or use Math::BigFloat at /home/ckirby/koha/Koha/Number/Price.pm line 44.

For help, please send mail to the webmaster ([no address given]), giving this error message and the time and date of the error. "
Comment 4 Kyle M Hall 2016-12-30 11:32:42 UTC
Chris, do you get that error on master alone? It would be worth checking on because I cannot imagine how this patch would cause an error like that!

> At step 3, I added a patron (and added values to CODE1, CODE2, and CODE3).
> When I subsequently clicked on the cgi-bin/koha/admin/categories.pl,
> I got the following error:
> 
> "Software error:
> 
> Template process failed: undef error - round() overflow. Try smaller
> precision or use Math::BigFloat at /home/ckirby/koha/Koha/Number/Price.pm
> line 44.
> 
> For help, please send mail to the webmaster ([no address given]), giving
> this error message and the time and date of the error. "
Comment 5 Kyle M Hall 2016-12-30 11:53:32 UTC
I just realized I never added the dependencies! This patch depends on bug 17792, that's probably why you got that error.

(In reply to Kyle M Hall from comment #4)
> Chris, do you get that error on master alone? It would be worth checking on
> because I cannot imagine how this patch would cause an error like that!
> 
> > At step 3, I added a patron (and added values to CODE1, CODE2, and CODE3).
> > When I subsequently clicked on the cgi-bin/koha/admin/categories.pl,
> > I got the following error:
> > 
> > "Software error:
> > 
> > Template process failed: undef error - round() overflow. Try smaller
> > precision or use Math::BigFloat at /home/ckirby/koha/Koha/Number/Price.pm
> > line 44.
> > 
> > For help, please send mail to the webmaster ([no address given]), giving
> > this error message and the time and date of the error. "
Comment 6 Chris Kirby 2016-12-30 17:11:15 UTC
(In reply to Kyle M Hall from comment #5)
> I just realized I never added the dependencies! This patch depends on bug
> 17792, that's probably why you got that error.
> 
> (In reply to Kyle M Hall from comment #4)
> > Chris, do you get that error on master alone? It would be worth checking on
> > because I cannot imagine how this patch would cause an error like that!
> > 
> > > At step 3, I added a patron (and added values to CODE1, CODE2, and CODE3).
> > > When I subsequently clicked on the cgi-bin/koha/admin/categories.pl,
> > > I got the following error:
> > > 
> > > "Software error:
> > > 
> > > Template process failed: undef error - round() overflow. Try smaller
> > > precision or use Math::BigFloat at /home/ckirby/koha/Koha/Number/Price.pm
> > > line 44.
> > > 
> > > For help, please send mail to the webmaster ([no address given]), giving
> > > this error message and the time and date of the error. "

Kyle, I am struggling to get bug 17792 to pass the tests (and to run the database update scripts). If you have a chance, look my comments on the bug.
Comment 7 Kyle M Hall 2017-02-09 12:04:36 UTC
Created attachment 60063 [details] [review]
Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2 fields

Some libraries need to be able to send additional patron data from the
extended patron attributes in made up SIP2 fields for the patron
information and patron status responses.

Test Plan:
1) Apply this patch
2) Create 3 new patron attributes with the codes CODE1, CODE2, CODE3.
   Make a least one repeatable.
3) Create a patron, add those attibutes for the patron, make sure there
   are at least two instances of the repeatable code
4) Edit your SIP2 config file, add the following within the login stanza:
   <patron_attribute field="XX" code="CODE1" />
   <patron_attribute field="XY" code="CODE2" />
   <patron_attribute field="XZ" code="CODE3" />
5) Using the sip cli emulator, run patron_information and
   patron_status_request messages for the patron
6) Note the values you set for the patron attributes are sent in the
   corrosponding fields!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Daniel Mauchley <dmauchley@duchesne.utah.gov>
Comment 8 Marcel de Rooy 2017-06-23 06:41:59 UTC
sub build_patron_attributes_string

Could you please add a small test in t/db_dependent/SIP for this new subroutine? Thanks.
Comment 9 Kyle M Hall 2017-10-26 15:14:38 UTC
Created attachment 68622 [details] [review]
Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2 fields

Some libraries need to be able to send additional patron data from the
extended patron attributes in made up SIP2 fields for the patron
information and patron status responses.

Test Plan:
1) Apply this patch
2) Create 3 new patron attributes with the codes CODE1, CODE2, CODE3.
   Make a least one repeatable.
3) Create a patron, add those attibutes for the patron, make sure there
   are at least two instances of the repeatable code
4) Edit your SIP2 config file, add the following within the login stanza:
   <patron_attribute field="XX" code="CODE1" />
   <patron_attribute field="XY" code="CODE2" />
   <patron_attribute field="XZ" code="CODE3" />
5) Using the sip cli emulator, run patron_information and
   patron_status_request messages for the patron
6) Note the values you set for the patron attributes are sent in the
   corrosponding fields!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Daniel Mauchley <dmauchley@duchesne.utah.gov>
Comment 10 Kyle M Hall 2017-10-26 15:49:33 UTC
(In reply to Marcel de Rooy from comment #8)
> sub build_patron_attributes_string
> 
> Could you please add a small test in t/db_dependent/SIP for this new
> subroutine? Thanks.

Good idea. I've added them to the initial patch!
Comment 11 Marcel de Rooy 2017-10-27 06:31:36 UTC
Created attachment 68655 [details] [review]
Bug 17826 - Allow extended patron attributes to be sent in arbitrary SIP2 fields

Some libraries need to be able to send additional patron data from the
extended patron attributes in made up SIP2 fields for the patron
information and patron status responses.

Test Plan:
1) Apply this patch
2) Create 3 new patron attributes with the codes CODE1, CODE2, CODE3.
   Make a least one repeatable.
3) Create a patron, add those attibutes for the patron, make sure there
   are at least two instances of the repeatable code
4) Edit your SIP2 config file, add the following within the login stanza:
   <patron_attribute field="XX" code="CODE1" />
   <patron_attribute field="XY" code="CODE2" />
   <patron_attribute field="XZ" code="CODE3" />
5) Using the sip cli emulator, run patron_information and
   patron_status_request messages for the patron
6) Note the values you set for the patron attributes are sent in the
   corrosponding fields!

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Daniel Mauchley <dmauchley@duchesne.utah.gov>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Amended: added parentheses on line 488 when assigning hashref to array.
Comment 12 Marcel de Rooy 2017-10-27 06:34:28 UTC
QA Comment:
[1] FAIL   pod coverage: POD is missing for build_patron_attributes_string
No blocker; the whole module has no POD.
[2] Wonder if we should check if ExtendedPatronAttributes has been set. If I disable it, the attributes are still visible. I guess this is a bug on itself, since it comes from Koha::Patron::Attribute(s)..
No blocker
Comment 13 Jonathan Druart 2017-10-27 14:30:37 UTC
FAIL   C4/SIP/ILS/Patron.pm
  FAIL   pod coverage
    POD is missing for build_patron_attributes_string