Summary: | Backend overriding status_graph element causes duplicate actions | ||
---|---|---|---|
Product: | Koha | Reporter: | Andrew Isherwood <bugzilla> |
Component: | ILL | Assignee: | Andrew Isherwood <bugzilla> |
Status: | CLOSED FIXED | QA Contact: | |
Severity: | major | ||
Priority: | P5 - low | CC: | andrew, kyle, martin.renvoize, tomascohen, victor, wainuiwitikapark |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
22.05.00,21.11.03,21.05.11,20.11.15
|
|
Circulation function: | |||
Attachments: |
Bug 28932: Fix duplicate next_action & prev_action
Bug 28932: Regression tests Bug 28932: Fix duplicate next_action & prev_action |
Created attachment 124309 [details] [review] Bug 28932: Fix duplicate next_action & prev_action This commit fixes the problem described in Bug 28932. Test plan: - Run the unit tests: prove t/db_dependent/Illrequests.t => TEST: Ensure all tests pass Created attachment 129020 [details] [review] Bug 28932: Regression tests Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> Created attachment 129021 [details] [review] Bug 28932: Fix duplicate next_action & prev_action This patch fixes the duplicate buttons issue. To test: 1. Apply the regression tests patch 2. Run: $ kshell k$ prove t/db_dependent/Illrequests.t => FAIL: Tests fail 3. Apply this patch 4. Repeat 2 => SUCCESS: Tests pass Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> I found this bug when trying an ILL backend recently. My previous attempts all used a completely decoupled graph, so never saw this before. What I was seeing was that buttons were showed randomly. With this patch most of that is gone, I still see only one button show/hide on refresh. Will try to dig a bit more. It is probably a problem with my graph, or another use case not covered by this bug. I splitted the patch in two, so it is easier to spot the problem and how the patch fixes it. Being bold here and upgrading Tomas's signoff to QA. There aren't many people capable of testing ILL to the degree Tomas clearly has.. and we're using this in production already here ;) Being bold here and upgrading Tomas's signoff to QA. There aren't many people capable of testing ILL to the degree Tomas clearly has.. and we're using this in production already here ;) (In reply to Martin Renvoize from comment #7) > Being bold here and upgrading Tomas's signoff to QA. > > There aren't many people capable of testing ILL to the degree Tomas clearly > has.. and we're using this in production already here ;) I agree. I hesitated because of the issue I was seeing on my backend, but it turned out it was because of a bad override if REQ Pushed to master for 22.05, thanks to everybody involved [U+1F984] Pushed to 21.11.x for 21.11.03 Pushed to 21.05.x for 21.05.11 Backported: Pushed to 20.11.x branch for 20.11.15 Not backported to 19.11.x for 19.11.26. Please request it if you need it. |
Koha::Illrequest::_status_graph_union merges the core status graph and one provided by a backend. Although not officially supported... > Whilst it is not intended that this works, you can override entries in $origin > with entries with the same key in $new_graph. ...a backend should be able to override a core method. This was necessary when implementing the RapidILL backend. However, this revealed a bug in which any next_actions and prev_actions declared in both would end up being duplicated, leading to duplicate buttons in the toolbar.