Bug 22907 - Cannot add new suggestion with strict SQL modes turned on
Summary: Cannot add new suggestion with strict SQL modes turned on
Status: Pushed to Stable
Alias: None
Product: Koha
Classification: Unclassified
Component: Acquisitions (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Target Milestone: ---
Assignee: Jonathan Druart
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on: 22908 12627
Blocks: 17258
  Show dependency treegraph
 
Reported: 2019-05-14 17:03 UTC by Jonathan Druart
Modified: 2019-05-30 20:37 UTC (History)
3 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:
19.05.00, 18.11.06


Attachments
Bug 22907: Add tests (808 bytes, patch)
2019-05-14 17:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22907: Fix new suggestion with strict sql modes (3.74 KB, patch)
2019-05-14 17:08 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22907: Fix new suggestion with strict sql modes (3.54 KB, patch)
2019-05-14 17:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22907: Remove previous code handling the same thing (1.89 KB, patch)
2019-05-14 17:28 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22907: Add missing use statement (605 bytes, patch)
2019-05-16 18:02 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22907: Add tests (853 bytes, patch)
2019-05-16 18:50 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22907: Fix new suggestion with strict sql modes (3.58 KB, patch)
2019-05-16 18:50 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22907: Remove previous code handling the same thing (1.93 KB, patch)
2019-05-16 18:50 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22907: Add missing use statement (649 bytes, patch)
2019-05-16 18:50 UTC, Liz Rea
Details | Diff | Splinter Review
Bug 22907: Fix new suggestion with strict sql modes (2.15 KB, patch)
2019-05-28 22:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22907: Remove previous code handling the same thing (1.93 KB, patch)
2019-05-28 22:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22907: Add missing use statement (649 bytes, patch)
2019-05-28 22:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 22907: Add tests (951 bytes, patch)
2019-05-29 12:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 22907: Fix new suggestion with strict sql modes (2.24 KB, patch)
2019-05-29 12:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 22907: Remove previous code handling the same thing (2.03 KB, patch)
2019-05-29 12:04 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 22907: Add missing use statement (747 bytes, patch)
2019-05-29 12:05 UTC, Marcel de Rooy
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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