Bug 16909 - Koha::Patrons - Remove checkuniquemember
Summary: Koha::Patrons - Remove checkuniquemember
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks: 16846 16941 19827
  Show dependency treegraph
 
Reported: 2016-07-12 18:15 UTC by Jonathan Druart
Modified: 2017-12-18 14:43 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 16909: Koha::Patrons - Remove checkuniquemember (4.37 KB, patch)
2016-07-12 18:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16909: Koha::Patrons - Remove checkuniquemember (4.33 KB, patch)
2016-07-12 19:19 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 16909: Koha::Patrons - Remove checkuniquemember (4.46 KB, patch)
2016-07-12 20:59 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 16909: Koha::Patrons - Remove checkuniquemember (4.51 KB, patch)
2016-07-15 15:34 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2016-07-12 18:15:13 UTC
C4::Members::checkuniquemember was not really nicely written, was only used once and was not covered by tests.
I think it does not make sense to keep such complexity and have this code in the subroutine/method.
Comment 1 Jonathan Druart 2016-07-12 18:31:37 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2016-07-12 19:05:39 UTC
Hi Jonathan,

If a duplicate entry is found, the link "View existing record" and the button "It's a duplicate" always redirect to a borrowernumber=1 and not to the borrowernumber found.
Comment 3 Jonathan Druart 2016-07-12 19:19:59 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2016-07-12 19:21:01 UTC
(In reply to Marc Véron from comment #2)
> Hi Jonathan,
> 
> If a duplicate entry is found, the link "View existing record" and the
> button "It's a duplicate" always redirect to a borrowernumber=1 and not to
> the borrowernumber found.

Yes good catch. I can now remove the FIXME I added, I understand the 2 variables :)
Comment 5 Marc Véron 2016-07-12 20:59:09 UTC Comment hidden (obsolete)
Comment 6 Tomás Cohen Arazi 2016-07-15 15:34:17 UTC
Created attachment 53445 [details] [review]
Bug 16909: Koha::Patrons - Remove checkuniquemember

C4::Members::checkuniquemember was not really nicely written, was only
used once and was not covered by tests.
I think it does not make sense to keep such complexity and have this
code in the subroutine/method.
Looking at this patch it seems that what this subroutine did can be done
easily in the pl script in few lines.

Test plan:
1/ Create 2 organisations with the same "surname": you should get a
warning.
2/ Create 2 patrons (non-organisation) with the same
surname/firstname/date of birth, you should get a warning

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Kyle M Hall 2016-07-15 18:15:15 UTC
Pushed to master for 16.11, thanks Jonathan!
Comment 8 Blou 2016-07-21 13:56:05 UTC
Wrote to bug 16889 by mistake.  Twice.

This patch broke add new member.  When pressing save, we cycle back to the page.

Not sure what to do with the status and importance, but put in "blocker" since, well, it is kind of a big problem.
Comment 9 Marc Véron 2016-07-21 15:13:17 UTC
(In reply to Blou from comment #8)
> Wrote to bug 16889 by mistake.  Twice.
> 
> This patch broke add new member.  When pressing save, we cycle back to the
> page.
> 
> Not sure what to do with the status and importance, but put in "blocker"
> since, well, it is kind of a big problem.

See Bug 16941 - Can not add new patron in staff client for the solution.