Bug 15135

The plugin use a C4::Members subroutine, you should not remove this line.

(In reply to Jonathan Druart from comment #3)
> The plugin use a C4::Members subroutine, you should not remove this line.

Yes, you are right,there is a soubroutine called in line 57:
return C4::Members::HasOverdues($borrowernumber);

Because of the way it is called I supposed it is save to remove line 26:
 	
use C4::Members qw(HasOverdues);

My mistake if that is wrong. 

What would be an alternative to get rid of the warning?

(In reply to Marc Véron from comment #4)
> What would be an alternative to get rid of the warning?

Rename the subroutine or
  no warnings 'redefine';
But the no warnings is not a good solution.

Kyle, it's not a good idea to replace use with require, you are skipping the compilation pass.

New solution with renaming, see comment #5 and bug 12933 comment #67.

Switching back to NSO

Marc,
The problem is that we have the same issue with IsDebarred.
The current code tricks us using not exporting the IsDebarred method:

  use Koha::Borrower::Debarments qw();

Either we continue to trick:

  -use C4::Members qw(HasOverdues);
  +use C4::Members qw();

Or we fix both.
But I am pretty sure that introducing another workaround is a good idea.

I would like to get Martin and Kyle POV here.

(In reply to Jonathan Druart from comment #11)

I should read before clicking the submit button...

> The current code tricks us using not exporting the IsDebarred method:

s/using//

> But I am pretty sure that introducing another workaround is a good idea.

is *not* a good idea

Sorry, I have no other ideas.
Maybe someone else finds a solution.

I agree adding another workaround is bad, doing it 'properly' is better.

So, am I reading it right that basically we also need IsDebarred also renaming and then this patch could stand.. or have I misunderstood something?

(In reply to Martin Renvoize from comment #14)
> I agree adding another workaround is bad, doing it 'properly' is better.
> 
> So, am I reading it right that basically we also need IsDebarred also
> renaming and then this patch could stand.. or have I misunderstood something?

No, you got it.
Renaming them could work, but otoh it would make sense to keep the same name: the subroutines only call the corresponding subroutine/method.

If I understood correctly, the problem is that we now have Borrowers::HasOverdues and Members::HasOverdues and similar for IsDebarred.
Looking at the Borrowers plugin, not importing Overdues from Members there, resolves the warning for now and is consistent with the IsDebarred case. So I agree with Jonathan.

Marc: Could you adjust your patch?

Too bad btw that TT forces us to create a stub for each Koha routine we want to call in the templates. (Exception for OO modules as below)

From the TT docs:
===
If a regular Perl module supports an object-oriented interface and a new() constructor then it can be loaded and instantiated automatically like:
[% USE file = IO.File('/tmp/mydata') %]
===

(In reply to Marcel de Rooy from comment #16)
> If I understood correctly, the problem is that we now have
> Borrowers::HasOverdues and Members::HasOverdues and similar for IsDebarred.
> Looking at the Borrowers plugin, not importing Overdues from Members there,
> resolves the warning for now and is consistent with the IsDebarred case. So
> I agree with Jonathan.
> 
> Marc: Could you adjust your patch?
> 
> Too bad btw that TT forces us to create a stub for each Koha routine we want
> to call in the templates. (Exception for OO modules as below)
> 
> From the TT docs:
> ===
> If a regular Perl module supports an object-oriented interface and a new()
> constructor then it can be loaded and instantiated automatically like:
> [% USE file = IO.File('/tmp/mydata') %]
> ===

"...not importing Overdues from Members there..."
Sorry, I lost track here (Ik ben the kluts kwijt). Wasn't that what I did in my very first patch?

(In reply to Marc Véron from comment #17)
> (In reply to Marcel de Rooy from comment #16)
> > If I understood correctly, the problem is that we now have
> > Borrowers::HasOverdues and Members::HasOverdues and similar for IsDebarred.
> > Looking at the Borrowers plugin, not importing Overdues from Members there,
> > resolves the warning for now and is consistent with the IsDebarred case. So
> > I agree with Jonathan.
> > 
> > Marc: Could you adjust your patch?
> > 
> > Too bad btw that TT forces us to create a stub for each Koha routine we want
> > to call in the templates. (Exception for OO modules as below)
> > 
> > From the TT docs:
> > ===
> > If a regular Perl module supports an object-oriented interface and a new()
> > constructor then it can be loaded and instantiated automatically like:
> > [% USE file = IO.File('/tmp/mydata') %]
> > ===
> 
> "...not importing Overdues from Members there..."
> Sorry, I lost track here (Ik ben the kluts kwijt). Wasn't that what I did in
> my very first patch?

No you removed the use completely there.
You only should say: use Module qw();
In that case you do not import any routines in the current namespace.

Not quit sure, should that go back to "Needs Signoff"?

(In reply to Marc Véron from comment #20)
> Not quit sure, should that go back to "Needs Signoff"?

I will add a signoff

This patch now received enough attention to pass QA.

Created attachment 46481 [details] [review]
Bug 15135: Remove Warning Subroutine HasOverdues redefined

In staff client, while displying patron related pages (e.g. moremember.pl) you get a warning like:
moremember.pl: Subroutine HasOverdues redefined at /usr/share/kohaclone/Koha/Template/Plugin/Borrowers.pm line 52.
This occurs with all pages that have [% USE Borrowers %] in their templates (directly or via members-toolbar.inc)

To test:
- Go to Home > Patrons > Patron details for....
- Search for warning above in log
- Apply patch
- Verify that no more warnings appear
- Verify that patron pages behave as before

(Solution as of comment #18)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified that warning disappears with this patch.

Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>

Pushed to Master - Should be in the May 2016 Release. Thanks!

Patch pushed to 3.22.x, will be in 3.22.3