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 reproduce: - Go to Home > Patrons > Patron details for.... - Search for warning above in log
Created attachment 44466 [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
Created attachment 44536 [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 Signed-off-by: Frederic Demians <f.demians@tamil.fr> Works as described
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.
Created attachment 45172 [details] [review] Bug 15135 [QA Followup] - Switch from use to require
Kyle, it's not a good idea to replace use with require, you are skipping the compilation pass.
Created attachment 46083 [details] [review] Bug 15135: Get rid of Warning 'Subroutine HasOverdues redefined' This patch renames the subroutine HasOverdues to WithOverdues in Koha/Template/Plugin/Borrowers.pm and in members-toolbar.inc (only place where it is used). To test: - First test without patch - Search for a patron with overdues (I used the 'Overdues' report on page Home > Cirulation - Display detail page, open drop down tool 'Print'. Verify that you have an item 'Print overdues' - Apply patch - Repeat steps above. Verify that the item 'Print overdues' still appears - Search for remaining occurences of HasOverdues in template files / template includes with git grep 'Patrons.HasOverdues'. There should not be any occurences.
New solution with renaming, see comment #5 and bug 12933 comment #67. Switching back to NSO
Created attachment 46166 [details] [review] Bug 15135: Get rid of Warning 'Subroutine HasOverdues redefined' This patch renames the subroutine HasOverdues to WithOverdues in Koha/Template/Plugin/Borrowers.pm and in members-toolbar.inc (only place where it is used). To test: - First test without patch - Search for a patron with overdues (I used the 'Overdues' report on page Home > Cirulation - Display detail page, open drop down tool 'Print'. Verify that you have an item 'Print overdues' - Apply patch - Repeat steps above. Verify that the item 'Print overdues' still appears - Search for remaining occurences of HasOverdues in template files / template includes with git grep 'Patrons.HasOverdues'. There should not be any occurences. Signed-off-by: Chris <chris@bigballofwax.co.nz>
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.
Created attachment 46478 [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)
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
Created attachment 46479 [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.
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