Bug 16909

Summary: Koha::Patrons - Remove checkuniquemember
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: enhancement    
Priority: P5 - low CC: kyle, philippe.blouin, veron
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 16846, 16941, 19827    
Attachments: Bug 16909: Koha::Patrons - Remove checkuniquemember
Bug 16909: Koha::Patrons - Remove checkuniquemember
Bug 16909: Koha::Patrons - Remove checkuniquemember
Bug 16909: Koha::Patrons - Remove checkuniquemember

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.