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
Assignee: Marcel de Rooy
QA Contact: Nick Clemens (kidclamp)
URL:
Keywords:
Depends on: 32478
Blocks:
  Show dependency tree / graph
 
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
Circulation function:

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
Show Obsolete (4) View All Add an attachment (proposed patch, testcase, etc.)

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 (tcohen) 2023-06-07 18:34:09 UTC 
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 10 Tomás Cohen Arazi (tcohen) 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