Bug 12949 - changing child to adult doesn't remove link to guarantor
Summary: changing child to adult doesn't remove link to guarantor
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Patrons (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Hayley Pelham
QA Contact: Testopia
URL:
Keywords:
: 15143 (view as bug list)
Depends on: 23224
Blocks:
  Show dependency treegraph
 
Reported: 2014-09-17 19:30 UTC by Nicole C. Engard
Modified: 2020-11-30 21:44 UTC (History)
17 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 12949: Added syspref to control display of unlinked guarantor (7.31 KB, patch)
2019-06-28 02:52 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 12949: Added syspref to control display of unlinked guarantor (7.36 KB, patch)
2019-06-28 19:09 UTC, Nadine Pierre
Details | Diff | Splinter Review
Bug 12949: (follow-up) Disable syspref by default (9.10 KB, patch)
2019-07-23 04:25 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 12949: (follow-up) Disable syspref by default (8.16 KB, patch)
2019-07-23 04:34 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 12949: (follow-up) Disable syspref by default (10.27 KB, patch)
2019-07-23 04:59 UTC, Alex Buckley
Details | Diff | Splinter Review
Bug 12949: (follow-up) Disable syspref by default (10.70 KB, patch)
2019-07-26 02:49 UTC, Alex Buckley
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2014-09-17 19:30:43 UTC
To recreate:

1. create an adult
2. add a child to said adult
3. edit child and change them to adult
4. see that link to guarantor is gone on child
5. search for guarantor (original adult)
6. see that there is still a guarantee there even though he/she is an adult now
Comment 1 Owen Leonard 2016-08-10 16:52:59 UTC
The way this works hasn't changed in master, but any automatic guarantor removal will have to take Bug 12446 into consideration (if 12446 makes it in).
Comment 2 Alex Buckley 2019-06-28 02:52:37 UTC Comment hidden (obsolete)
Comment 3 Nadine Pierre 2019-06-28 19:09:49 UTC
Created attachment 91121 [details] [review]
Bug 12949: Added syspref to control display of unlinked guarantor

When upgrading a child to adult patron the
guarantor details are still displayed on the new adult patrons record.

This is because upgrading a child to an adult patron involves clearing
the borrowers.guarantorid value but not the borrowers.contactname,
borrowers.contactfirstname

Bug 15928 introduced functionality to display contactname and
contactfirstname when no guarantorid exists. This was so unlinked guarantors (non-librart patrons) could be set.

The display of guarantor information when the guarantorid has been
cleared following upgrading to an adult patron category is confusing, as it appears that the guarantor relationship
already exists even though the child has been upgraded to an adult.

In this patch we introduce a sytem preference GuarantorsMustBeLinkedToPatronAccount (enabled by default) - which
keeps the default Koha functionality introduced by 15928 when the
syspref is disabled - to only display gurantor information if a
guarantorid exists.

Test plan:
1. Create an adult patron (make sure to set a first and last name) and select 'Add child' and save the child
record

2. Run the following database query, and notice there are values for guarantorid, contactname, contactfirstname:
SELECT guarantorid, contactname, contactfirstname FROM borrowers WHERE
borrowernumber=<childsborrowernumber>;

3. On the child record select More > Update child to adult patron.

4. Select a new adult patron category and save

5. Notice guarantor information is still displayed on the new adults
record

6. Repeat step 2 and notice guarantorid is empty/NULL and contactname
and contactfirstname are unchanged

7. Now create a child record with an unlinked guarantor: In the patron home page select 'New patron'.

Note: Make sure the patron category you select is of category type 'Child'

8. In the guarantor area of the patron creation page input a firstname
and surname and don't select 'Set to patron' and save/submit the page

9. Repeat step 2 making the borrowernumber you enter the borrowernumber
of the patron you made in step 8 and notice guarantorid is empty and
contactname and contactfirstname are not empty. i.e. our guarantor is
not linked to a Koha patron

10. Notice guarantor information is shown on the patron summary page of
our user

11. Repeat step 3,4,5 and notice guarantor information is still
displayed

12. Apply patch

13. Navigate to installer/data/mysql directory

Enter the Koha shell:
sudo koha-shell <instancename>

Run:
./updatedatabase.pl

14. In staff client visit Administration > Global System preferences and
search for GuarantorsMustBeLinkedToPatronAccount syspref and confirm it
is enabled

15. Repeat steps 1-5 and notice that the guarantor information is not
displayed on the new adult's record

16. Repeat steps 7-10 and notice that the guarantor information is not
displayed on the new adult's record

17. Set GuarantorsMustBeLinkedToPatronAccount syspref to disabled

18. Repeat steps 1-5 and confirm the guarantor information is displayed
on the new adult's record (i.e. setting GuarantorsMustBeLinkedToPatronAccount
syspref to disabled keeps the current Koha behaviour)

