Bug 22907

Summary: Cannot add new suggestion with strict SQL modes turned on
Product: Koha Reporter: Jonathan Druart <jonathan.druart>
Component: AcquisitionsAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: m.de.rooy, martin.renvoize, nick
Version: unspecified   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
19.05.00, 18.11.06
Bug Depends on: 12627, 22908    
Bug Blocks: 17258, 24244    
Attachments: Bug 22907: Add tests
Bug 22907: Fix new suggestion with strict sql modes
Bug 22907: Fix new suggestion with strict sql modes
Bug 22907: Remove previous code handling the same thing
Bug 22907: Add missing use statement
Bug 22907: Add tests
Bug 22907: Fix new suggestion with strict sql modes
Bug 22907: Remove previous code handling the same thing
Bug 22907: Add missing use statement
Bug 22907: Fix new suggestion with strict sql modes
Bug 22907: Remove previous code handling the same thing
Bug 22907: Add missing use statement
Bug 22907: Add tests
Bug 22907: Fix new suggestion with strict sql modes
Bug 22907: Remove previous code handling the same thing
Bug 22907: Add missing use statement

Description Jonathan Druart 2019-05-14 17:03:16 UTC
DBIx::Class::Storage::DBI::_dbh_execute(): Incorrect integer value: '' for column 'copyrightdate' at row 1 at /home/vagrant/kohaclone/C4/Suggestions.pm line 457
Comment 1 Jonathan Druart 2019-05-14 17:08:00 UTC
Created attachment 89763 [details] [review]
Bug 22907: Add tests

Do not use copyrightdate as it is weird to have it int
Comment 2 Jonathan Druart 2019-05-14 17:08:03 UTC
Created attachment 89764 [details] [review]
Bug 22907: Fix new suggestion with strict sql modes

With sql_strict_modes turned on, if you try to create a new suggestion
you will get:
Incorrect integer value: '' for column '$INT_COLUMN' at row 1

Test plan:
Turn on strict_sql_modes, create a new suggestion, then edit it.
Comment 3 Jonathan Druart 2019-05-14 17:28:37 UTC
Created attachment 89767 [details] [review]
Bug 22907: Fix new suggestion with strict sql modes

With sql_strict_modes turned on, if you try to create a new suggestion
you will get:
Incorrect integer value: '' for column '$INT_COLUMN' at row 1

Test plan:
Turn on strict_sql_modes, create a new suggestion, then edit it.
Comment 4 Jonathan Druart 2019-05-14 17:28:40 UTC
Created attachment 89768 [details] [review]
Bug 22907: Remove previous code handling the same thing

See:
  commit a71b96295fb07084e4f87ac1a471e89355f3c5a4
  Bug 12627: Fix default values
Comment 5 Jonathan Druart 2019-05-14 17:30:58 UTC
Note that the new test will generate a DBI error, we already have one in the tests (from Patrons.t IIRC). I will try to deal with them on a separate bug report.
Comment 6 Jonathan Druart 2019-05-16 18:02:45 UTC
Created attachment 89865 [details] [review]
Bug 22907: Add missing use statement
Comment 7 Liz Rea 2019-05-16 18:50:18 UTC
Created attachment 89872 [details] [review]
Bug 22907: Add tests

Do not use copyrightdate as it is weird to have it int

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 8 Liz Rea 2019-05-16 18:50:25 UTC
Created attachment 89873 [details] [review]
Bug 22907: Fix new suggestion with strict sql modes

With sql_strict_modes turned on, if you try to create a new suggestion
you will get:
Incorrect integer value: '' for column '$INT_COLUMN' at row 1

Test plan:
Turn on strict_sql_modes, create a new suggestion, then edit it.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 9 Liz Rea 2019-05-16 18:50:27 UTC
Created attachment 89874 [details] [review]
Bug 22907: Remove previous code handling the same thing

