Bug 17522 - opac-user.pl gives error of OpacRenewalAllowed is enabled
Summary: opac-user.pl gives error of OpacRenewalAllowed is enabled
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low blocker
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
: 17517 (view as bug list)
Depends on: 14610 17579 17604
Blocks:
  Show dependency treegraph
 
Reported: 2016-10-31 14:06 UTC by Kyle M Hall (khall)
Modified: 2019-06-27 09:24 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:


Attachments
Bug 17522 - opac-user.pl gives error of OpacRenewalAllowed is enabled (6.87 KB, patch)
2016-10-31 14:07 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 17522 - opac-user.pl gives error of OpacRenewalAllowed is enabled (9.56 KB, patch)
2016-11-01 15:43 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 17522 - opac-user.pl gives error of OpacRenewalAllowed is enabled (9.57 KB, patch)
2016-11-01 15:44 UTC, Kyle M Hall (khall)
Details | Diff | Splinter Review
Bug 17522 - opac-user.pl gives error of OpacRenewalAllowed is enabled (9.79 KB, patch)
2016-11-02 13:57 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 17522: Fix opac-user.pl if OpacRenewalAllowed is set (5.35 KB, patch)
2016-11-10 08:52 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 17522: Fix opac-user.pl if OpacRenewalAllowed is set (5.40 KB, patch)
2016-11-10 09:40 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 17522: Fix opac-user.pl if OpacRenewalAllowed is set (5.47 KB, patch)
2016-11-15 10:06 UTC, Josef Moravec
Details | Diff | Splinter Review
Bug 17522: Fix opac-user.pl if OpacRenewalAllowed is set (5.51 KB, patch)
2016-11-15 15:48 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Kyle M Hall (khall) 2016-10-31 14:06:08 UTC
If OpacRenewalAllowed is enabled, the following error message is displayed:

Template process failed: undef error - The method is_expired is not covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.
Comment 1 Kyle M Hall (khall) 2016-10-31 14:07:36 UTC
Created attachment 56999 [details] [review]
Bug 17522 - opac-user.pl gives error of OpacRenewalAllowed is enabled

If OpacRenewalAllowed is enabled, the following error message is
displayed:

Template process failed: undef error - The method is_expired is not
covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.

Test Plan:
1) Check out an item for a patron
2) Enable OpacRenewalAllowed
3) Log in as that patron, go to opac-user.pl
4) Note the error
5) Apply this patch
6) Reload opac-user.pl
7) Page should now load!
Comment 2 Katrin Fischer 2016-11-01 14:28:34 UTC
*** Bug 17517 has been marked as a duplicate of this bug. ***
Comment 3 Katrin Fischer 2016-11-01 14:30:25 UTC
I think the patch is not correct:

 [% IF ( canrenew && !userdebarred && OpacRenewalAllowed && !( borrower.is_expired && Koha.Preference('BlockExpiredPatronOpacActions') ) ) %] will only check the preference, but this works differently. The preference is only the default, that can be overwritten by patron category.
Comment 4 Kyle M Hall (khall) 2016-11-01 15:43:01 UTC
Created attachment 57047 [details] [review]
Bug 17522 - opac-user.pl gives error of OpacRenewalAllowed is enabled

If OpacRenewalAllowed is enabled, the following error message is
displayed:

Template process failed: undef error - The method is_expired is not
covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.

Test Plan:
1) Check out an item for a patron
2) Enable OpacRenewalAllowed
3) Log in as that patron, go to opac-user.pl
4) Note the error
5) Apply this patch
6) Reload opac-user.pl
7) Page should now load!
Comment 5 Kyle M Hall (khall) 2016-11-01 15:44:07 UTC
Created attachment 57048 [details] [review]
Bug 17522 - opac-user.pl gives error of OpacRenewalAllowed is enabled

If OpacRenewalAllowed is enabled, the following error message is
displayed:

Template process failed: undef error - The method is_expired is not
covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.

Test Plan:
1) Check out an item for a patron
2) Enable OpacRenewalAllowed
3) Log in as that patron, go to opac-user.pl
4) Note the error
5) Apply this patch
6) Reload opac-user.pl
7) Page should now load!
Comment 6 Jonathan Druart 2016-11-02 13:26:07 UTC
Comment on attachment 57048 [details] [review]
Bug 17522 - opac-user.pl gives error of OpacRenewalAllowed is enabled

