Bug 24614 - Can't edit reports if not using cache
Summary: Can't edit reports if not using cache
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Reports (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Kyle M Hall
URL:
Keywords:
Depends on:
Blocks: 25000
  Show dependency treegraph
 
Reported: 2020-02-07 19:42 UTC by Nick Clemens (kidclamp)
Modified: 2020-11-30 21:47 UTC (History)
6 users (show)

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


Attachments
Bug 24614: Add tests (1.28 KB, patch)
2020-02-11 16:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24614: Use Koha::Reports from save_report and update_sql (4.08 KB, patch)
2020-02-11 16:25 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 24614: Add tests (1.34 KB, patch)
2020-03-25 12:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24614: Use Koha::Reports from save_report and update_sql (4.14 KB, patch)
2020-03-25 12:48 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 24614: Add tests (1.41 KB, patch)
2020-03-25 19:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24614: Use Koha::Reports from save_report and update_sql (4.21 KB, patch)
2020-03-25 19:00 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 24614: Fix Reports/Guided.t (1.81 KB, patch)
2020-03-27 16:12 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 Nick Clemens (kidclamp) 2020-02-07 19:42:58 UTC
To test:
Enable strict mode
Disable/stop memcached
Create a report (note you don't see the cache expiry option)
Try to edit the report
It fails
Enable memcached
You get the cache expiry option back
You can edit
Comment 1 Jonathan Druart 2020-02-11 16:25:11 UTC
Created attachment 98714 [details] [review]
Bug 24614: Add tests
Comment 2 Jonathan Druart 2020-02-11 16:25:15 UTC
Created attachment 98715 [details] [review]
Bug 24614: Use Koha::Reports from save_report and update_sql

In order to get the default value defined at DBMS level, we use
Koha::Reports (to inherit from Koha::Object->store) from the 2 add/edit
methods of C4::Reports::Guided.
A second step would be to remove completely those CRUD subroutines and
use directly Koha::Reports instead.

Test plan:
1. Add and edit some reports
2. Disable memcached, create a report, edit it
=> Should not crash
3. Make sure the tests make sense and that they pass after the second
patch.

The error was:
DBD::mysql::db do failed: Column 'cache_expiry' cannot be null [for
Statement "UPDATE saved_sql SET savedsql = ?, last_modified = now(),
report_name = ?, report_group = ?, report_subgroup = ?, notes = ?,
cache_expiry = ?, public = ? WHERE id = ? "] at
/kohadevbox/koha/C4/Reports/Guided.pm line 633.
Comment 3 Martin Renvoize 2020-03-25 12:48:44 UTC
Created attachment 101700 [details] [review]
Bug 24614: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 4 Martin Renvoize 2020-03-25 12:48:46 UTC
Created attachment 101701 [details] [review]
Bug 24614: Use Koha::Reports from save_report and update_sql

In order to get the default value defined at DBMS level, we use
Koha::Reports (to inherit from Koha::Object->store) from the 2 add/edit
methods of C4::Reports::Guided.
A second step would be to remove completely those CRUD subroutines and
use directly Koha::Reports instead.

Test plan:
1. Add and edit some reports
2. Disable memcached, create a report, edit it
=> Should not crash
3. Make sure the tests make sense and that they pass after the second
patch.

The error was:
DBD::mysql::db do failed: Column 'cache_expiry' cannot be null [for
Statement "UPDATE saved_sql SET savedsql = ?, last_modified = now(),
report_name = ?, report_group = ?, report_subgroup = ?, notes = ?,
cache_expiry = ?, public = ? WHERE id = ? "] at
/kohadevbox/koha/C4/Reports/Guided.pm line 633.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Martin Renvoize 2020-03-25 12:49:00 UTC
Does the trick, Signing off.
Comment 6 Kyle M Hall 2020-03-25 19:00:43 UTC
Created attachment 101764 [details] [review]
Bug 24614: Add tests

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 7 Kyle M Hall 2020-03-25 19:00:54 UTC
Created attachment 101765 [details] [review]
Bug 24614: Use Koha::Reports from save_report and update_sql

In order to get the default value defined at DBMS level, we use
Koha::Reports (to inherit from Koha::Object->store) from the 2 add/edit
methods of C4::Reports::Guided.
A second step would be to remove completely those CRUD subroutines and
use directly Koha::Reports instead.

Test plan:
1. Add and edit some reports
2. Disable memcached, create a report, edit it
=> Should not crash
3. Make sure the tests make sense and that they pass after the second
patch.

The error was:
DBD::mysql::db do failed: Column 'cache_expiry' cannot be null [for
Statement "UPDATE saved_sql SET savedsql = ?, last_modified = now(),
report_name = ?, report_group = ?, report_subgroup = ?, notes = ?,
cache_expiry = ?, public = ? WHERE id = ? "] at
/kohadevbox/koha/C4/Reports/Guided.pm line 633.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 8 Martin Renvoize 2020-03-26 11:40:31 UTC
Nice work everyone!

Pushed to master for 20.05
Comment 9 Jonathan Druart 2020-03-27 16:12:14 UTC
Created attachment 102013 [details] [review]
Bug 24614: Fix Reports/Guided.t

DBD::mysql::st execute failed: Column 'public' cannot be null [for Statement "UPDATE `saved_sql` SET `last_modified` = ?, `notes` = ?, `public` = ?, `report_group` = ?, `report_name` = ?, `report_subgroup` = ?,
`savedsql` = ? WHERE ( `id` = ? )" with ParamValues: 0='2020-03-27T16:03:04', 1=undef, 2=undef, 3=undef, 4='Just another report', 5=undef, 6=undef, 7=25] at /usr/share/perl5/DBIx/Class/Storage/DBI.pm line 1836.
    # Looks like you planned 17 tests but ran 6.

update_sql expect all the paramters to be set, otherwise it will be
nulled.

The best way to fix it is at test level. There is only 1 occurrence in
controller/module, and it sends all the parameters. That is the correct
way to do and will make things easier when we will removed them to use
Koha::Reports directly
Comment 10 Joy Nelson 2020-04-03 22:39:58 UTC
backported to 19.11.x for 19.11.05
Comment 11 Lucas Gass 2020-04-13 20:42:18 UTC
backported to 19.05.x for 19.05.10
Comment 12 Hayley Pelham 2020-04-20 00:02:58 UTC
Normal patch will not be backported to 18.11.x series