Bug 12914 - Wrong message 'Patron(..) is blocked for 2014-09-30 day(s).
Summary: Wrong message 'Patron(..) is blocked for 2014-09-30 day(s).
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor (vote)
Assignee: Marc Véron
QA Contact: Testopia
URL:
Keywords:
Depends on: 643
Blocks:
  Show dependency treegraph
 
Reported: 2014-09-12 16:51 UTC by Marc Véron
Modified: 2015-06-04 23:32 UTC (History)
6 users (show)

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


Attachments
Bug 12914 - Wrong message 'Patron(..) is blocked for 2014-09-30 day(s). (3.17 KB, patch)
2014-09-13 10:58 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED OFF] Bug 12914 - Wrong message 'Patron(..) is blocked for 2014-09-30 day(s). (3.23 KB, patch)
2014-10-05 03:36 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
This Screenshots shows the message "Patron has had overdue items and is blocked for 2014-10-31 day(s)." (49.95 KB, image/png)
2014-10-12 09:10 UTC, Marc Véron
Details
Bug 12914 - Wrong message 'Patron(..) is blocked for 2014-09-30 day(s). (3.30 KB, patch)
2014-10-14 14:55 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 12914 - Wrong message 'Patron(..) is blocked for 2014-09-30 day(s). (3.27 KB, patch)
2014-10-14 15:05 UTC, Marc Véron
Details | Diff | Splinter Review
Screenshot showing messages when overriding debarment (40.58 KB, image/png)
2014-10-16 12:49 UTC, Katrin Fischer
Details
[PASSED QA] Bug 12914 - Wrong message 'Patron(..) is blocked for 2014-09-30 day(s). (3.32 KB, patch)
2014-10-17 14:29 UTC, Kyle M Hall
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marc Véron 2014-09-12 16:51:13 UTC
If a patron has a manual restriction, /cgi-bin/koha/circ/circulation.pl can display a message similar to the following:

Patron has had overdue items and is blocked for 2014-09-30 day(s).

This is related to sub IsMemberBlocked in C4/Members.pm that returns a date but is expected to return a number of days. It should have an additional status for returning dates instead of days to use in C4/Circulation.pm and circulation.tt
Comment 1 Marc Véron 2014-09-13 10:58:30 UTC Comment hidden (obsolete)
Comment 2 Marc Véron 2014-09-13 11:21:11 UTC
Test plan

Apply patch on top of Bug 643

Add manual restriction with expiration date to patron A
Add manual restriction without expiration date to patron B
For both try to check out an item and override restriction (Bug 643)

The message for patron A should now be similar to:
Patron has a restriction until 04/12/2014.

The message for patron B should now read:
Patron has a restriction (no expiry date).
Comment 3 Nick Clemens (kidclamp) 2014-10-05 03:36:42 UTC Comment hidden (obsolete)
Comment 4 Kyle M Hall 2014-10-10 12:40:05 UTC
I cannot reproduce this bug on master with bug 643 applied. I think perhaps the wording of the message has changed to correct this already.
Comment 5 Marc Véron 2014-10-12 09:10:57 UTC
Created attachment 32283 [details]
This Screenshots shows the message "Patron has had overdue items and is blocked for 2014-10-31 day(s)."

I did a new test on curret master 3.17.00.027
The test was done after applying patches for bug 643. The message appears after checking out to a debarred patron using "Override debarment temorarily".

Without patch for Bug 12914 the message reads:
 "Patron has had overdue items and is blocked for 2014-10-31 day(s)."

With patch, it reads: "Patron has a restriction until 31/10/2014."

The patch was signed off by Nick Clemens. I kindly ask to switch back status from "Failed QA" to "Signed off"
Comment 6 Katrin Fischer 2014-10-14 14:02:52 UTC
Hi Marc, looking at this patch I think it should not be using format_date but the Dates template plugin instead. Could you provide a new patch? As this won't change the logic you can set it back to 'signed off'.
Comment 7 Marc Véron 2014-10-14 14:32:18 UTC
Hi Katrin,

I had a look at C4/Circulation.pm

There are a more calls to the function format_date (that's why I followed this pattern).

I suppose that those calls should be replaced as well? If yes, I propose to move the replacement to a separate bug.
Comment 8 Katrin Fischer 2014-10-14 14:33:37 UTC
You are right that the other ones should be removed as well, but I'd like to not introduce new ones if possible :)
Comment 9 Marc Véron 2014-10-14 14:55:36 UTC Comment hidden (obsolete)
Comment 10 Marc Véron 2014-10-14 14:57:08 UTC
Comment on attachment 32322 [details] [review]
Bug 12914 - Wrong message 'Patron(..) is blocked for 2014-09-30 day(s).

Oh, something went wront with this patch.
Comment 11 Marc Véron 2014-10-14 15:05:21 UTC Comment hidden (obsolete)
Comment 12 Marc Véron 2014-10-14 15:07:14 UTC
Setting back to Signed-off (see comment #6)
Comment 13 Katrin Fischer 2014-10-16 12:45:43 UTC
Hi Marc,

I can't reproduce what I can see in your screenshot. The messages I get look quite different and don't include the date (which they should?). I am attaching a screenshot, could you take a look? How are you setting the restriction?
I have tested on top of bug 643.
Comment 14 Katrin Fischer 2014-10-16 12:49:01 UTC
Created attachment 32403 [details]
Screenshot showing messages when overriding debarment
Comment 15 Marc Véron 2014-10-16 13:08:43 UTC
Hi Katrin,

It seems that my test plan was not precise enough, sorry...

It should read:

Add manual restriction with expiration date to patron A
Add manual restriction without expiration date to patron B

For both:

Step a)
go to check out 

Step b)
override restriction (Bug 643)
Result of this step: like screenshot 2014-10-16 by Katrin

Step c)
check out an item 
Result: like Marc's screenshot 2014-10-14 with wrong text. This is fixed by the patch.


The message for patron A should now be similar to:
Patron has a restriction until 04/12/2014.

The message for patron B should now read:
Patron has a restriction (no expiry date).
Comment 16 Kyle M Hall 2014-10-17 14:29:11 UTC
Created attachment 32506 [details] [review]
[PASSED QA] Bug 12914 - Wrong message 'Patron(..) is blocked for 2014-09-30 day(s).

The message in circulation.tt assumed to get days but date is given. Updated comments and message depending on expiration date or no expiration of restriction.

The message shows up on top of Bug 643 Allow override of 'debarred' status if a patron has a restriction.

Replaced date_format with date template (see comment #6)

Signed-off-by: Nick Clemens <nick@quecheelibrary.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 17 Kyle M Hall 2014-10-17 14:30:35 UTC
Looks good! I was also looking in the wrong place when I marked as failed qa.
Comment 18 Tomás Cohen Arazi 2014-10-21 19:18:27 UTC
Patch pushed to master.

Thanks Marc!

Note: It feels it's time to refactor CanBookBeIsued.