Review of attachment 57048 [details] [review]:
-----------------------------------------------------------------

::: Koha/Patron.pm
@@ +467,5 @@
> +=head3 should_block_expired_patron_opac_actions
> +
> +=cut
> +
> +sub should_block_expired_patron_opac_actions {

It sounds to me that this method should actually override Koha::Patron::Category->BlockExpiredPatronOpacActions, don't you think?
Comment 7 Marc Véron 2016-11-02 13:57:48 UTC
Created attachment 57102 [details] [review]
Bug 17522 - opac-user.pl gives error of OpacRenewalAllowed is enabled

If OpacRenewalAllowed is enabled, the following error message is
displayed:

Template process failed: undef error - The method is_expired is not
covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.

Test Plan:
1) Check out an item for a patron
2) Enable OpacRenewalAllowed
3) Log in as that patron, go to opac-user.pl
4) Note the error
5) Apply this patch
6) Reload opac-user.pl
7) Page should now load!

Problem reproduced, patch fixes it. Signing off to move it forward.
However see comment #6 for QA
Signed-off-by: Marc Véron <veron@veron.ch>
Comment 8 Jonathan Druart 2016-11-04 08:54:26 UTC
(In reply to Jonathan Druart from comment #6)
> Comment on attachment 57048 [details] [review] [review]
> Bug 17522 - opac-user.pl gives error of OpacRenewalAllowed is enabled
> 
> Review of attachment 57048 [details] [review] [review]:
> -----------------------------------------------------------------
> 
> ::: Koha/Patron.pm
> @@ +467,5 @@
> > +=head3 should_block_expired_patron_opac_actions
> > +
> > +=cut
> > +
> > +sub should_block_expired_patron_opac_actions {
> 
> It sounds to me that this method should actually override
> Koha::Patron::Category->BlockExpiredPatronOpacActions, don't you think?

This is waiting for an answer.
Comment 9 Jonathan Druart 2016-11-10 08:18:55 UTC
Also note that Koha::Patron->is_expired is added by bug 17579. It is much more robust and covered by tests than the one provided in this patch.
Comment 10 Jonathan Druart 2016-11-10 08:20:25 UTC
(In reply to Jonathan Druart from comment #8)
> (In reply to Jonathan Druart from comment #6)
> > It sounds to me that this method should actually override
> > Koha::Patron::Category->BlockExpiredPatronOpacActions, don't you think?
> 
> This is waiting for an answer.

This has been done on bug 17578.
Comment 11 Katrin Fischer 2016-11-10 08:24:30 UTC
Jonathan, what are your suggested next steps? Not sure I understand the different dependencies.
Comment 12 Jonathan Druart 2016-11-10 08:49:30 UTC
Caused by bug 14610.

I am going to propose a way to fix this.
Comment 13 Jonathan Druart 2016-11-10 08:52:13 UTC
Created attachment 57414 [details] [review]
Bug 17522: Fix opac-user.pl if OpacRenewalAllowed is set

This patch fixes a bug added by commit 81a04af27804621609004dc370284413baa5caa2
    Bug 14610 - Add and update scripts

If OpacRenewalAllowed is enabled, the following error message is
displayed:

Template process failed: undef error - The method is_expired is not
covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.

The is_expired and BlockExpiredPatronOpacActions methods did not exist
for the Koha::Patron object. These 2 values were returned by
GetMemberDetails.

To fix this, we need bug 17579 which introduces the
Koha::Patron->is_expired method and also bug 17604 for the
Koha::Patron->effective_BlockExpiredPatronOpacActions.

Test Plan:
1) Check out an item for a patron
2) Enable OpacRenewalAllowed
3) Log in as that patron, go to opac-user.pl
4) Note the error
5) Apply this patch
6) Reload opac-user.pl
7) Page should now load!
Comment 14 Jonathan Druart 2016-11-10 08:53:40 UTC
% git log --oneline
25b6a92 Bug 17522: Fix opac-user.pl if OpacRenewalAllowed is set
2da5f21 Bug 17604: GetMemberDetails - Add Koha::Patron::Category->effective_BlockExpiredPatronOpacActions
2c69552 Bug 17555: Add Koha::Patron->category
aafa890 Bug 17579: Make sure we are testing the real life
ea99ecd Bug 17579: Add the Koha::Patron->is_expired
56baeb1 Bug 17594: Make Koha::Object->discard_changes available
Comment 15 Marcel de Rooy 2016-11-10 09:11:51 UTC
Koha::Patron->effective_BlockExpiredPatronOpacActions ?
Nice ;)
Comment 16 Jonathan Druart 2016-11-10 09:40:41 UTC
Created attachment 57415 [details] [review]
Bug 17522: Fix opac-user.pl if OpacRenewalAllowed is set