19. Repeat steps 7-10 and confirm the guarantor information is displayed

20. Sign off

Sponsored-By: Brimbank Library, Australia
Signed-off-by: Nadine Pierre <nadine.pierre@inLibro.com>
Comment 4 Nick Clemens (kidclamp) 2019-07-19 11:18:12 UTC
Hi Alex,

A few comments:
1 - New preferences should be defaulted to 'off' - i.e. user should have to activate the behavior change
2 - This pref does not prevent entering unlinked information, it only blocks display. I think if we are not going to show it, maybe we shouldn't let the user enter it?
3 - I don't think this tackles the problem as reported, we should remove the contactname and contactfirstname if removing the guarantor id. The doesn't have to be handled here, but if it isn't then we should open a new bug for the original report
Comment 5 Alex Buckley 2019-07-23 03:08:27 UTC
Hi Nick, 

Thanks very much for your test feedback I will make changes now. 

Cheers,
Alex
Comment 6 Alex Buckley 2019-07-23 04:25:14 UTC Comment hidden (obsolete)
Comment 7 Alex Buckley 2019-07-23 04:34:25 UTC Comment hidden (obsolete)
Comment 8 Alex Buckley 2019-07-23 04:59:36 UTC Comment hidden (obsolete)
Comment 9 Alex Buckley 2019-07-23 05:13:59 UTC
Hi Nadine and Nick, 

Thanks for taking the time to test/look at my patches.

I've done the following in the follow-up patch I've just attached in response to your notes in comment #4 Nick:

Re. point 1. 

* Renamed the syspref so it has a name more relevant to what it does: RemoveGuarantorDataFromGuaranteeWhenUpdatedToAdult

* Set the preference to OFF by default so if the library wants the behaviour change they have to actively enable it. 

Re. point 2. 

* As the syspref has been renamed to reflect that it doesn't block unlinked patrons from being set as guarantors this should be 
resolved now. Having said that it could be a good idea in the future to block unlinked patrons from being set as guarantors in
another bug report in the future. 

Re. point 3. 

* When the aforementioned syspref is enabled then the contactname, contactfirstname and relationship are set to NULL upon a guarantee being updated to a guarantor via members/update-child.pl or members/memberentry.pl

Can you please retest?

Cheers,
Alex
Comment 10 Alex Buckley 2019-07-26 02:49:54 UTC
Created attachment 91790 [details] [review]
Bug 12949: (follow-up) Disable syspref by default

Implemented fixes based on test feedback:

* Renamed syspref now called:
RemoveGuarantorDataFromGuaranteeWhenUpdatedToAdult

* Also set syspref to 0 (disabled) by default

* When syspref is enabled and a guarantee is updated to an 'Adult'
their borrowers.guarantorid, borrowers.contactname,
borrowers.contactfirstname, borrowers.relationship are reset to NULL.

Updated test plan:
1. Create an adult patron (make sure to set a first and last name) and
select 'Add child' and save the child
record

