https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=37528

--- Comment #10 from Johanna Räisä <[email protected]> ---
(In reply to Baptiste Wojtkowski (bwoj) from comment #9)
> Hi ! I'm not the author of the patch but I'm quite interesting in helping it
> to pass :)
> 
> 
> I think the reason is in Relationship.pm (code introduced in Bug 14570,
> v19.11). The matter of current patch is to react by a lock in the interface
> to the change and preventing from a dirty crash.
> 
>  47 sub store {
>  48     my ( $self ) = @_;
>  49 
>  50     my @valid_relationships = split /\|/,
> C4::Context->preference('borrowerRelationship'), -1;
>  51     @valid_relationships = ('') unless @valid_relationships;
>  52 
>  53     Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw(
>  54         no_relationship => 1 )
>  55         unless defined $self->relationship;
>  56 
>  57     Koha::Exceptions::Patron::Relationship::InvalidRelationship->throw(
>  58         relationship => $self->relationship )
>  59         unless any { $_ eq $self->relationship } @valid_relationships;
>  60 
>  61     return try {
>  62         $self->SUPER::store;
>  63     }
>  64     catch {
>  65         if ( ref($_) eq 'Koha::Exceptions::Object::DuplicateID' ) {
>  66            
> Koha::Exceptions::Patron::Relationship::DuplicateRelationship->throw(
>  67                 guarantee_id => $self->guarantee_id,
>  68                 guarantor_id => $self->guarantor_id
>  69             );
>  70         }
>  71     };
>  72 }
> 
> In these lines, we can clearly see that "" is invalid unless 
> 1 - it is expressly specified in borrowerRelationship, by adding a "|" at
> the end
> OR 2- thes is nothing in borrowerRelationship. It looks intentional to me
> but I could not find any reference in concerned BZ.
> 
> 
> Forbidding the use of "" might be an undesired behaviour but the 500 error
> is way more an issue. So I see multiple options :
> 1 - We consider this patch is valid and add to the documentation that ""
> must be explicitly allowed (which is my opinion here). We can add a tooltip
> to help librarian configuring it properly (something like "Your
> configuration requires relationships to be filled", linking to syspref, and
> in syspref a text like "finish by '|' to allow leaving it blank").
> 2 - (not sure it can be done tbh) In relationship.pm, We check the content
> of mandatory subfields to check if guarantor relationship and add "" in
> relationships.pm.
> 3 - We consider that forbidding "" by default is a regression and therefore
> this patch is just a workaround for a problem, hence we add "" by default in
> the list of @validRelationships in the aforementionned code.

I think it is a bit unclear how those should be configured to work together.
The systempreference should have more information about the empty value. I had
to go to see the code to figure out that there can be pipe added at the end of
the preference to allow the empty value.

-- 
You are receiving this mail because:
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list
[email protected]
https://lists.koha-community.org/cgi-bin/mailman/listinfo/koha-bugs
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/

Reply via email to