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

--- Comment #4 from Jonathan Druart <[email protected]> ---
(In reply to Baptiste Wojtkowski (bwoj) from comment #3)
> Hi,
> Working on it but I have a few questions.
> Concerning point 1, I do not think it is related to the bug since it does
> not concern the same part of the code.

Indeed, it could be moved to a separate bug report.

> Concerning point 2
> Since we now always submit the old relationship to the user, it appears it
> is detected as a duplicate and an error is thrown while storing the object. 
> 
> I'm wondering why we throw an exception here since storing an existing
> relationship cannot have impact on data ? Could the fix be removing the
> exception thrown ? Maybe I can rework this part of the code to use no
> try/catch.

You cannot remove the exception, it is at the DB level:
  UNIQUE KEY `guarantor_guarantee_idx` (`guarantor_id`,`guarantee_id`),

It ensures that we won't have a guarantor with different relationships on a
given guarantee.

All this code is weird to be honest, Koha::Patron->store takes guarantors in
parameters but does not update them. We are passing the existing guarantors,
(even if deleted!) and the new ones.

We are removing and adding guarantors before and after the store. So if the
store fails we have already removed the guarantors (you will be prompted with a
"cannot save" error, but cancelling at this point will be confusing).


Anyway, it seems that the logic is flaw and the code is buggy.

What I would do is to simply test (in the controller, sub add_guarantors) if
the guarantor already exists before trying to insert it.

-- 
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