Bug 14733 - Prevent a record from having holds with duplicate priorities
Summary: Prevent a record from having holds with duplicate priorities
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Hold requests (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low major (vote)
Assignee: Kyle M Hall
QA Contact: Testopia
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-26 15:42 UTC by Kyle M Hall
Modified: 2016-06-21 21:40 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 14733 - Unit Tests (1.72 KB, patch)
2015-08-26 15:47 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14733 - Prevent a record from having holds with duplicate priorities (1.32 KB, patch)
2015-08-26 15:48 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14733 - Tidy AddReserve (5.79 KB, text/plain)
2015-08-26 15:48 UTC, Kyle M Hall
Details
Bug 14733 - Unit Tests (1.76 KB, patch)
2015-09-17 12:55 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Bug 14733 - Prevent a record from having holds with duplicate priorities (1.42 KB, patch)
2015-09-17 12:55 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Bug 14733 - Unit Tests (1.83 KB, patch)
2015-09-21 15:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14733 - Prevent a record from having holds with duplicate priorities (1.49 KB, patch)
2015-09-21 15:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14733: Replace 'priority' with 'estimated priority' (1.03 KB, patch)
2015-09-21 15:46 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14733: (QA followup) make tests independent of already defined branches (10.74 KB, patch)
2015-09-25 14:50 UTC, Tomás Cohen Arazi
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 2015-08-26 15:42:26 UTC
It is possible to create holds with duplicate priorities.

The reason for this is that typically the priority is calculated before placing the hold. When the hold is placed the priority is calculated. This can easily be shown by opening up two browser windows and starting to place a hold for a record in each one. You'll see that both list the same priority. If you than place the hold in each window, both holds will have the same priority!
Comment 1 Kyle M Hall 2015-08-26 15:47:53 UTC Comment hidden (obsolete)
Comment 2 Kyle M Hall 2015-08-26 15:48:02 UTC Comment hidden (obsolete)
Comment 3 Kyle M Hall 2015-08-26 15:48:05 UTC Comment hidden (obsolete)
Comment 4 Chris Cormack 2015-09-02 22:39:35 UTC
This works, but it does have the side effect that in the staff interface where it shows the priority when placing a reserve.

It will say Priority 1 for example

Then you save it, and now its priority 2 ... I think this will prompt bug reports from Librarians. Do we care about that?
Comment 5 Kyle M Hall 2015-09-14 17:42:57 UTC
(In reply to Chris Cormack from comment #4)
> This works, but it does have the side effect that in the staff interface
> where it shows the priority when placing a reserve.
> 
> It will say Priority 1 for example
> 
> Then you save it, and now its priority 2 ... I think this will prompt bug
> reports from Librarians. Do we care about that?

That thought had occurred to me, but I didn't take any action on it. We could change the "Priority" label to "Estimated priority", or just do away with the field altogether. I'm not sure I see the advantage of having it at all.

What do you think?
Comment 6 Nicole C. Engard 2015-09-17 12:55:37 UTC Comment hidden (obsolete)
Comment 7 Nicole C. Engard 2015-09-17 12:55:54 UTC Comment hidden (obsolete)
Comment 8 Jonathan Druart 2015-09-21 15:46:28 UTC
Created attachment 42734 [details] [review]
Bug 14733 - Unit Tests

Signed-off-by: Heather Braum <hbraum@nekls.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 9 Jonathan Druart 2015-09-21 15:46:32 UTC
Created attachment 42735 [details] [review]
Bug 14733 - Prevent a record from having holds with duplicate priorities

It is possible to create holds with duplicate priorities.

The reason for this is that typically the priority is calculated before
placing the hold. When the hold is placed the priority is calculated.
This can easily be shown by opening up two browser windows and starting
to place a hold for a record in each one. You'll see that both list the
same priority. If you than place the hold in each window, both holds
will have the same priority!

Test Plan:
1) Run unit tests pre-patch, note they fail
2) Run unit tests post-patch, note they succeed

Signed-off-by: Heather Braum <hbraum@nekls.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Jonathan Druart 2015-09-21 15:46:36 UTC
Created attachment 42736 [details] [review]
Bug 14733: Replace 'priority' with 'estimated priority'

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 11 Tomás Cohen Arazi 2015-09-25 14:50:53 UTC
Created attachment 42880 [details] [review]
Bug 14733: (QA followup) make tests independent of already defined branches

The current tests were expecting the first defined branch to be CPL. That's
not the case on my box so they failed. This patch adds the creation of two
new random branches/branchcodes, and replaces the legacy use of CPL and MPL
in favour of the new ones.
It relies on TestBuilder for the task.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 12 Tomás Cohen Arazi 2015-09-25 14:57:20 UTC
Patches pushed to master.

Thanks Kyle and Jonathan!
Comment 13 Liz Rea 2015-10-16 01:31:30 UTC
Pushed to 3.18.x, will be in 3.18.12.

Noting that I resolved a conflict in the unit tests regarding the number of tests.
Comment 14 Liz Rea 2015-11-02 21:01:27 UTC
Pushed to 3.18.x, is in 3.18.12.

Note that I left off the tests for 3.18.