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.
Created attachment 134823 [details] [review] Bug 30728: Add RealTimeHoldsQueue syspref Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134824 [details] [review] Bug 30728: Add unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134825 [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>
Created attachment 134826 [details] [review] Bug 30728: Add RealTimeHoldsQueue syspref Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Created attachment 134827 [details] [review] Bug 30728: Add unit tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
Created attachment 134829 [details] [review] [OPTIONAL] Bug 30728: Make RealTimeHoldsQueue disabled by default Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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.
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>
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>
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>
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>
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?
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>
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>
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>
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>
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
Pushed to master for 22.05, thanks to everybody involved [U+1F984]
Created attachment 134967 [details] [review] Bug 30728: (QA follow-up) Fix missing comma
Followup pushed to get jenkins (and koha-testing-docker) happy again.. easily missed comma lost somewhere along the line.