Bug 24215

Summary: Warnings about guarantor relationships show ARRAY errors
Product: Koha Reporter: Martin Renvoize <martin.renvoize>
Component: AboutAssignee: Martin Renvoize <martin.renvoize>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: 1joynelson, jonathan.druart, katrin.fischer, lucas, rudolfbyker, tomascohen
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.05.00, 19.11.02
Bug Depends on: 14570    
Bug Blocks: 24263    
Attachments: Bug 24215: Correct display of guarantor relationship errors
Bug 24215: Correct display of guarantor relationship errors
Bug 24215: Correct display of guarantor relationship errors
Bug 24215: Do not always display an empty entry

Description Martin Renvoize 2019-12-11 11:33:25 UTC
Bug 14570 introduced a warning to the about page to catch and display data inconsistencies with respect to guarantor relationships matching te system preference values.

However, the warning code is incorrect and therefore instead of the missing values being displayed in a list we get a nasty perl ARRAY(0x55f2846812d0) error.
Comment 1 Martin Renvoize 2019-12-11 11:36:36 UTC
Created attachment 96195 [details] [review]
Bug 24215: Correct display of guarantor relationship errors

Test plan:
1) Add some bad data to your borrower_relationships table. ( add or
   update a relationship such that the 'relationship' field contains a
   value not listed in the corresponding system preference.
2) Go to the about system information page and note the nasty display of
   ARRAY(whatever)
3) Apply the patch
4) Repeat step 2 and note that a list of bad values is now displayed
   instead.
Comment 2 Kyle M Hall 2019-12-11 13:39:26 UTC
Created attachment 96208 [details] [review]
Bug 24215: Correct display of guarantor relationship errors

Test plan:
1) Add some bad data to your borrower_relationships table. ( add or
   update a relationship such that the 'relationship' field contains a
   value not listed in the corresponding system preference.
2) Go to the about system information page and note the nasty display of
   ARRAY(whatever)
3) Apply the patch
4) Repeat step 2 and note that a list of bad values is now displayed
   instead.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 3 Jonathan Druart 2019-12-11 19:50:30 UTC
+    my $wrong_relationships = [ grep { !$valid_relationships{ $_->[0] } } @{$existing_relationships} ];
+    if ( $wrong_relationships or $bad_relationships_count ) {

I am expecting this condition to be always true (ARRAYREF)
Comment 4 Jonathan Druart 2019-12-12 14:51:10 UTC
Confirmed.
To recreate:
Do nothing, go to about.pl.
You see: https://snipboard.io/83ID0r.jpg
Comment 5 Katrin Fischer 2019-12-18 07:16:00 UTC
As this is affecting new 19.11 installations and has come up on the mailing list already, it would be nice to have it fixed!
Comment 6 Jonathan Druart 2019-12-18 08:05:23 UTC
Created attachment 96395 [details] [review]
Bug 24215: Correct display of guarantor relationship errors

Test plan:
1) Add some bad data to your borrower_relationships table. ( add or
   update a relationship such that the 'relationship' field contains a
   value not listed in the corresponding system preference.
2) Go to the about system information page and note the nasty display of
   ARRAY(whatever)
3) Apply the patch
4) Repeat step 2 and note that a list of bad values is now displayed
   instead.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 7 Jonathan Druart 2019-12-18 08:05:27 UTC
Created attachment 96396 [details] [review]
Bug 24215: Do not always display an empty entry

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 8 Jonathan Druart 2019-12-18 08:07:51 UTC
This second patch fixes the following: on a fresh install we do not display anything on the about page (see screenshot on comment 4).

However there is still something wrong, but not sure it comes from this patch.
When you create a new patron, its relationship field is populated with an empty string, and it seems we are expecting NULL.

So we should include "" in this patch, and/or fix the original issue (ie. insert NULL instead of empty string, and update the existing occurrences in DB).
Comment 9 Martin Renvoize 2019-12-18 12:30:22 UTC
Thanks for the followup Jonathan.. I've been struggling to keep up the last few days.