2. Run the following database query, and notice there are values for
guarantorid, contactname, contactfirstname, relationship:
SELECT guarantorid, contactname, contactfirstname, relationship FROM borrowers WHERE
borrowernumber=<childsborrowernumber>;

3. On the child record select More > Update child to adult patron.

4. Select a new adult patron category and save

5. Notice guarantor information is still displayed on the new adults
record

6. Repeat step 2 and notice guarantorid is empty/NULL and contactname
and contactfirstname, relationship are unchanged

7. Now create a child record with an unlinked guarantor: In the patron
home page select 'New patron'.

Note: Make sure the patron category you select is of category type
'Child'

8. In the guarantor area of the patron creation page input a firstname
and surname and don't select 'Set to patron' and save/submit the page

9. Repeat step 2 making the borrowernumber you enter the borrowernumber
of the patron you made in step 8 and notice guarantorid is empty and
contactname and contactfirstname are not empty. i.e. our guarantor is
not linked to a Koha patron

10. Notice guarantor information is shown on the patron summary page of
our user

11. Repeat step 3,4,5 and notice guarantor information is still
displayed

12. Apply patch

13. Navigate to installer/data/mysql directory

Enter the Koha shell:
sudo koha-shell <instancename>

Run:
./updatedatabase.pl

14. In staff client visit Administration > Global System preferences and
search for RemoveGuarantorDataFromGuaranteeWhenUpdatedToAdult syspref and confirm it
is disabled

15. Repeat steps 1-5 and notice that the guarantor information is
displayed on the new adult's record (i.e. when the
RemoveGuarantorDataFromGuaranteeWhenUpdatedToAdult syspref is
disabled the current Koha behaviour occurs).

16. In the database confirm the
guarantorid is NULL and contactname, contactfirstname and relationship
are not set to NULL.

17. Repeat steps 7-10 and notice that the guarantor information is
displayed on the new adult's record

Also repeat step 16 and confirm the same outcome

18. Set GuarantorsMustBeLinkedToPatronAccount syspref to enabled

19. Repeat steps 1-5 and confirm the guarantor information is not displayed
on the new adult's record (i.e. setting
GuarantorsMustBeLinkedToPatronAccount
syspref to enabled implements the behaviour change).

Also repeat step 16 and confirm the guarantorid, contactname,
contactfirstname and relationship are all NULL

20. Repeat steps 7-10 and confirm the guarantor information is not displayed

Also repeat step 16 and confirm the guarantorid,
contactname, contactfirstname and relationship are NULL

21. Sign off

Sponsored-By: Brimbank Library, Australia
Comment 11 Alex Buckley 2019-07-26 02:51:15 UTC
I've rebased and reattached the second commit so it applies cleanly on master.
Comment 12 Arthur Suzuki 2019-10-01 08:41:20 UTC
merge conflict on master:

Fusion automatique de members/moremember.pl
CONFLIT (contenu) : Conflit de fusion dans members/moremember.pl
Comment 13 Katrin Fischer 2019-10-04 06:33:14 UTC
Could this be kind of a duplicate to bug 23224?
Comment 14 Martin Renvoize 2019-10-25 16:23:00 UTC
I believe this is at least partially resolved as part of bug 17168, see the RM follow-up there that shift the logic into Koha::Patron such that any change of categorycode from guarantee-able to non-guarantee-able will result in the relationship being deleted as part of the action.
Comment 15 Martin Renvoize 2019-10-25 16:23:57 UTC
*** Bug 15143 has been marked as a duplicate of this bug. ***
Comment 16 Aleisha Amohia 2020-03-26 21:02:33 UTC
I believe this patch is no longer required - in master, guarantors and guarantees are stored in borrower_relationships in the database. the relationship information is deleted when changing from child patron to adult patron using memberentry.pl and child-update.pl. Closing bug report