This patch fixes a bug added by commit 81a04af27804621609004dc370284413baa5caa2
    Bug 14610 - Add and update scripts

If OpacRenewalAllowed is enabled, the following error message is
displayed:

Template process failed: undef error - The method is_expired is not
covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.

The is_expired and BlockExpiredPatronOpacActions methods did not exist
for the Koha::Patron object. These 2 values were returned by
GetMemberDetails.

To fix this, we need bug 17579 which introduces the
Koha::Patron->is_expired method and also bug 17604 for the
Koha::Patron->effective_BlockExpiredPatronOpacActions.

Test Plan:
1) Check out an item for a patron
2) Enable OpacRenewalAllowed
3) Log in as that patron, go to opac-user.pl
4) Note the error
5) Apply this patch
6) Reload opac-user.pl
7) Page should now load!
Comment 17 Jonathan Druart 2016-11-10 09:41:44 UTC
(In reply to Marcel de Rooy from comment #15)
> Koha::Patron->effective_BlockExpiredPatronOpacActions ?
> Nice ;)

Oops, it's Koha::Patron::Category->effective_BlockExpiredPatronOpacActions indeed.
I am tired of dependencies...
Comment 18 Josef Moravec 2016-11-15 10:06:02 UTC
Created attachment 57478 [details] [review]
[SIGNED-OFF] Bug 17522: Fix opac-user.pl if OpacRenewalAllowed is set

This patch fixes a bug added by commit 81a04af27804621609004dc370284413baa5caa2
    Bug 14610 - Add and update scripts

If OpacRenewalAllowed is enabled, the following error message is
displayed:

Template process failed: undef error - The method is_expired is not
covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.

The is_expired and BlockExpiredPatronOpacActions methods did not exist
for the Koha::Patron object. These 2 values were returned by
GetMemberDetails.

To fix this, we need bug 17579 which introduces the
Koha::Patron->is_expired method and also bug 17604 for the
Koha::Patron->effective_BlockExpiredPatronOpacActions.

Test Plan:
1) Check out an item for a patron
2) Enable OpacRenewalAllowed
3) Log in as that patron, go to opac-user.pl
4) Note the error
5) Apply this patch
6) Reload opac-user.pl
7) Page should now load!

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Comment 19 Tomás Cohen Arazi (tcohen) 2016-11-15 15:48:48 UTC
Created attachment 57505 [details] [review]
Bug 17522: Fix opac-user.pl if OpacRenewalAllowed is set

This patch fixes a bug added by commit 81a04af27804621609004dc370284413baa5caa2
    Bug 14610 - Add and update scripts

If OpacRenewalAllowed is enabled, the following error message is
displayed:

Template process failed: undef error - The method is_expired is not
covered by tests! at /home/vagrant/kohaclone/C4/Templates.pm line 121.

The is_expired and BlockExpiredPatronOpacActions methods did not exist
for the Koha::Patron object. These 2 values were returned by
GetMemberDetails.

To fix this, we need bug 17579 which introduces the
Koha::Patron->is_expired method and also bug 17604 for the
Koha::Patron->effective_BlockExpiredPatronOpacActions.

Test Plan:
1) Check out an item for a patron
2) Enable OpacRenewalAllowed
3) Log in as that patron, go to opac-user.pl
4) Note the error
5) Apply this patch
6) Reload opac-user.pl
7) Page should now load!

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 20 Kyle M Hall (khall) 2016-11-15 15:50:06 UTC
Pushed to master for 16.11, thanks Jonathan!
Comment 21 Mason James 2016-12-08 03:04:27 UTC
Blocked by new-feature, skipping for 16.05.x