See:
  commit a71b96295fb07084e4f87ac1a471e89355f3c5a4
  Bug 12627: Fix default values

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 10 Liz Rea 2019-05-16 18:50:30 UTC
Created attachment 89875 [details] [review]
Bug 22907: Add missing use statement

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 11 Marcel de Rooy 2019-05-27 10:13:35 UTC
QA: Looking here
Comment 12 Marcel de Rooy 2019-05-27 11:12:54 UTC
I am having trouble understanding what you do here and why you do it:
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/newordersuggestion.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/acqui/newordersuggestion.tt
@@ -107,14 +107,19 @@
     $(document).ready(function() {
         var suggestionst = $("#suggestionst").dataTable($.extend(true, {}, dataTablesDefaults, {
             "aoColumnDefs": [
-                { "aTargets": [ 0 ],  "bVisible": false, "bSearchable": true }, // must be searchable for fnFilter
+                { "aTargets": [ 0 ],  "bVisible": false, "bSearchable": false },
                 { "aTargets": [ -1 ], "bSortable": false, "bSearchable": false },
             ],
             "sPaginationType": "four_button"
         }));
         $("#show_only_mine").on('click', function(e){
             e.preventDefault();
+            //suggestionst.settings()[0].aoColumns[index].bSearchable = true;
+            suggestionst.setColumnSearchable(0, true);
+            suggestionst.rows().invalidate();
             suggestionst.fnFilter('^[% loggedinuser | html %]$', 0, true);
+            suggestionst.setColumnSearchable(0, false);
+            suggestionst.rows().invalidate();

This is not about adding a new suggestion but is about adding an order from an accepted suggestion.
The form newordersuggestion has some problems in current master with and without your patch. The fnFilter on borrowernumber (only mine) does not seem to work at all..
It seems unrelated to the problem you are solving on this report.
Comment 13 Jonathan Druart 2019-05-28 22:27:33 UTC
Created attachment 90168 [details] [review]
Bug 22907: Fix new suggestion with strict sql modes

With sql_strict_modes turned on, if you try to create a new suggestion
you will get:
Incorrect integer value: '' for column '$INT_COLUMN' at row 1

Test plan:
Turn on strict_sql_modes, create a new suggestion, then edit it.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

JD: Edit after SO, remove unecessary changes (not related to this bug)
Comment 14 Jonathan Druart 2019-05-28 22:27:38 UTC
Created attachment 90169 [details] [review]
Bug 22907: Remove previous code handling the same thing

See:
  commit a71b96295fb07084e4f87ac1a471e89355f3c5a4
  Bug 12627: Fix default values

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 15 Jonathan Druart 2019-05-28 22:27:43 UTC
Created attachment 90170 [details] [review]
Bug 22907: Add missing use statement

Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Comment 16 Jonathan Druart 2019-05-28 22:28:31 UTC
(In reply to Marcel de Rooy from comment #12)
> I am having trouble understanding what you do here and why you do it:
[...]

Oops, sorry for catching that. It was indeed related to another bug. I have amended the patch to remove those changes.
Comment 17 Marcel de Rooy 2019-05-29 12:04:49 UTC
Created attachment 90177 [details] [review]
Bug 22907: Add tests

Do not use copyrightdate as it is weird to have it int

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 18 Marcel de Rooy 2019-05-29 12:04:53 UTC
Created attachment 90178 [details] [review]
Bug 22907: Fix new suggestion with strict sql modes

With sql_strict_modes turned on, if you try to create a new suggestion
you will get:
Incorrect integer value: '' for column '$INT_COLUMN' at row 1

Test plan:
Turn on strict_sql_modes, create a new suggestion, then edit it.

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

JD: Edit after SO, remove unecessary changes (not related to this bug)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 19 Marcel de Rooy 2019-05-29 12:04:57 UTC
Created attachment 90179 [details] [review]
Bug 22907: Remove previous code handling the same thing

See:
  commit a71b96295fb07084e4f87ac1a471e89355f3c5a4
  Bug 12627: Fix default values

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 20 Marcel de Rooy 2019-05-29 12:05:01 UTC
Created attachment 90180 [details] [review]
Bug 22907: Add missing use statement

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 21 Nick Clemens 2019-05-30 10:35:45 UTC
Awesome work all!

Pushed to master for 19.05
Comment 22 Martin Renvoize 2019-05-30 20:37:33 UTC
Pushed to 18.11.x for 18.11.06