Bug 33844 - item->is_denied_renewal should check column from associated pref
Summary: item->is_denied_renewal should check column from associated pref
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Marcel de Rooy
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords:
Depends on: 32478
Blocks:
  Show dependency treegraph
 
Reported: 2023-05-26 10:14 UTC by Marcel de Rooy
Modified: 2023-12-28 20:46 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.01,22.11.08


Attachments
Bug 33844: Modify test to show problem (1.39 KB, patch)
2023-05-26 13:03 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33844: Fix is_denied_renewal (1.09 KB, patch)
2023-05-26 13:03 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 33844: Modify test to show problem (1.41 KB, patch)
2023-05-29 17:15 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33844: Fix is_denied_renewal (1.11 KB, patch)
2023-05-29 17:15 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33844: Modify test to show problem (1.46 KB, patch)
2023-06-07 14:32 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review
Bug 33844: Fix is_denied_renewal (1.16 KB, patch)
2023-06-07 14:32 UTC, Nick Clemens (kidclamp)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Marcel de Rooy 2023-05-26 10:14:29 UTC
See bug 32478.
You could even add a field 'delete' in the pref. This is really bad.
Comment 1 Marcel de Rooy 2023-05-26 13:03:49 UTC
Created attachment 151759 [details] [review]
Bug 33844: Modify test to show problem

Add delete key.
Without the follow-up, this triggers a delete. Test fails.
DBIx::Class::Row::delete(): Not in database at /usr/share/koha/Koha/Object.pm line 234

Test plan:
Run t/db_dependent/Koha/Item.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 2 Marcel de Rooy 2023-05-26 13:03:51 UTC
Created attachment 151760 [details] [review]
Bug 33844: Fix is_denied_renewal

Going via result_set->has_column does cost actually nothing if
the object is already there.

Test plan:
Run again t/db_dependent/Koha/Item.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 3 Katrin Fischer 2023-05-29 17:15:48 UTC
Created attachment 151785 [details] [review]
Bug 33844: Modify test to show problem

Add delete key.
Without the follow-up, this triggers a delete. Test fails.
DBIx::Class::Row::delete(): Not in database at /usr/share/koha/Koha/Object.pm line 234

Test plan:
Run t/db_dependent/Koha/Item.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 4 Katrin Fischer 2023-05-29 17:15:51 UTC
Created attachment 151786 [details] [review]
Bug 33844: Fix is_denied_renewal

Going via result_set->has_column does cost actually nothing if
the object is already there.

Test plan:
Run again t/db_dependent/Koha/Item.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 5 Jonathan Druart 2023-05-30 07:23:30 UTC
We usually use Koha::Objects->columns to retrieve the exist columns. Not sure what's the fastest here however.
Comment 6 Marcel de Rooy 2023-05-30 07:58:42 UTC
(In reply to Jonathan Druart from comment #5)
> We usually use Koha::Objects->columns to retrieve the exist columns. Not
> sure what's the fastest here however.

This is the fastest. Timed a few things here.
Comment 7 Nick Clemens (kidclamp) 2023-06-07 14:32:18 UTC
Created attachment 152110 [details] [review]
Bug 33844: Modify test to show problem

Add delete key.
Without the follow-up, this triggers a delete. Test fails.
DBIx::Class::Row::delete(): Not in database at /usr/share/koha/Koha/Object.pm line 234

Test plan:
Run t/db_dependent/Koha/Item.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 8 Nick Clemens (kidclamp) 2023-06-07 14:32:20 UTC
Created attachment 152111 [details] [review]
Bug 33844: Fix is_denied_renewal

Going via result_set->has_column does cost actually nothing if
the object is already there.

Test plan:
Run again t/db_dependent/Koha/Item.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 9 Tomás Cohen Arazi 2023-06-07 18:34:09 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 10 Tomás Cohen Arazi 2023-06-09 19:06:11 UTC
Pushed to master for 23.05.

Nice work everyone, thanks!
Comment 11 Pedro Amorim 2023-07-18 09:23:50 UTC
Nice work everyone!

Pushed to 22.11.x for next release