Bug 30728 - Allow real-time holds queue opt-out
Summary: Allow real-time holds queue opt-out
Status: Pushed to master
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Tomás Cohen Arazi
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 30710 30727
Blocks:
  Show dependency treegraph
 
Reported: 2022-05-10 16:14 UTC by Tomás Cohen Arazi
Modified: 2022-05-13 11:54 UTC (History)
6 users (show)

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


Attachments
Bug 30728: Add RealTimeHoldsQueue syspref (2.06 KB, patch)
2022-05-10 17:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30728: Add unit tests (13.69 KB, patch)
2022-05-10 17:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30728: Only trigger real-time holds queue update if enabled (6.46 KB, patch)
2022-05-10 17:22 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30728: Add RealTimeHoldsQueue syspref (3.27 KB, patch)
2022-05-10 17:27 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30728: Add unit tests (13.69 KB, patch)
2022-05-10 17:27 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30728: Only trigger real-time holds queue update if enabled (6.46 KB, patch)
2022-05-10 17:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[OPTIONAL] Bug 30728: Make RealTimeHoldsQueue disabled by default (2.33 KB, patch)
2022-05-10 17:28 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 30728: Add RealTimeHoldsQueue syspref (3.32 KB, patch)
2022-05-10 20:24 UTC, David Nind
Details | Diff | Splinter Review
Bug 30728: Add unit tests (13.74 KB, patch)
2022-05-10 20:24 UTC, David Nind
Details | Diff | Splinter Review
Bug 30728: Only trigger real-time holds queue update if enabled (6.51 KB, patch)
2022-05-10 20:24 UTC, David Nind
Details | Diff | Splinter Review
Bug 30728: Make RealTimeHoldsQueue disabled by default (2.36 KB, patch)
2022-05-10 20:24 UTC, David Nind
Details | Diff | Splinter Review
Bug 30728: Add RealTimeHoldsQueue syspref (3.38 KB, patch)
2022-05-11 10:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30728: Add unit tests (13.80 KB, patch)
2022-05-11 10:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30728: Only trigger real-time holds queue update if enabled (6.57 KB, patch)
2022-05-11 10:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30728: Make RealTimeHoldsQueue disabled by default (2.42 KB, patch)
2022-05-11 10:02 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 30728: (QA follow-up) Fix missing comma (1.57 KB, patch)
2022-05-13 11:48 UTC, Nick Clemens
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi 2022-05-10 16:14:17 UTC
Somehow, the patchset got an initially developed piece that made it optional lost. Probably my fault on some submission.

We need to add that back.
Comment 1 Tomás Cohen Arazi 2022-05-10 17:22:37 UTC Comment hidden (obsolete)
Comment 2 Tomás Cohen Arazi 2022-05-10 17:22:41 UTC Comment hidden (obsolete)
Comment 3 Tomás Cohen Arazi 2022-05-10 17:22:47 UTC Comment hidden (obsolete)
Comment 4 Tomás Cohen Arazi 2022-05-10 17:27:50 UTC
Created attachment 134826 [details] [review]
Bug 30728: Add RealTimeHoldsQueue syspref

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 5 Tomás Cohen Arazi 2022-05-10 17:27:55 UTC
Created attachment 134827 [details] [review]
Bug 30728: Add unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 6 Tomás Cohen Arazi 2022-05-10 17:28:00 UTC
Created attachment 134828 [details] [review]
Bug 30728: Only trigger real-time holds queue update if enabled

This patch makes the places in which Koha enqueues holds queue for
real time updates verify the feature is enabled.

To test:
1. Apply this patches up to the unit tests
2. Run:
   $ updatedatabase
   $ kshell
  k$ git diff origin/master --name-only | grep -e '\.t$' | xargs prove
=> FAIL: tests fail, the code doesn't care about the syspref
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Be happy!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 7 Tomás Cohen Arazi 2022-05-10 17:28:05 UTC
Created attachment 134829 [details] [review]
[OPTIONAL] Bug 30728: Make RealTimeHoldsQueue disabled by default

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 8 Tomás Cohen Arazi 2022-05-10 17:29:19 UTC
Personally, I prefer to have the feature enabled by default. But I've added a follow-up patch disabling it by default so we can choose to disable it by default.
Comment 9 David Nind 2022-05-10 20:24:39 UTC
Created attachment 134831 [details] [review]
Bug 30728: Add RealTimeHoldsQueue syspref

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>
Comment 10 David Nind 2022-05-10 20:24:44 UTC
Created attachment 134832 [details] [review]
Bug 30728: Add unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>
Comment 11 David Nind 2022-05-10 20:24:50 UTC
Created attachment 134833 [details] [review]
Bug 30728: Only trigger real-time holds queue update if enabled

This patch makes the places in which Koha enqueues holds queue for
real time updates verify the feature is enabled.

To test:
1. Apply this patches up to the unit tests
2. Run:
   $ updatedatabase
   $ kshell
  k$ git diff origin/master --name-only | grep -e '\.t$' | xargs prove
=> FAIL: tests fail, the code doesn't care about the syspref
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Be happy!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 David Nind 2022-05-10 20:24:55 UTC
Created attachment 134834 [details] [review]
Bug 30728: Make RealTimeHoldsQueue disabled by default

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 David Nind 2022-05-10 20:53:01 UTC
After signing off, the [OPTIONAL] patch is still there as well as the signed off version without optional in the title - does this need to be obsoleted?
Comment 14 Martin Renvoize 2022-05-11 10:02:08 UTC
Created attachment 134857 [details] [review]
Bug 30728: Add RealTimeHoldsQueue syspref

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 15 Martin Renvoize 2022-05-11 10:02:12 UTC
Created attachment 134858 [details] [review]
Bug 30728: Add unit tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 16 Martin Renvoize 2022-05-11 10:02:16 UTC
Created attachment 134859 [details] [review]
Bug 30728: Only trigger real-time holds queue update if enabled

This patch makes the places in which Koha enqueues holds queue for
real time updates verify the feature is enabled.

To test:
1. Apply this patches up to the unit tests
2. Run:
   $ updatedatabase
   $ kshell
  k$ git diff origin/master --name-only | grep -e '\.t$' | xargs prove
=> FAIL: tests fail, the code doesn't care about the syspref
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Be happy!
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 17 Martin Renvoize 2022-05-11 10:02:20 UTC
Created attachment 134860 [details] [review]
Bug 30728: Make RealTimeHoldsQueue disabled by default

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 18 Martin Renvoize 2022-05-11 10:07:01 UTC
Solid improvement, could have swarn when first testing this was in the original patchset.. I agree it's a required addition.. we have libraries that want this queue to be static for X hours whilst they have people walk around the library with a paper list printed from the results.

All works as expected, QA script is happy and the Unit tests all pass.

Passing QA
Comment 19 Fridolin Somers 2022-05-13 08:17:31 UTC
Pushed to master for 22.05, thanks to everybody involved 🦄
Comment 20 Nick Clemens 2022-05-13 11:48:22 UTC
Created attachment 134967 [details] [review]
Bug 30728: (QA follow-up) Fix missing comma
Comment 21 Martin Renvoize 2022-05-13 11:54:45 UTC
Followup pushed to get jenkins (and koha-testing-docker) happy again.. easily missed comma lost somewhere along the line.