Bug 12133 changed the way that these categories work. Before hand, a single category could be set up, and two patrons in that category could be set to have a guarantor/guarantee relationship. In upgrades, we are seeing that if this relationship is preexisting, patrons are unable to return items as we throw an exception: "Child cannot be a guarantor" I understand preventing the same patron from being a guarantor and a guarantee, however, is it correct that the same category cannot? We either need to: 1 - Add an update to fix or warn of this existing condition 2 - Remove this restriction
Created attachment 171334 [details] [review] Bug 37892: Unit tests These patches will alter the checks for a patron that prevent a category with 'can_be_guarantee' from being a guarantor. Two patrons in the same category should be allowed to have a guarantee/guarantor relationship The tests below assume you are using the KTD sample data. Update borrowernumbers if not. To test: 0 - Apply tests patch 1 - Set the 'Patron' category as 'Can be a guarantee' 2 - Add a relationship between two patrons of the same category This is restricted from the staff interface perl -e 'use Koha::Patrons; my $p = Koha::Patrons->find(5)->add_guarantor({ guarantor_id => 23, relationship => 'father'});' 3 - Note there is no warning or exception. This should be allowed. 4 - Checkout an item to Edna (borrowernumber 5) 5 - Set 'TrackLastPatronActivityTriggers' to 'Checking in an item' 6 - Try to check the item in, KABOOM 7 - Set 'TrackLastPatronActivityTriggers' to 'Checking out an item' 8 - Try to issue an item to Enda, KABOOM 9 - prove -v t/db_dependent/Koha/Patron.t, fail 10 - Apply second patch 11 - prove -v t/db_dependent/Koha/Patron.t, one more test passes, but then fail 12 - Apply third patch 13 - prove -v t/db_dependent/Koha/Patron.t, pass! 14 - restart_all 15 - Checkout to Enda, OK! 16 - Checkin from Edna, OK! 17 - Find two more patrons in the category and attempt to link them 18 - 'Guarantor cannot be a guarantee' 19 - Apply fourth patch 20 - You can add a guarantor from the same category in interface 21 - Try to add a guarantor to the guarantor assigned in 20 22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee"
Created attachment 171335 [details] [review] Bug 37892: Fix patron updates
Created attachment 171336 [details] [review] Bug 37892: Fix patron creation
Created attachment 171337 [details] [review] Bug 37892: Fix memberentry.pl
Hi ! I could not reproduce the errror in 18- : I do not have any button in the interface to add a guarantor and the command you gave in 2- works perfectly. The rest works perfectly atm
(In reply to Baptiste Wojtkowski (bwoj) from comment #5) > Hi ! > I could not reproduce the errror in 18- : I do not have any button in the > interface to add a guarantor and the command you gave in 2- works perfectly. > The rest works perfectly atm For 18 it is not a single button, apologies: 1 - Find a patron in the category that can be a guarantee 2 - Edit the patron 3 - The second section should be 'Patron guarantor' with an 'Add guarantor' button 4 - Find/Select a patron in the same category - you can 5 - Save the patron 6 - Error
Oh my bad, I picked a user that could be guarantor to be guarantor and was waiting for an error.. However I cannot reproduce the last point. With all patches, I could obtain following only using the interface (data are the same as KTD ones I guess, Dawn and Johnny) : +-----+--------------+--------------+--------------+ | id | guarantor_id | guarantee_id | relationship | +-----+--------------+--------------+--------------+ | 102 | 23 | 31 | father | | 103 | 31 | 23 | father | +-----+--------------+--------------+--------------+
Created attachment 171451 [details] [review] Bug 37892: (follow-up) Fix patron creation This patch fixes the 22. of the test plan (22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee" )
Note : Can I signoff the patch and request to signoff my follow-up or does it need a third person to sign off the whole ?
Created attachment 171463 [details] [review] Bug 37892: Unit tests These patches will alter the checks for a patron that prevent a category with 'can_be_guarantee' from being a guarantor. Two patrons in the same category should be allowed to have a guarantee/guarantor relationship The tests below assume you are using the KTD sample data. Update borrowernumbers if not. To test: 0 - Apply tests patch 1 - Set the 'Patron' category as 'Can be a guarantee' 2 - Add a relationship between two patrons of the same category This is restricted from the staff interface perl -e 'use Koha::Patrons; my $p = Koha::Patrons->find(5)->add_guarantor({ guarantor_id => 23, relationship => 'father'});' 3 - Note there is no warning or exception. This should be allowed. 4 - Checkout an item to Edna (borrowernumber 5) 5 - Set 'TrackLastPatronActivityTriggers' to 'Checking in an item' 6 - Try to check the item in, KABOOM 7 - Set 'TrackLastPatronActivityTriggers' to 'Checking out an item' 8 - Try to issue an item to Enda, KABOOM 9 - prove -v t/db_dependent/Koha/Patron.t, fail 10 - Apply second patch 11 - prove -v t/db_dependent/Koha/Patron.t, one more test passes, but then fail 12 - Apply third patch 13 - prove -v t/db_dependent/Koha/Patron.t, pass! 14 - restart_all 15 - Checkout to Enda, OK! 16 - Checkin from Edna, OK! 17 - Find two more patrons in the category and attempt to link them 18 - 'Guarantor cannot be a guarantee' 19 - Apply fourth patch 20 - You can add a guarantor from the same category in interface 21 - Try to add a guarantor to the guarantor assigned in 20 22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee" Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Created attachment 171464 [details] [review] Bug 37892: Fix patron updates Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Created attachment 171465 [details] [review] Bug 37892: Fix patron creation Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Created attachment 171466 [details] [review] Bug 37892: Fix memberentry.pl Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
Created attachment 171467 [details] [review] Bug 37892: (follow-up) Fix patron creation This patch fixes the 22. of the test plan (22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee" ) Signed-off-by: Olivier V <olivier.vezina@inLibro.com>
(In reply to Baptiste Wojtkowski (bwoj) from comment #7) > Oh my bad, I picked a user that could be guarantor to be guarantor and was > waiting for an error.. > > However I cannot reproduce the last point. With all patches, I could obtain > following only using the interface (data are the same as KTD ones I guess, > Dawn and Johnny) : > > +-----+--------------+--------------+--------------+ > | id | guarantor_id | guarantee_id | relationship | > +-----+--------------+--------------+--------------+ > | 102 | 23 | 31 | father | > | 103 | 31 | 23 | father | > +-----+--------------+--------------+--------------+ When I tested I found the 5th patch resolved this issue, and if you try you get the error message: A guarantor cannot be a guarantee. But if you use a third patron when you add the guarantor it still lets you. +----+--------------+--------------+--------------+ | id | guarantor_id | guarantee_id | relationship | +----+--------------+--------------+--------------+ | 1 | 23 | 5 | father | | 2 | 38 | 23 | father | +----+--------------+--------------+--------------+ Not sure if this is an issue or if this is supposed to be allowed.
Hum, I guess this should definitely be patched. Could you please tell me in which order you added guarantors to each other ?
I'm stupid it's the order of the rules... Looks coherent with the patch applied, it checks the guarantor is not a guarantee, but it should also check that the guarantee is not already a guarantor
(In reply to Baptiste Wojtkowski (bwoj) from comment #17) > I'm stupid it's the order of the rules... > > Looks coherent with the patch applied, it checks the guarantor is not a > guarantee, but it should also check that the guarantee is not already a > guarantor I will test again and sign off when there's a follow up
Created attachment 171472 [details] [review] Bug 37892: (follow-up) Fix patron creation TEST PLAN: 1 - Do the 22 parts of the test plan 2 - Add a guarantor to one patron not selected before (let's say A is the guarantee, B the guarantor) 3 - Try and add a guarantor to B -> you will success 4 - Remove B's guarantor 5 - Apply this patch 6 - Repeat 3 -> you will not be able to
My bad i missed this detail. The patch is working fine on my part, i'll let Brendan check it and sign it off to be sure
Created attachment 171475 [details] [review] Bug 37892: Unit tests These patches will alter the checks for a patron that prevent a category with 'can_be_guarantee' from being a guarantor. Two patrons in the same category should be allowed to have a guarantee/guarantor relationship The tests below assume you are using the KTD sample data. Update borrowernumbers if not. To test: 0 - Apply tests patch 1 - Set the 'Patron' category as 'Can be a guarantee' 2 - Add a relationship between two patrons of the same category This is restricted from the staff interface perl -e 'use Koha::Patrons; my $p = Koha::Patrons->find(5)->add_guarantor({ guarantor_id => 23, relationship => 'father'});' 3 - Note there is no warning or exception. This should be allowed. 4 - Checkout an item to Edna (borrowernumber 5) 5 - Set 'TrackLastPatronActivityTriggers' to 'Checking in an item' 6 - Try to check the item in, KABOOM 7 - Set 'TrackLastPatronActivityTriggers' to 'Checking out an item' 8 - Try to issue an item to Enda, KABOOM 9 - prove -v t/db_dependent/Koha/Patron.t, fail 10 - Apply second patch 11 - prove -v t/db_dependent/Koha/Patron.t, one more test passes, but then fail 12 - Apply third patch 13 - prove -v t/db_dependent/Koha/Patron.t, pass! 14 - restart_all 15 - Checkout to Enda, OK! 16 - Checkin from Edna, OK! 17 - Find two more patrons in the category and attempt to link them 18 - 'Guarantor cannot be a guarantee' 19 - Apply fourth patch 20 - You can add a guarantor from the same category in interface 21 - Try to add a guarantor to the guarantor assigned in 20 22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee" Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Created attachment 171476 [details] [review] Bug 37892: Fix patron updates Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Created attachment 171477 [details] [review] Bug 37892: Fix patron creation Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Created attachment 171478 [details] [review] Bug 37892: Fix memberentry.pl Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Created attachment 171479 [details] [review] Bug 37892: (follow-up) Fix patron creation This patch fixes the 22. of the test plan (22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee" ) Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Created attachment 171480 [details] [review] Bug 37892: (follow-up) Fix patron creation TEST PLAN: 1 - Do the 22 parts of the test plan 2 - Add a guarantor to one patron not selected before (let's say A is the guarantee, B the guarantor) 3 - Try and add a guarantor to B -> you will success 4 - Remove B's guarantor 5 - Apply this patch 6 - Repeat 3 -> you will not be able to Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Created attachment 171546 [details] [review] Bug 37892: Unit tests These patches will alter the checks for a patron that prevent a category with 'can_be_guarantee' from being a guarantor. Two patrons in the same category should be allowed to have a guarantee/guarantor relationship The tests below assume you are using the KTD sample data. Update borrowernumbers if not. To test: 0 - Apply tests patch 1 - Set the 'Patron' category as 'Can be a guarantee' 2 - Add a relationship between two patrons of the same category This is restricted from the staff interface perl -e 'use Koha::Patrons; my $p = Koha::Patrons->find(5)->add_guarantor({ guarantor_id => 23, relationship => 'father'});' 3 - Note there is no warning or exception. This should be allowed. 4 - Checkout an item to Edna (borrowernumber 5) 5 - Set 'TrackLastPatronActivityTriggers' to 'Checking in an item' 6 - Try to check the item in, KABOOM 7 - Set 'TrackLastPatronActivityTriggers' to 'Checking out an item' 8 - Try to issue an item to Enda, KABOOM 9 - prove -v t/db_dependent/Koha/Patron.t, fail 10 - Apply second patch 11 - prove -v t/db_dependent/Koha/Patron.t, one more test passes, but then fail 12 - Apply third patch 13 - prove -v t/db_dependent/Koha/Patron.t, pass! 14 - restart_all 15 - Checkout to Enda, OK! 16 - Checkin from Edna, OK! 17 - Find two more patrons in the category and attempt to link them 18 - 'Guarantor cannot be a guarantee' 19 - Apply fourth patch 20 - You can add a guarantor from the same category in interface 21 - Try to add a guarantor to the guarantor assigned in 20 22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee" Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Created attachment 171547 [details] [review] Bug 37892: Fix patron updates Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Created attachment 171548 [details] [review] Bug 37892: Fix patron creation Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Created attachment 171549 [details] [review] Bug 37892: Fix memberentry.pl Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Created attachment 171550 [details] [review] Bug 37892: (follow-up) Tidyness
Created attachment 171663 [details] [review] Bug 37892: (follow-up) Fix patron creation This patch fixes the 22. of the test plan (22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee" ) Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Created attachment 171664 [details] [review] Bug 37892: (follow-up) Fix patron creation TEST PLAN: 1 - Do the 22 parts of the test plan 2 - Add a guarantor to one patron not selected before (let's say A is the guarantee, B the guarantor) 3 - Try and add a guarantor to B -> you will success 4 - Remove B's guarantor 5 - Apply this patch 6 - Repeat 3 -> you will not be able to Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org>
Created attachment 171665 [details] [review] Bug 37892: (follow-up) Tidyness
Looking here. Squashing those patches.
Created attachment 171824 [details] [review] Bug 37892: Fix guarantor restriction, add tests [SQUASHED IN QA] These patches will alter the checks for a patron that prevent a category with 'can_be_guarantee' from being a guarantor. Two patrons in the same category should be allowed to have a guarantee/guarantor relationship The tests below assume you are using the KTD sample data. Update borrowernumbers if not. To test: 0 - Apply tests patch 1 - Set the 'Patron' category as 'Can be a guarantee' 2 - Add a relationship between two patrons of the same category This is restricted from the staff interface perl -e 'use Koha::Patrons; my $p = Koha::Patrons->find(5)->add_guarantor({ guarantor_id => 23, relationship => 'father'});' 3 - Note there is no warning or exception. This should be allowed. 4 - Checkout an item to Edna (borrowernumber 5) 5 - Set 'TrackLastPatronActivityTriggers' to 'Checking in an item' 6 - Try to check the item in, KABOOM 7 - Set 'TrackLastPatronActivityTriggers' to 'Checking out an item' 8 - Try to issue an item to Enda, KABOOM 9 - prove -v t/db_dependent/Koha/Patron.t, fail 10 - Apply second patch 11 - prove -v t/db_dependent/Koha/Patron.t, one more test passes, but then fail 12 - Apply third patch 13 - prove -v t/db_dependent/Koha/Patron.t, pass! 14 - restart_all 15 - Checkout to Enda, OK! 16 - Checkin from Edna, OK! 17 - Find two more patrons in the category and attempt to link them 18 - 'Guarantor cannot be a guarantee' 19 - Apply fourth patch 20 - You can add a guarantor from the same category in interface 21 - Try to add a guarantor to the guarantor assigned in 20 22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee" Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Bug 37892: Fix patron updates Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Bug 37892: Fix patron creation Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Bug 37892: Fix memberentry.pl Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Bug 37892: (follow-up) Fix patron creation This patch fixes the 22. of the test plan (22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee" ) Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Bug 37892: (follow-up) Fix patron creation TEST PLAN: 1 - Do the 22 parts of the test plan 2 - Add a guarantor to one patron not selected before (let's say A is the guarantee, B the guarantor) 3 - Try and add a guarantor to B -> you will success 4 - Remove B's guarantor 5 - Apply this patch 6 - Repeat 3 -> you will not be able to Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Bug 37892: (follow-up) Tidyness Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT] Renamed a subtest to patron creation tests in Patron.t.
Created attachment 171825 [details] [review] Bug 37892: (QA follow-up) Add tests for ->is_guarantor/ee Test plan: Run t/db_dependent/Koha/Patron.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Created attachment 171826 [details] [review] Bug 37892: (QA follow-up) Fix condition in memberentry Patron may be undefined. So the test may crash. Also there is an issue with operator precedence.
We are not there yet unfortunately. There is still an issue with at least patron creation. The patron is created now but I am missing the borrower_relationships. Also note that if I do not fill the relationship, the form crashes on an invalid relation.
NOTE: I left the original patches, but the last three patches should do the tric.
> We are not there yet unfortunately. > > There is still an issue with at least patron creation. The patron is created > now but I am missing the borrower_relationships. I'll work on it on Monday > Also note that if I do not fill the relationship, the form crashes on an > invalid relation. This is not the scope of this bug I guess, it should be solved by BZ37528
(In reply to Marcel de Rooy from comment #39) > We are not there yet unfortunately. > > There is still an issue with at least patron creation. The patron is created > now but I am missing the borrower_relationships. Hum.. I could not reproduce, I upgraded to the most recent main but I could not manage to have an unfilled borrowers_relationship when I My test plan: 1 - Editing one Patron (category patron) to add Edna Acosta as father -> does work 2 - Creating a new patron (category patron), select Edna Acosta as father in the form -> does work Could you please provide your test plan so that I can try and reproduce ? :)
Since Marcel squashed many patches here I obsoleted the ones we don't need to make it less confusing.
(In reply to Baptiste Wojtkowski (bwoj) from comment #42) > (In reply to Marcel de Rooy from comment #39) > > We are not there yet unfortunately. > > > > There is still an issue with at least patron creation. The patron is created > > now but I am missing the borrower_relationships. > > Hum.. I could not reproduce, I upgraded to the most recent main but I could > not manage to have an unfilled borrowers_relationship when I > > My test plan: > 1 - Editing one Patron (category patron) to add Edna Acosta as father -> > does work > 2 - Creating a new patron (category patron), select Edna Acosta as father in > the form -> does work > > Could you please provide your test plan so that I can try and reproduce ? :) I was also unable to reproduce this.
> > Also note that if I do not fill the relationship, the form crashes on an > > invalid relation. > > This is not the scope of this bug I guess, it should be solved by BZ37528 Ah, yes. I think we should make Bug 37528 a dependency of this one. I do have a question on that bug. It would be nice to move these along together.
Since there is no functional dependency and they only cover separate regressions (?) introduced by recent changes in Borrower object, I'm not a great fan of setting a dependency relationship between both bugs. In particular, imo this patch makes sense by itself and should be pushed to main as soon as it passes QA, even if it does not solve the whole problem of guarantors.
From what I can tell this is FQA for 2 reasons: 1. There is still an issue with at least patron creation. The patron is created now but I am missing the borrower_relationships. - I can't recreate this. Seems like Baptiste cannot either. 2. Also note that if I do not fill the relationship, the form crashes on an invalid relation. - Work is being done a seperate bug to deal with this. Let's set this back to NQA as it is important to get this fix through ASAP.
Marcel, will you come back to this one?
Created attachment 172896 [details] [review] Bug 37892: Fix guarantor restriction, add tests [SQUASHED IN QA] These patches will alter the checks for a patron that prevent a category with 'can_be_guarantee' from being a guarantor. Two patrons in the same category should be allowed to have a guarantee/guarantor relationship The tests below assume you are using the KTD sample data. Update borrowernumbers if not. To test: 0 - Apply tests patch 1 - Set the 'Patron' category as 'Can be a guarantee' 2 - Add a relationship between two patrons of the same category This is restricted from the staff interface perl -e 'use Koha::Patrons; my $p = Koha::Patrons->find(5)->add_guarantor({ guarantor_id => 23, relationship => 'father'});' 3 - Note there is no warning or exception. This should be allowed. 4 - Checkout an item to Edna (borrowernumber 5) 5 - Set 'TrackLastPatronActivityTriggers' to 'Checking in an item' 6 - Try to check the item in, KABOOM 7 - Set 'TrackLastPatronActivityTriggers' to 'Checking out an item' 8 - Try to issue an item to Enda, KABOOM 9 - prove -v t/db_dependent/Koha/Patron.t, fail 10 - Apply second patch 11 - prove -v t/db_dependent/Koha/Patron.t, one more test passes, but then fail 12 - Apply third patch 13 - prove -v t/db_dependent/Koha/Patron.t, pass! 14 - restart_all 15 - Checkout to Enda, OK! 16 - Checkin from Edna, OK! 17 - Find two more patrons in the category and attempt to link them 18 - 'Guarantor cannot be a guarantee' 19 - Apply fourth patch 20 - You can add a guarantor from the same category in interface 21 - Try to add a guarantor to the guarantor assigned in 20 22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee" Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Bug 37892: Fix patron updates Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Bug 37892: Fix patron creation Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Bug 37892: Fix memberentry.pl Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com> Bug 37892: (follow-up) Fix patron creation This patch fixes the 22. of the test plan (22 - Confirm you cannot add a guarantor - "Guarantor cannot be a guarantee" ) Signed-off-by: Olivier V <olivier.vezina@inLibro.com> Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Bug 37892: (follow-up) Fix patron creation TEST PLAN: 1 - Do the 22 parts of the test plan 2 - Add a guarantor to one patron not selected before (let's say A is the guarantee, B the guarantor) 3 - Try and add a guarantor to B -> you will success 4 - Remove B's guarantor 5 - Apply this patch 6 - Repeat 3 -> you will not be able to Signed-off-by: Brendan Lawlor <blawlor@clamsnet.org> Bug 37892: (follow-up) Tidyness Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> [EDIT] Renamed a subtest to patron creation tests in Patron.t. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 172897 [details] [review] Bug 37892: (QA follow-up) Add tests for ->is_guarantor/ee Test plan: Run t/db_dependent/Koha/Patron.t Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Created attachment 172898 [details] [review] Bug 37892: (QA follow-up) Fix condition in memberentry Patron may be undefined. So the test may crash. Also there is an issue with operator precedence. Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Clear fix of a regression, Works as described and makes sense atop it's dependencies. Passing QA
(In reply to Lucas Gass (lukeg) from comment #48) > Marcel, will you come back to this one? Guess, i am too late :)
Pushed for 24.11! Well done everyone, thank you!
Any chance this can be back ported to 24.05? Your consideration is greatly appreciated.
Backported to 24.05.x for upcoming 24.05.05
(In reply to Lucas Gass (lukeg) from comment #56) > Backported to 24.05.x for upcoming 24.05.05 I don't know what happened here, but this did NOT get in for 24.05.05. I will mark it as a backport candidate for 24.05.06. Sorry for the confusion.
Backported to 24.05.x for upcoming 24.05.06. Sorry for the delay!
Depends on Bug 37528 currently only in main/24.11 ?