Bug 32132 - Missing budget_period_id in aqbudgets kills lateorders.pl
Summary: Missing budget_period_id in aqbudgets kills lateorders.pl
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Emmi Takkinen
QA Contact: Testopia
URL:
Keywords: additional_work_needed
Depends on:
Blocks:
 
Reported: 2022-11-08 11:59 UTC by Emmi Takkinen
Modified: 2024-04-19 17:55 UTC (History)
1 user (show)

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


Attachments
Bug 32132: Return undef if budget is not found (2.57 KB, patch)
2022-11-17 10:00 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 32132: Do not allow NULL value in column aqbudgets.budget_period_id (2.98 KB, patch)
2024-02-02 08:00 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 32132: Do not allow NULL value in column aqbudgets.budget_period_id (4.19 KB, patch)
2024-02-16 06:12 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 32132: DO NOT PUSH! Schema changes (4.08 KB, patch)
2024-02-16 06:12 UTC, Emmi Takkinen
Details | Diff | Splinter Review
Bug 32132: Do not allow NULL value in column aqbudgets.budget_period_id (4.23 KB, patch)
2024-02-16 15:38 UTC, Lucas Gass
Details | Diff | Splinter Review
Bug 32132: Do not allow NULL value in column aqbudgets.budget_period_id (4.31 KB, patch)
2024-02-29 10:35 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Emmi Takkinen 2022-11-08 11:59:29 UTC
If column budget_period_id in aqbudgets table is NULL lateorders.pl, Koha dies on error 500. Logs read:

Template process failed: undef error - DBIC result _type  isn't of the _type Aqbudget at /home/koha/Koha/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt line 157. 

Of course solution for this is to fix missing values, but I think we should still prevent Koha dying on this.
Comment 1 Emmi Takkinen 2022-11-17 10:00:21 UTC
Created attachment 143988 [details] [review]
Bug 32132: Return undef if budget is not found

If budget_period_id column is set as NULL, "Late orders"
page dies on error 500. Logs read:

Template process failed: undef error - DBIC result _type isn't of the _type Aqbudget at
/home/koha/Koha/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt line 157

Prevent this happening by returning undef from method "budget".

To test:
1. Navigate to "Late orders" page.
2. Choose any order, take a note in which budget it belongs to.
3. Update chosen budget from database:
UPDATE aqbudgets SET budget_period_id WHERE budget_id = <budget_id>;
4. Reload "Late orders" page.
=> Page dies on error 500.
5. Apply this patch.
6. Reload page again.
=> Late orders should now be displayed correctly.

Also prove t/db_dependent/Koha/Acquisition/Fund.t.

Sponsored-by: Koha-Suomi Oy
Comment 2 Jonathan Druart 2023-05-04 11:53:36 UTC
There is the ON DELETE CASCADE clause on this FK so I think the correct fix is to make budget_period_id NOT NULL instead.
Comment 3 Emmi Takkinen 2023-08-23 06:12:27 UTC
Setting this assigned to remind myself to look into this again at some point.
Comment 4 Emmi Takkinen 2024-02-02 08:00:08 UTC
Created attachment 161721 [details] [review]
Bug 32132: Do not allow NULL value in column aqbudgets.budget_period_id

If budget_period_id column is set as NULL, "Late orders"
page dies on error 500. Logs read:

Template process failed: undef error - DBIC result _type isn't of the _type Aqbudget at
/home/koha/Koha/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt line 157

This patch makes changes to column aqbudgets.budget_period_id so that it
no longer allows NULL values. This is in line with aqbudgets tables foreign key
"budget_period_id" which has ON DELETE CASCADE clause.

To test:
1. Run prove t/db_dependent/Koha/Acquisition/Fund.t
=> Tests should fail
2. Run updatedatabase.pl and restart services if needed
3. Rerun prove t/db_dependent/Koha/Acquisition/Fund.t
=> Tests should now pass

Sponsored-by: Koha-Suomi Oy
Comment 5 Emmi Takkinen 2024-02-16 06:12:38 UTC
Created attachment 162232 [details] [review]
Bug 32132: Do not allow NULL value in column aqbudgets.budget_period_id

If budget_period_id column is set as NULL, "Late orders"
page dies on error 500. Logs read:

Template process failed: undef error - DBIC result _type isn't of the _type Aqbudget at
/home/koha/Koha/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt line 157

This patch makes changes to column aqbudgets.budget_period_id so that it
no longer allows NULL values. This is in line with aqbudgets tables foreign key
"budget_period_id" which has ON DELETE CASCADE clause.

To test:
1. Run prove t/db_dependent/Koha/Acquisition/Fund.t
=> Tests should fail
2. Run updatedatabase.pl and restart services if needed
3. Rerun prove t/db_dependent/Koha/Acquisition/Fund.t
=> Tests should now pass

Sponsored-by: Koha-Suomi Oy
Comment 6 Emmi Takkinen 2024-02-16 06:12:58 UTC
Created attachment 162233 [details] [review]
Bug 32132: DO NOT PUSH! Schema changes
Comment 7 Emmi Takkinen 2024-02-16 06:13:21 UTC
Added changes to kohastructure.sql and schema changes.
Comment 8 Lucas Gass 2024-02-16 15:38:23 UTC
Created attachment 162241 [details] [review]
Bug 32132: Do not allow NULL value in column aqbudgets.budget_period_id

If budget_period_id column is set as NULL, "Late orders"
page dies on error 500. Logs read:

Template process failed: undef error - DBIC result _type isn't of the _type Aqbudget at
/home/koha/Koha/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt line 157

This patch makes changes to column aqbudgets.budget_period_id so that it
no longer allows NULL values. This is in line with aqbudgets tables foreign key
"budget_period_id" which has ON DELETE CASCADE clause.

To test:
1. Run prove t/db_dependent/Koha/Acquisition/Fund.t
=> Tests should fail
2. Run updatedatabase.pl and restart services if needed
3. Rerun prove t/db_dependent/Koha/Acquisition/Fund.t
=> Tests should now pass

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Comment 9 Jonathan Druart 2024-02-29 10:35:06 UTC
Created attachment 162584 [details] [review]
Bug 32132: Do not allow NULL value in column aqbudgets.budget_period_id

If budget_period_id column is set as NULL, "Late orders"
page dies on error 500. Logs read:

Template process failed: undef error - DBIC result _type isn't of the _type Aqbudget at
/home/koha/Koha/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/lateorders.tt line 157

This patch makes changes to column aqbudgets.budget_period_id so that it
no longer allows NULL values. This is in line with aqbudgets tables foreign key
"budget_period_id" which has ON DELETE CASCADE clause.

To test:
1. Run prove t/db_dependent/Koha/Acquisition/Fund.t
=> Tests should fail
2. Run updatedatabase.pl and restart services if needed
3. Rerun prove t/db_dependent/Koha/Acquisition/Fund.t
=> Tests should now pass

Sponsored-by: Koha-Suomi Oy
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 10 Katrin Fischer 2024-04-19 16:59:17 UTC
Question: I believe the database update will fail if the library has data with budget_period_id = NULL. Could we handle this a bit more gracefully in the database update maybe?
Comment 11 Katrin Fischer 2024-04-19 17:55:40 UTC
Pushed for 24.05!

Well done everyone, thank you!