|
Description
Andrew Fuerste-Henry
2021-09-29 19:13:36 UTC
Created attachment 125462 [details] [review] Bug 29138: Use zero instead of no in LoadSearchHistoryToTheFirstLoggedUser To test: - apply patch - set LoadSearchHistoryToTheFirstLoggedUser to 'Don't add' - confirm via sql that value=0 - confirm on OPAC that search history is not loaded to account on login Created attachment 125717 [details] [review] Bug 29138: Use zero instead of no in LoadSearchHistoryToTheFirstLoggedUser To test: - apply patch - set LoadSearchHistoryToTheFirstLoggedUser to 'Don't add' - confirm via sql that value=0 - confirm on OPAC that search history is not loaded to account on login Signed-off-by: David Nind <david@davidnind.com> Testing notes (koha-testing-docker): 1. Check that LoadSearchHistoryToTheFirstLoggedUser is not working as expected when set to 'Don't add': a. Set LoadSearchHistoryToTheFirstLoggedUser to 'Add' b. In the OPAC make sure you are logged out c. Perform some searches d. Check search history is recorded by selecting "Search history" in the top-right navigation menu e. Log in as a user f. Check search history recorded (From the top-right navigation menu select Welcome, [username] > Search history) g. Search history should be populated with searches from when not logged in h. Select all then select 'Remove selected searches' i. Change LoadSearchHistoryToTheFirstLoggedUser to 'Don't add' j. Repeat from b-g k. Search history will still be shown, even though it should be empty 2. Check database to see what value is recorded: a. koha-mysql kohadev b. SQL query: select * from systempreferences where variable='LoadSearchHistoryToTheFirstLoggedUser'; c. When LoadSearchHistoryToTheFirstLoggedUser is set to 'Don't add' value = "no" 3. Apply the patch. 4. Update the database (updatedatabase) and restart services (restart_all) 5. Check value in database again from step 2 - value should now be 0 6. Repeat the steps from 1 - this time it should work as expected, that is: . When LoadSearchHistoryToTheFirstLoggedUser is set to 'Don't add' . Search history should be empty when you log in 7. Sign-off! Created attachment 125856 [details] [review] Bug 29138: Use zero instead of no in LoadSearchHistoryToTheFirstLoggedUser To test: - apply patch - set LoadSearchHistoryToTheFirstLoggedUser to 'Don't add' - confirm via sql that value=0 - confirm on OPAC that search history is not loaded to account on login Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Caused by commit 1fe1b704f9a81172f2a6b86367b314572a854d18 Bug 22824: Fix yes/no vs 1/0 UPDATE systempreferences SET value= IF(value='no',0,1) What if it's 0 already? :) (In reply to Jonathan Druart from comment #6) > UPDATE systempreferences SET value= IF(value='no',0,1) > > What if it's 0 already? :) Good catch! Created attachment 125886 [details] [review] Bug 29138: Use zero instead of no in LoadSearchHistoryToTheFirstLoggedUser To test: - apply patch - set LoadSearchHistoryToTheFirstLoggedUser to 'Don't add' - confirm via sql that value=0 - confirm on OPAC that search history is not loaded to account on login Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Updated to: UPDATE systempreferences SET value= IF(value in (0,'no'),0,1) WHERE variable = 'LoadSearchHistoryToTheFirstLoggedUser'; (In reply to Andrew Fuerste-Henry from comment #9) > Updated to: > UPDATE systempreferences SET value= IF(value in (0,'no'),0,1) WHERE variable > = 'LoadSearchHistoryToTheFirstLoggedUser'; Does it work? MariaDB [koha_kohadev]> select value from systempreferences where variable="LoadSearchHistoryToTheFirstLoggedUser"; +-------+ | value | +-------+ | no | +-------+ 1 row in set (0.001 sec) MariaDB [koha_kohadev]> UPDATE systempreferences SET value=IF(value in (0,'no'),0,1) WHERE variable = 'LoadSearchHistoryToTheFirstLoggedUser'; ERROR 1292 (22007): Truncated incorrect DOUBLE value: 'no' Huh, I didn't get that error: MariaDB [koha_kohadev]> select value from systempreferences where variable="LoadSearchHistoryToTheFirstLoggedUser"; +-------+ | value | +-------+ | no | +-------+ 1 row in set (0.000 sec) MariaDB [koha_kohadev]> UPDATE systempreferences SET value= IF(value in (0,'no'),0,1) WHERE variable = 'LoadSearchHistoryToTheFirstLoggedUser'; Query OK, 1 row affected, 1 warning (0.003 sec) Rows matched: 1 Changed: 1 Warnings: 1 MariaDB [koha_kohadev]> select value from systempreferences where variable="LoadSearchHistoryToTheFirstLoggedUser"; +-------+ | value | +-------+ | 0 | +-------+ 1 row in set (0.001 sec) Created attachment 125922 [details] [review] Bug 29138: Use zero instead of no in LoadSearchHistoryToTheFirstLoggedUser To test: - apply patch - set LoadSearchHistoryToTheFirstLoggedUser to 'Don't add' - confirm via sql that value=0 - confirm on OPAC that search history is not loaded to account on login Signed-off-by: David Nind <david@davidnind.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Created attachment 125923 [details] [review] Bug 29138: (QA follow-up) Changes to UPDATE Move the condition to the WHERE clause. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested on NULL, 0, 1, 2, no, yes. (2 and yes become 0 too. Fine.) Created attachment 125924 [details] [review] Bug 29138: (QA follow-up) Changes to UPDATE Move the condition to the WHERE clause. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested on NULL, 0, 1, 2, no, yes. (2 and yes become 0 too. Fine.) Replaced IFNULL by COALESCE. Created attachment 125925 [details] [review] Bug 29138: (QA follow-up) Changes to UPDATE Move the condition to the WHERE clause. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested on NULL, 0, 1, 2, no, yes. (2 and yes become 0 too. Fine.) Replaced IFNULL by COALESCE. Still not working for me
ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Truncated incorrect DOUBLE value: 'no' at /kohadevbox/koha/C4/Installer.pm line 736
10.5.12-MariaDB-1
Why not simply:
UPDATE systempreferences SET value=0 WHERE variable='LoadSearchHistoryToTheFirstLoggedUser' and value="no";
?
Created attachment 125992 [details] [review] Bug 29138: (QA follow-up) Changes to UPDATE Move the condition to the WHERE clause, using COALESCE to prevent mysqlism (IFNULL) and strings for strict SQL. Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Tested on NULL, 0, 1, 2, no, yes. (2 and yes become 0 too. Fine.) Tested strict sql mode: no truncated incorrect DOUBLE value-error anymore. (In reply to Jonathan Druart from comment #16) > Still not working for me > > ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Truncated > incorrect DOUBLE value: 'no' at /kohadevbox/koha/C4/Installer.pm line 736 > > 10.5.12-MariaDB-1 Obscure message, caused by strict SQL. > Why not simply: > > UPDATE systempreferences SET value=0 WHERE > variable='LoadSearchHistoryToTheFirstLoggedUser' and value="no"; Current condition catches everything outside '1' (In reply to Marcel de Rooy from comment #18) > (In reply to Jonathan Druart from comment #16) > > Still not working for me > > > > ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Truncated > > incorrect DOUBLE value: 'no' at /kohadevbox/koha/C4/Installer.pm line 736 > > > > 10.5.12-MariaDB-1 > > Obscure message, caused by strict SQL. > > > Why not simply: > > > > UPDATE systempreferences SET value=0 WHERE > > variable='LoadSearchHistoryToTheFirstLoggedUser' and value="no"; > > Current condition catches everything outside '1' Yes, and so it catches "yes". I won't block the patch because of that, but I don't understand how it can be more correct than setting to "0" when was "no". The problem was with "no": - no : "Don't add" + 0: "Don't add" Changing my mind, the bug title and DBrev description are "Use a zero instead of a 'no' in LoadSearchHistoryToTheFirstLoggedUser". It's what we must do, nothing else. DB rev will be changed to: UPDATE systempreferences SET value='0' WHERE variable='LoadSearchHistoryToTheFirstLoggedUser' AND value='no'; Pushed to master for 21.11, thanks to everybody involved! Thanks, Jonathan and everybody! |