Bug 11565 - decreaseLoanHighHolds needs Override
Summary: decreaseLoanHighHolds needs Override
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Keywords:
Depends on: 14694
Blocks:
  Show dependency treegraph
 
Reported: 2014-01-16 00:12 UTC by Christopher Brannon
Modified: 2017-06-14 22:15 UTC (History)
7 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 11565 - decreaseLoanHighHolds needs Override (9.35 KB, patch)
2015-09-30 16:37 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11565 - decreaseLoanHighHolds needs Override (10.43 KB, patch)
2015-10-06 11:26 UTC, Kyle M Hall
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 11565 - decreaseLoanHighHolds needs Override (12.07 KB, patch)
2015-10-19 16:03 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11565 - decreaseLoanHighHolds needs Override (11.99 KB, patch)
2016-03-04 12:49 UTC, Kyle M Hall
Details | Diff | Splinter Review
Screenshot of the suggested addition to the checkout screen (17.12 KB, image/png)
2016-03-22 07:07 UTC, Katrin Fischer
Details
Bug 11565: (QA followup) add class to allow feature to be easily hidden (3.47 KB, patch)
2016-04-05 23:32 UTC, Jesse Weaver
Details | Diff | Splinter Review
[PASSED QA] Bug 11565 - decreaseLoanHighHolds needs Override (12.06 KB, patch)
2016-04-27 12:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11565: (QA followup) add class to allow feature to be easily hidden (3.54 KB, patch)
2016-04-27 12:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11565: QA Follo-up: Improve translatability of 'not shortened' message (1.32 KB, patch)
2016-04-27 12:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 11565 [Work in progress] decreaseLoanHighHolds needs Override (12.34 KB, patch)
2016-04-27 14:57 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 11565 [QA Followup] - Hide checkout options when not in use (11.78 KB, patch)
2016-04-27 15:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11565 [QA Followup] - Only hide options when none are in use (1.34 KB, patch)
2016-04-27 15:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11565 [QA Followup] - Only hide options when none are in use (3.20 KB, patch)
2016-04-27 15:35 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11565 [QA Followup] - Hide checkout options when not in use (11.35 KB, patch)
2016-04-27 15:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 11565 [QA Followup] - Only hide options when none are in use (1.37 KB, patch)
2016-04-27 15:40 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 11565 [QA Followup] - Hide checkout options when not in use (11.42 KB, patch)
2016-04-27 16:14 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 11565 [QA Followup] - Only hide options when none are in use (1.44 KB, patch)
2016-04-27 16:15 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Christopher Brannon 2014-01-16 00:12:52 UTC
When checking out to someone that is homebound or in a limited access area (such as a bookmobile that comes through once a month), it is necessary to set a fixed loan period.  There needs to be a way to override this feature on the fly.

It might be worth having a rule under Circulation and Fine Rules to specify an item type or patron category that ignores this preference, but it is still necessary to be able to override on the fly.

I would envision an option on the warning box that allows you to continue to check out at the normal (or specified) due date, if you have a permission set for your account.

Christopher
Comment 1 Kyle M Hall 2015-09-30 16:37:46 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2015-10-06 11:26:05 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2015-10-19 16:03:10 UTC Comment hidden (obsolete)
Comment 4 Christopher Brannon 2015-10-20 14:27:18 UTC
We've been waiting a while for this enhancement.  Would love it if someone could run it through QA ASAP.  :)

Christopher
Comment 5 Katrin Fischer 2015-10-24 22:50:06 UTC
Blocked for now as we are waiting for bug 14694 to move.
Comment 6 Christopher Brannon 2016-01-19 19:37:51 UTC
Bug 14694 has passed QA.
Comment 7 Jonathan Druart 2016-03-01 11:28:10 UTC
I cannot apply this patch, bug 14694 needs a rebase.
Comment 8 Kyle M Hall 2016-03-04 12:49:22 UTC Comment hidden (obsolete)
Comment 9 Christopher Brannon 2016-03-16 14:37:23 UTC
Would love to see this in the May release as well.  Will it make it?
Comment 10 Nicole C. Engard 2016-03-17 15:17:23 UTC
Christopher, 

We're waiting on QA on this one.

Nicole
Comment 11 Christopher Brannon 2016-03-17 15:36:14 UTC
(In reply to Nicole C. Engard from comment #10)
> Christopher, 
> 
> We're waiting on QA on this one.
> 
> Nicole

Yes.  This was my attempt to politely nudge.  :)  I know people are busy.

Christopher
Comment 12 Katrin Fischer 2016-03-22 06:55:45 UTC
Comment on attachment 48679 [details] [review]
Bug 11565 - decreaseLoanHighHolds needs Override

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