An interesting note about NULL vs ""... as you say, feels like that should be fixed at a lower level
Comment 10 Jonathan Druart 2019-12-18 13:03:58 UTC
(In reply to Martin Renvoize from comment #9)
> An interesting note about NULL vs ""... as you say, feels like that should
> be fixed at a lower level

See bug 24263
Comment 11 Martin Renvoize 2019-12-18 16:06:36 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 12 Martin Renvoize 2019-12-20 12:09:56 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 13 Joy Nelson 2020-01-03 18:13:44 UTC
Pushed to 19.11.x branch for 19.11.02
Comment 14 Lucas Gass 2020-01-07 23:04:37 UTC
missing dependency for 19.05.x, no backport
Comment 15 Rudolf Byker 2020-01-15 09:46:17 UTC
Sorry to rain on your parade, but after installing 19.11, with no relationships in use whatsoever, I still get this on the about page:

=====

The following values have been used for guarantee/guarantor relationships, but do not exist in the 'borrowerRelationship' system preference:

[Bullet point with no content]

If the relationship is one you want, please add it to the 'borrowerRelationship' system preference, otherwise have your system's administrator correct the values in borrowers.relationship and/or borrower_relationships.relationship in the database.

=====

Here is some info about my DB:

=====

select relationship, count(*) from borrowers group by
relationship;

relationship  count(*) 	
NULL          6

=====

The borrower_relationships table is empty.
Comment 16 Joy Nelson 2020-01-15 14:46:36 UTC
This patch will not be in Koha until the 19.11.02 release which is scheduled for next week.  It would not be in 19.11.00 or 19.11.01


(In reply to Rudolf Byker from comment #15)
> Sorry to rain on your parade, but after installing 19.11, with no
> relationships in use whatsoever, I still get this on the about page:
> 
> =====
> 
> The following values have been used for guarantee/guarantor relationships,
> but do not exist in the 'borrowerRelationship' system preference:
> 
> [Bullet point with no content]
> 
> If the relationship is one you want, please add it to the
> 'borrowerRelationship' system preference, otherwise have your system's
> administrator correct the values in borrowers.relationship and/or
> borrower_relationships.relationship in the database.
> 
> =====
> 
> Here is some info about my DB:
> 
> =====
> 
> select relationship, count(*) from borrowers group by
> relationship;
> 
> relationship  count(*) 	
> NULL          6
> 
> =====
> 
> The borrower_relationships table is empty.
Comment 17 Joy Nelson 2020-01-15 14:48:29 UTC
Also, it seems like you are looking for this fix: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24263 which has not yet been pushed to the 19.11.x branch.

joy

(In reply to Joy Nelson from comment #16)
> This patch will not be in Koha until the 19.11.02 release which is scheduled
> for next week.  It would not be in 19.11.00 or 19.11.01
> 
> 
> (In reply to Rudolf Byker from comment #15)
> > Sorry to rain on your parade, but after installing 19.11, with no
> > relationships in use whatsoever, I still get this on the about page:
> > 
> > =====
> > 
> > The following values have been used for guarantee/guarantor relationships,
> > but do not exist in the 'borrowerRelationship' system preference:
> > 
> > [Bullet point with no content]
> > 
> > If the relationship is one you want, please add it to the
> > 'borrowerRelationship' system preference, otherwise have your system's
> > administrator correct the values in borrowers.relationship and/or
> > borrower_relationships.relationship in the database.
> > 
> > =====
> > 
> > Here is some info about my DB:
> > 
> > =====
> > 
> > select relationship, count(*) from borrowers group by
> > relationship;
> > 
> > relationship  count(*) 	
> > NULL          6
> > 
> > =====
> > 
> > The borrower_relationships table is empty.
Comment 18 Rudolf Byker 2020-01-16 06:23:17 UTC
Sorry, I misread the third part of the version number! :)