While working on Bug 29303 I noticed a small problem. If you add relationship to the PatronQuickAddFields system preference it will be added to the quick form two times. It looks like it is due to some JS that is no longer needed: if ( $("#memberentry_guarantor").length ) { $("#memberentry_guarantor").appendTo("#quick_add_form"); }
This is still a bug but I am completely wrong about the JS I mentioned. It is not the cause.
If you add relationship to the QuickAddFields, go to the quick add form, and press "+ Add guarantor" the relationship field will appear 3 times.
Still valid.
The problem here seems to be that we have 2 ways of adding the Relationship field. 1. If borrowerRelationship is NOT empty we assume the field should be added. 2. It can also be added via PatronQuickAddFields I think we need to remove the logic that automatically adds the field if borrowerRelationship is NOT empty: 104 my @relations = split /\|/, C4::Context->preference('borrowerRelationship'), -1; 105 @relations = ('') unless @relations; 106 my $empty_relationship_allowed = grep {$_ eq ""} @relations; 107 $template->param( empty_relationship_allowed => $empty_relationship_allowed ); We can rely on BorrowerUnwantedField, BorrowerMandatoryField, and PatronQuickAddFields to setup this field in the desired way.
Created attachment 162461 [details] [review] Bug 30987: Fix relationship fields for patron full and quick add forms To test: 1. APPLY PATACH and restart services. 2. Find the borrowerRelationship system preference. The description should no longer include the words "Leave empty to deactivate." 3. Populate the system preference with at least 1 choice. 4. Find a patron category with can_be_guarantee set to 'Yes'. 5. Quick add a patron of that type, making sure the relationship field shows in the Patron guarantor section. ( You have to +Add gaurantor before this field will show ) 6. The values in the dropdown should refelct the borrowerRelationship values. 7. With BorrowerMandatoryField make relationship mandatory. 8. Try step 5 again, this time the Relationship field should be mandatory. 9. Remove the field from BorrowerMandatoryField and add it to BorrowerUnwantedField. 10. Do step 5 again, the relationship field should not show on the quick add form.
Created attachment 162860 [details] [review] Bug 30987: Fix relationship fields for patron full and quick add forms To test: 1. APPLY PATACH and restart services. 2. Find the borrowerRelationship system preference. The description should no longer include the words "Leave empty to deactivate." 3. Populate the system preference with at least 1 choice. 4. Find a patron category with can_be_guarantee set to 'Yes'. 5. Quick add a patron of that type, making sure the relationship field shows in the Patron guarantor section. ( You have to +Add gaurantor before this field will show ) 6. The values in the dropdown should refelct the borrowerRelationship values. 7. With BorrowerMandatoryField make relationship mandatory. 8. Try step 5 again, this time the Relationship field should be mandatory. 9. Remove the field from BorrowerMandatoryField and add it to BorrowerUnwantedField. 10. Do step 5 again, the relationship field should not show on the quick add form. Signed-off-by: Myka Kennedy Stephens <mkstephens@fosgail.org>
Patch works as described. Great work!
Hm there is also an additional issue here: the one that is there first always appears as mandatory. QA please? :)
Created attachment 165005 [details] [review] Bug 30987: Fix relationship fields for patron full and quick add forms To test: 1. APPLY PATACH and restart services. 2. Find the borrowerRelationship system preference. The description should no longer include the words "Leave empty to deactivate." 3. Populate the system preference with at least 1 choice. 4. Find a patron category with can_be_guarantee set to 'Yes'. 5. Quick add a patron of that type, making sure the relationship field shows in the Patron guarantor section. ( You have to +Add gaurantor before this field will show ) 6. The values in the dropdown should refelct the borrowerRelationship values. 7. With BorrowerMandatoryField make relationship mandatory. 8. Try step 5 again, this time the Relationship field should be mandatory. 9. Remove the field from BorrowerMandatoryField and add it to BorrowerUnwantedField. 10. Do step 5 again, the relationship field should not show on the quick add form. Signed-off-by: Myka Kennedy Stephens <mkstephens@fosgail.org> Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Good fix, works as described and QA scripts happy. Passing QA
How do we deactivate the feature if leaving it empty is no longer an option? --> I guess we are referring to the *Unwanted* prefs then? What happens with libraries who left it empty, do we need to handle that somehow?
As we check the category in order to display the relationship or not, should we remove "relationship" as an option from PatronQuickAddFields?
Pushed for 24.05! Well done everyone, thank you!
(In reply to Katrin Fischer from comment #12) > As we check the category in order to display the relationship or not, should > we remove "relationship" as an option from PatronQuickAddFields? *ping*
(In reply to Katrin Fischer from comment #12) > As we check the category in order to display the relationship or not, should > we remove "relationship" as an option from PatronQuickAddFields? In my opinion, yes.
(In reply to Lucas Gass from comment #15) > (In reply to Katrin Fischer from comment #12) > > As we check the category in order to display the relationship or not, should > > we remove "relationship" as an option from PatronQuickAddFields? > > In my opinion, yes. Just running out - could you file a bug OR follow-up and remove additional_work_needed? Thanks!
(In reply to Katrin Fischer from comment #16) > (In reply to Lucas Gass from comment #15) > > (In reply to Katrin Fischer from comment #12) > > > As we check the category in order to display the relationship or not, should > > > we remove "relationship" as an option from PatronQuickAddFields? > > > > In my opinion, yes. > > Just running out - could you file a bug OR follow-up and remove > additional_work_needed? Thanks! Filed Bug 36719, add will add a patch asap.
Pushed to 23.11.x for 23.11.06
Backported to 23.05.x for upcoming 23.05.12