Bug 21604

Summary: Cannot add/edit funds, cannot add budgets
Product: Koha Reporter: Nick Clemens (kidclamp) <nick>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: major    
Priority: P5 - low CC: fridolin.somers, katrin.fischer, m.de.rooy, martin.renvoize, pierre-marc.thibault
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 17258    
Attachments: Bug 21604: Fix add/edit fund/budget
Bug 21604: Fix add/edit fund/budget
Bug 21604: Add two trivial test cases
Bug 21604: Fix add/edit fund/budget

Description Nick Clemens (kidclamp) 2018-10-18 13:48:47 UTC
Default values are causing issues:

DBIx::Class::Storage::DBI::_dbh_execute(): Incorrect integer value: '' for column 'budget_period_id' at row 1 at /home/vagrant/kohaclone/C4/Budgets.pm line 138
 at /usr/share/perl5/DBIx/Class/Exception.pm line 77
Comment 1 Jonathan Druart 2018-10-18 22:21:53 UTC
Created attachment 80885 [details] [review]
Bug 21604: Fix add/edit fund/budget

[Strict SQL modes context]

For insert aqbudgetperiods:
Incorrect integer value: '' for column 'budget_period_id'

For insert/update aqbudgets
Incorrect integer value: '' for column 'budget_owner_id'
Incorrect decimal value: '' for column 'budget_encumb'

Test plan:
Add/edit budgets and funds

A good example of why we should use Koha::Object for all our objects
Comment 2 Pierre-Marc Thibault 2018-10-19 19:42:39 UTC
Created attachment 80959 [details] [review]
Bug 21604: Fix add/edit fund/budget

[Strict SQL modes context]

For insert aqbudgetperiods:
Incorrect integer value: '' for column 'budget_period_id'

For insert/update aqbudgets
Incorrect integer value: '' for column 'budget_owner_id'
Incorrect decimal value: '' for column 'budget_encumb'

Test plan:
Add/edit budgets and funds

A good example of why we should use Koha::Object for all our objects

Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com>
Comment 3 Katrin Fischer 2018-10-28 13:05:34 UTC
I added <strict_sql_modes>1</strict_sql_modes> to  /etc/koha/sites/kohadev/koha-conf.xml and restart_all and restarted memcached separately too, but I fail to make things explode. What am I missing?
Comment 4 Marcel de Rooy 2018-11-02 06:51:04 UTC
Created attachment 81858 [details] [review]
Bug 21604: Add two trivial test cases

Adding an id in AddBudgetPeriod and an empty string in AddBudget for
budget_encumb.

Test plan:
Run Budgets.t with/without this patch in strict sql mode.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 5 Marcel de Rooy 2018-11-02 06:51:08 UTC
Created attachment 81859 [details] [review]
Bug 21604: Fix add/edit fund/budget

[Strict SQL modes context]

For insert aqbudgetperiods:
Incorrect integer value: '' for column 'budget_period_id'

For insert/update aqbudgets
Incorrect integer value: '' for column 'budget_owner_id'
Incorrect decimal value: '' for column 'budget_encumb'

Test plan:
Add/edit budgets and funds

A good example of why we should use Koha::Object for all our objects

Signed-off-by: Pierre-Marc Thibault <pierre-marc.thibault@inLibro.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 6 Marcel de Rooy 2018-11-02 06:51:43 UTC
(In reply to Katrin Fischer from comment #3)
> I added <strict_sql_modes>1</strict_sql_modes> to 
> /etc/koha/sites/kohadev/koha-conf.xml and restart_all and restarted
> memcached separately too, but I fail to make things explode. What am I
> missing?

Sabotaged the test to show it :)
Comment 7 Katrin Fischer 2018-11-02 08:54:05 UTC
(In reply to Marcel de Rooy from comment #6)
> (In reply to Katrin Fischer from comment #3)
> > I added <strict_sql_modes>1</strict_sql_modes> to 
> > /etc/koha/sites/kohadev/koha-conf.xml and restart_all and restarted
> > memcached separately too, but I fail to make things explode. What am I
> > missing?
> 
> Sabotaged the test to show it :)

Turns out the entry needs to go within the config section (figured it out later, updated wiki)
Comment 8 Nick Clemens (kidclamp) 2018-11-02 10:42:50 UTC
Awesome work all!

Pushed to master for 18.11
Comment 9 Martin Renvoize 2018-11-09 16:37:11 UTC
Pushed to 18.05.x for 18.05.06
Comment 10 Fridolin Somers 2018-11-28 14:08:28 UTC
SQL modes not in 17.11.x