::: koha-tmpl/intranet-tmpl/prog/en/modules/circ/circulation.tt
@@ +207,5 @@
>      <div class="dialog message">The patron has unpaid charges for holds, rentals etc of [% alert.OTHER_CHARGES %]</div>
>  [% END %]
>  
> +[% IF alert.HIGHHOLDS %]
> +    <div class="dialog message">High demand item. Loan period <i>not</i> shortened to [% alert.HIGHHOLDS.duration %] days (due [% alert.HIGHHOLDS.returndate %]) due to override.</div>

Please avoid the <i> mark-up as this will split the string into several parts in translation and cause issues for translators. Variables are handled with placeholders, so those are no problem.

If that remains the only issue, I can fix it in a quick QA follow-up by removing the <i>. Will continue testing.
Comment 13 Katrin Fischer 2016-03-22 07:07:17 UTC
Created attachment 49409 [details]
Screenshot of the suggested addition to the checkout screen

We have been adding a lot of elements to the once simple checkout area - I'd like to get Owen's and possible others opinions on how to handle this.

A German translation of the text for example would also be longer than the English one and break the display a bit. This might also happen for other languages, that need more words/characters.

The new checkbox will only show up when the preference is enabled, which is good. But it currently can't be hidden easily, as the label is missing an id/class.
Comment 14 Owen Leonard 2016-03-22 14:17:10 UTC
I agree that the checkout form is getting too cluttered. What if we hide all those settings by default?

http://zivotdesign.com/p/#1/14586558299862

Shown by clicking the settings link:

http://zivotdesign.com/p/#1/14586558297113

Libraries could still choose to have the panel shown by default by adding some CSS to  IntranetUserCSS.
Comment 15 Christopher Brannon 2016-03-22 14:34:59 UTC
(In reply to Owen Leonard from comment #14)
> I agree that the checkout form is getting too cluttered. What if we hide all
> those settings by default?
> 
> http://zivotdesign.com/p/#1/14586558299862
> 
> Shown by clicking the settings link:
> 
> http://zivotdesign.com/p/#1/14586558297113
> 
> Libraries could still choose to have the panel shown by default by adding
> some CSS to  IntranetUserCSS.

I'm all for simplifying the screen, but there needs to be some indicator showing that features are applied.  Too easy to forget.  Perhaps a marker (red initial or icon) next to the link?

I also question if this feature is beyond the scope of the enhancement.  Maybe another enhancement should be opened to address this concern?  I'm fine with adding it to this if it doesn't drive up the cost of the project or delay it too much longer.

Christopher
Comment 16 Christopher Brannon 2016-03-22 14:44:02 UTC
Another approach would be to have greyed out icons with no words.  They would be colored if active, and when you hover over the icon, it reminds you what it is in a tip (in the proper language).  Just a thought.
Comment 17 Katrin Fischer 2016-03-22 22:19:09 UTC
How about having the options visible once one of the options was activated? 
Or maybe just have the active one visible.

I agree that 'decluttering' the display should be moved to a different report. But I think it's increasingly necessary, because this is a really important area.

Also, to be more clear - if you have been using the feature already, this patch will make the new option show up and you have no good way of hiding it. I think the least we need to do is make it easy to hide.
Comment 18 Christopher Brannon 2016-03-23 16:12:24 UTC
(In reply to Katrin Fischer from comment #17)
> How about having the options visible once one of the options was activated? 
> Or maybe just have the active one visible.
> 
> I agree that 'decluttering' the display should be moved to a different
> report. But I think it's increasingly necessary, because this is a really
> important area.
> 
> Also, to be more clear - if you have been using the feature already, this
> patch will make the new option show up and you have no good way of hiding
> it. I think the least we need to do is make it easy to hide.

I'm all for having the dropdown choices, and an indicator to show the feature is active.  Again, as long as it doesn't drive up the cost of this project, or prolong it.

Christopher
Comment 19 Owen Leonard 2016-03-23 16:39:49 UTC
(In reply to Christopher Brannon from comment #18)

> Again, as long as it doesn't drive up the cost of this
> project

I promise not to charge you for the time I volunteered to the project when I created a fully-functional test interface as a proposal.
Comment 20 Christopher Brannon 2016-03-23 17:07:52 UTC
(In reply to Owen Leonard from comment #19)
> (In reply to Christopher Brannon from comment #18)
> 
> > Again, as long as it doesn't drive up the cost of this
> > project
> 
> I promise not to charge you for the time I volunteered to the project when I
> created a fully-functional test interface as a proposal.

:) That is kind of you.  I wasn't sure if Kyle would have to redo his work on this.  I didn't know you could join in.  Thanks, Owen!

Should we change the status until we have the addition to test?
Comment 21 Owen Leonard 2016-03-23 17:28:54 UTC
(In reply to Christopher Brannon from comment #20)
> Should we change the status until we have the addition to test?

No, I won't be submitting a patch for the interface change at this time.
Comment 22 Christopher Brannon 2016-03-23 17:42:12 UTC
Awesome.  Then I hope someone will be able to QA soon.  :)  Thanks for your input!
Comment 23 Katrin Fischer 2016-03-23 19:54:34 UTC
I may regret it, but please keep in mind that while everyone can understand about schedules and budgets... this isn't important here. The whole process for integrating changes into Koha is about what's in the best interest of Koha and all those very different libraries using it.

I personally liked Owen's suggestion a lot and would have liked to see it move forward. We owe a lot of Koha's 'prettiness' and 'easy-to-use-ness' to Owen's ongoing work and the hundreds of patches he has submitted to the project.
Comment 24 Jesse Weaver 2016-04-05 23:32:47 UTC Comment hidden (obsolete)
Comment 25 Katrin Fischer 2016-04-27 12:51:48 UTC
Created attachment 50798 [details] [review]
[PASSED QA] Bug 11565 - decreaseLoanHighHolds needs Override

This patch allows the high holds loan length decrease to be overridden
either by a checkbox that remembers its setting during a series of
checkouts, or by a one time use override checkbox that will show
in the warning popup if a checkout is affected by high holds.

Test Plan:
1) Set up a checkout that will be affected by decreaseLoanHighHolds
2) Attempt to check out an item
3) Check the override checkbox
4) Note the checkout date is not reduced
5) Return the item
6) Start a new checkout for the patron
7) Check the "Don't decrease lean length based on holds" checkbox
8) Check out the item
9) Note you are not warned about the high holds decrease
   and that the checkout length is not reduced

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 26 Katrin Fischer 2016-04-27 12:51:53 UTC
Created attachment 50799 [details] [review]
[PASSED QA] Bug 11565: (QA followup) add class to allow feature to be easily hidden

