Description
Nicole C. Engard
2014-09-17 19:30:43 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). Created attachment 91057 [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 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> 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 Hi Nick, Thanks very much for your test feedback I will make changes now. Cheers, Alex Created attachment 91724 [details] [review] Bug 12949: (follow-up) Disable syspref by default Implemented fixes based on test feedback: * Set the GuarantorsMustBeLinkedToPatronAccount 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 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 also in the database confirm the guarantorid, contactname, contactfirstname, relationship are set to NULL. 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) also check in the database and confirm that guarantorid is NULL, and contactname, contactfirstname and relationship are not NULL. 19. Repeat steps 7-10 and confirm the guarantor information is displayed Also check in the database and confirm the guarantorid is NULL and contactname, contactfirstname and relationship are not NULL 20. Sign off Sponsored-By: Brimbank Library, Australia Created attachment 91725 [details] [review] Bug 12949: (follow-up) Disable syspref by default Implemented fixes based on test feedback: * Set the GuarantorsMustBeLinkedToPatronAccount 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 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 also in the database confirm the guarantorid, contactname, contactfirstname, relationship are set to NULL. 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) also check in the database and confirm that guarantorid is NULL, and contactname, contactfirstname and relationship are not NULL. 19. Repeat steps 7-10 and confirm the guarantor information is displayed Also check in the database and confirm the guarantorid is NULL and contactname, contactfirstname and relationship are not NULL 20. Sign off Sponsored-By: Brimbank Library, Australia Created attachment 91726 [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 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 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 I've rebased and reattached the second commit so it applies cleanly on master. merge conflict on master: Fusion automatique de members/moremember.pl CONFLIT (contenu) : Conflit de fusion dans members/moremember.pl Could this be kind of a duplicate to bug 23224? 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. *** Bug 15143 has been marked as a duplicate of this bug. *** 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 |