This wraps both checkboxes (in the confirmation dialog and checkout
interface) in a class of "circ-override-high-holds" so they can be
easily hidden.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 27 Katrin Fischer 2016-04-27 12:51:57 UTC
Created attachment 50800 [details] [review]
Bug 11565: QA Follo-up: Improve translatability of 'not shortened' message

Mark-up inside sentences splits them of up for translation, so it's better
to only highlight full sentences for easier translation.
Comment 28 Owen Leonard 2016-04-27 14:57:37 UTC Comment hidden (obsolete)
Comment 29 Owen Leonard 2016-04-27 14:58:55 UTC
I have attached my patch for proposed interface changes in case it is of interest to others. I do not mean for it to be considered in the QA process at this time.
Comment 30 Kyle M Hall 2016-04-27 15:17:32 UTC Comment hidden (obsolete)
Comment 31 Kyle M Hall 2016-04-27 15:17:40 UTC Comment hidden (obsolete)
Comment 32 Kyle M Hall 2016-04-27 15:35:32 UTC Comment hidden (obsolete)
Comment 33 Kyle M Hall 2016-04-27 15:40:36 UTC Comment hidden (obsolete)
Comment 34 Kyle M Hall 2016-04-27 15:40:44 UTC Comment hidden (obsolete)
Comment 35 Katrin Fischer 2016-04-27 16:14:45 UTC
Created attachment 50828 [details] [review]
[PASSED QA] Bug 11565 [QA Followup] - Hide checkout options when not in use

This follow-up reformats the checkout screen so that checkout settings
like specify due date, on-site checkout, and override high holds are
grouped in a panel which is hidden by default. Clicking a link shows all
the available settings.

Also modified in this patch: The default color for div.hint has been
made a little darker for readability.

To test, apply the patch and clear your browser cache if necessary.

- Check out to a patron who is not blocked from checking out.
- Click the "Checkout settings" link and confirm that the settings panel
  is displayed.
- Test that the settings in the panel behave as expected.
- Test with preferences on and off: decreaseLoanHighHolds,
  OnSiteCheckouts, SpecifyDueDate.

NOTE: This patch is a work in progress. It does not address the issue of
checkout setting being hidden even if they are active.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 36 Katrin Fischer 2016-04-27 16:15:31 UTC
Created attachment 50829 [details] [review]
[PASSED QA] Bug 11565 [QA Followup] - Only hide options when none are in use

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 37 Kyle M Hall 2016-04-29 12:14:23 UTC
Pushed to master for the May 2016 release! Thanks Katrin!