Bug 36986 - (Bug 26176 follow-up) Fix rename StaffLoginBranchBasedOnIP in DBRev
Summary: (Bug 26176 follow-up) Fix rename StaffLoginBranchBasedOnIP in DBRev
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Installation and upgrade (command-line installer) (show other bugs)
Version: Main
Hardware: All All
: P5 - low blocker (vote)
Assignee: Fridolin Somers
QA Contact: Martin Renvoize
URL:
Keywords:
Depends on: 26176
Blocks: 37106
  Show dependency treegraph
 
Reported: 2024-05-29 15:31 UTC by Fridolin Somers
Modified: 2024-06-17 20:49 UTC (History)
4 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
24.11.00,24.05.01,23.11.06


Attachments
Bug 36986: (Bug 26176 follow-up) Fix rename StaffLoginBranchBasedOnIP in BDRev (1.58 KB, patch)
2024-05-29 15:36 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 36986: (Bug 26176 follow-up) Fix rename StaffLoginBranchBasedOnIP in BDRev (1.62 KB, patch)
2024-05-30 01:54 UTC, David Nind
Details | Diff | Splinter Review
Bug 36986: (Bug 26176 follow-up) Fix rename StaffLoginBranchBasedOnIP in BDRev (1.68 KB, patch)
2024-05-30 20:02 UTC, Victor Grousset/tuxayo
Details | Diff | Splinter Review
Bug 36986: (Bug 26176 follow-up) Fix rename StaffLoginBranchBasedOnIP in BDRev (1.75 KB, patch)
2024-06-04 10:17 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 36986: (QA follow-up) Catch the case where someone already upgraded (1.57 KB, patch)
2024-06-04 10:17 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Fridolin Somers 2024-05-29 15:31:30 UTC
Bug 26176 follow-up to fix rename StaffLoginBranchBasedOnIP in BBRev :
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=26176#c24
Comment 1 Fridolin Somers 2024-05-29 15:36:08 UTC
Created attachment 167247 [details] [review]
Bug 36986: (Bug 26176 follow-up) Fix rename StaffLoginBranchBasedOnIP in BDRev

Test by running upgrade from 23.11.00 to main
Check you see :
Upgrade to 23.12.00.061  [15:34:36]: Bug 26176 - Rename AutoLocation and StaffLoginBranchBasedOnIP system preferences
	Renamed system preference 'AutoLocation' to 'StaffLoginRestrictLibraryByIP'
	Renamed system preference 'StaffLoginBranchBasedOnIP' to 'StaffLoginLibraryBasedOnIP'
Comment 2 Fridolin Somers 2024-05-29 15:42:12 UTC
Double check with mysql after upgrade :

mysql> select variable,value from systempreferences where variable like 'StaffLogin%IP';
+-------------------------------+-------+
| variable                      | value |
+-------------------------------+-------+
| StaffLoginLibraryBasedOnIP    | 0     |
| StaffLoginRestrictLibraryByIP | 0     |
+-------------------------------+-------+
2 rows in set (0.00 sec)
Comment 3 David Nind 2024-05-30 01:54:34 UTC
Created attachment 167253 [details] [review]
Bug 36986: (Bug 26176 follow-up) Fix rename StaffLoginBranchBasedOnIP in BDRev

Test by running upgrade from 23.11.00 to main
Check you see :
Upgrade to 23.12.00.061  [15:34:36]: Bug 26176 - Rename AutoLocation and StaffLoginBranchBasedOnIP system preferences
	Renamed system preference 'AutoLocation' to 'StaffLoginRestrictLibraryByIP'
	Renamed system preference 'StaffLoginBranchBasedOnIP' to 'StaffLoginLibraryBasedOnIP'

Signed-off-by: David Nind <david@davidnind.com>
Comment 4 David Nind 2024-05-30 01:58:23 UTC
I'm not sure that I've tested it right, so maybe someone else who knows how to replicate an upgrade properly in KTD should test as well.

On a maybe semi-related note, KTD has StaffLoginLibraryBasedOnIP enabled by default since bug 26176. I don't think it should be - it never was in the past. Happy to log a separate bug for that if required.
Comment 5 Victor Grousset/tuxayo 2024-05-30 20:02:25 UTC
Before Bug 26176

select variable,value from systempreferences
where variable like 'StaffLogin%IP';
| StaffLoginBranchBasedOnIP | 1     |
select variable,value from systempreferences
where variable like 'AutoLo%';
| AutoLocation | 0     |


---------------------
After Bug 26176 (on v24.05.00) (SO WITHOUT THE PATCH, this is what will happen to people having already upgraded)

Upgrade to 23.12.00.061  [19:45:10]: Bug 26176 - Rename AutoLocation and StaffLoginBranchBasedOnIP system preferences
	Renamed system preference 'AutoLocation' to 'StaffLoginRestrictLibraryByIP'
Upgrade to 24.05.00.000  [19:45:10]: Koha 24.05.00 release

select variable,value from systempreferences
where variable like 'StaffLogin%IP';
| StaffLoginBranchBasedOnIP     | 1     |
| StaffLoginRestrictLibraryByIP | 0     |
select variable,value from systempreferences
where variable like 'AutoLo%';
Empty set (0.000 sec)


--------
WITH THE PATCH, so direct upgrade from before Bug 26176 to main + this patch
select variable,value from systempreferences
where variable like 'StaffLogin%IP';
| StaffLoginLibraryBasedOnIP    | 1     |
| StaffLoginRestrictLibraryByIP | 0     |
select variable,value from systempreferences
    -> where variable like 'AutoLo%';
Empty set (0.001 sec)


Seems to work as advertised but I'm way to confused about these sysprefs renames ^^"

----


Is that out of scope if this ticket to handle the instances that upgraded to v24.05.00 ?
Comment 6 Victor Grousset/tuxayo 2024-05-30 20:02:43 UTC
Created attachment 167286 [details] [review]
Bug 36986: (Bug 26176 follow-up) Fix rename StaffLoginBranchBasedOnIP in BDRev

Test by running upgrade from 23.11.00 to main
Check you see :
Upgrade to 23.12.00.061  [15:34:36]: Bug 26176 - Rename AutoLocation and StaffLoginBranchBasedOnIP system preferences
	Renamed system preference 'AutoLocation' to 'StaffLoginRestrictLibraryByIP'
	Renamed system preference 'StaffLoginBranchBasedOnIP' to 'StaffLoginLibraryBasedOnIP'

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Comment 7 Fridolin Somers 2024-06-03 14:22:40 UTC

> Is that out of scope if this ticket to handle the instances that upgraded to v24.05.00 ?
This fix will be in first 24.05 release coming soon.
Comment 8 Victor Grousset/tuxayo 2024-06-03 17:55:36 UTC
24.05.00 is already out so having this in 24.05.01 isn't enough.

Anyway, this is still ready for the QA step.
Comment 9 Martin Renvoize 2024-06-04 10:17:34 UTC
Created attachment 167371 [details] [review]
Bug 36986: (Bug 26176 follow-up) Fix rename StaffLoginBranchBasedOnIP in BDRev

Test by running upgrade from 23.11.00 to main
Check you see :
Upgrade to 23.12.00.061  [15:34:36]: Bug 26176 - Rename AutoLocation and StaffLoginBranchBasedOnIP system preferences
	Renamed system preference 'AutoLocation' to 'StaffLoginRestrictLibraryByIP'
	Renamed system preference 'StaffLoginBranchBasedOnIP' to 'StaffLoginLibraryBasedOnIP'

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2024-06-04 10:17:37 UTC
Created attachment 167372 [details] [review]
Bug 36986: (QA follow-up) Catch the case where someone already upgraded

This adds a corresponding atomic update for the case where someone may
have already upgraded past the version we're correcting in the prior
patch.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 11 Fridolin Somers 2024-06-04 13:48:52 UTC
Thanks a lot Martin ;)
Comment 12 Martin Renvoize 2024-06-05 13:38:21 UTC
Pushed to main for 24.11.00, thanks all.
Comment 13 Martin Renvoize 2024-06-05 16:37:06 UTC
More work needed here, Jenkins doesn't like it.. but I'm a bit confused to why.. we appear to already have the key before the update runs so it fails unique constraints..
Comment 14 Martin Renvoize 2024-06-05 16:37:48 UTC
>15:21:04 koha_1       | Upgrade to 23.12.00.060  [14:21:01]: Bug 36665 - Add system preference StaffLoginBranchBasedOnIP
15:21:04 koha_1       | 	Added new system preference 'StaffLoginBranchBasedOnIP'
15:21:04 koha_1       | Upgrade to 23.12.00.061  [14:21:01]: Bug 26176 - Rename AutoLocation and StaffLoginBranchBasedOnIP system preferences
15:21:04 koha_1       | ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate entry 'StaffLoginLibraryBasedOnIP' for key 'PRIMARY' at /kohadevbox/koha/C4/Installer.pm line 741
Comment 15 Victor Grousset/tuxayo 2024-06-05 23:57:44 UTC
The CI does:

> Upgrade to 23.12.00.059  [18:09:07]: Bug 36819 - Change barcode width value if it still has the wrong default value
> 	[33mChanged the barcode width in patron card creator default layout from 8% to 80%.[0m
> Upgrade to 23.12.00.060  [18:09:07]: Bug 36665 - Add system preference StaffLoginBranchBasedOnIP
> 	Added new system preference 'StaffLoginBranchBasedOnIP'
> Upgrade to 23.12.00.061  [18:09:07]: Bug 26176 - Rename AutoLocation and StaffLoginBranchBasedOnIP system preferences
> 	[32mRenamed system preference 'StaffLoginBranchBasedOnIP' to 'StaffLoginLibraryBasedOnIP'[0m
> Upgrade to 24.05.00.000  [18:09:07]: Koha 24.05.00 release
> Upgrade to 24.06.00.000  [18:09:07]: Increase DBRev for 24.06
> 	🎵 Mamma mia, here we go again!
> Upgrade to 24.06.00.001  [18:09:07]: Bug 36986 - Rename StaffLoginLibraryBasedOnIP system preference
> ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do failed: Duplicate entry 'StaffLoginLibraryBasedOnIP' for key 'PRIMARY' at /kohadevbox/koha/C4/Installer.pm line 741

And on my PC setting up a DB before Bug 26176 and doing the updatedatabase to main (Bug 36986L (follow-up) Ensure idempotency) does this:

> 	Changed the barcode width in patron card creator default layout from 8% to 80%.
> Upgrade to 23.12.00.060  [21:09:31]: Bug 36665 - Add system preference StaffLoginBranchBasedOnIP
> 	Added new system preference 'StaffLoginBranchBasedOnIP'
> Upgrade to 23.12.00.061  [21:09:31]: Bug 26176 - Rename AutoLocation and StaffLoginBranchBasedOnIP system preferences
> 	Renamed system preference 'AutoLocation' to 'StaffLoginRestrictLibraryByIP'
> 	Renamed system preference 'StaffLoginBranchBasedOnIP' to 'StaffLoginLibraryBasedOnIP'
> Upgrade to 24.05.00.000  [21:09:31]: Koha 24.05.00 release
> Upgrade to 24.06.00.000  [21:09:31]: Increase DBRev for 24.06
> 	🎵 Mamma mia, here we go again!
> Upgrade to 24.06.00.001  [21:09:31]: Bug 36986 - Rename StaffLoginLibraryBasedOnIP system preference

The difference is that my local run has «Renamed system preference 'AutoLocation' to 'StaffLoginRestrictLibraryByIP'» which the CI doesn't have (why? :o ) and not the error that the CI sees at the end.

I also got the same with MariaDB 10.5 as the CI run:
https://jenkins.koha-community.org/view/main/job/Koha_Main/2927/consoleText
Comment 16 Fridolin Somers 2024-06-06 08:01:16 UTC
(In reply to Martin Renvoize from comment #14)
> >15:21:04 koha_1       | Upgrade to 23.12.00.060  [14:21:01]: Bug 36665 - Add system preference StaffLoginBranchBasedOnIP
> 15:21:04 koha_1       | 	Added new system preference
> 'StaffLoginBranchBasedOnIP'
> 15:21:04 koha_1       | Upgrade to 23.12.00.061  [14:21:01]: Bug 26176 -
> Rename AutoLocation and StaffLoginBranchBasedOnIP system preferences
> 15:21:04 koha_1       | ERROR - {UNKNOWN}: DBI Exception: DBD::mysql::db do
> failed: Duplicate entry 'StaffLoginLibraryBasedOnIP' for key 'PRIMARY' at
> /kohadevbox/koha/C4/Installer.pm line 741

Ah but this will be OK when first patch is backported to 23.11.x right ?
It will fix installer/data/mysql/db_revs/231105008.pl
Comment 17 Martin Renvoize 2024-06-06 08:07:08 UTC
I've pushed a couple of additional follow-ups now. Jenkins is happier again.

The issue was that mysql/Maria does the primary key unique check before applying the WHERE part of the UPDATE clause.  As such the update wasn't fully idempotent and would throw a breaking error.

I updated the statement to UPDATE IGNORE which is perhaps a MySQLism, but seems to work and can be corrected later if we do really ever want to support Postgres etc
Comment 18 Fridolin Somers 2024-06-06 08:40:00 UTC
Just to alert :
Commit in main : 1c779aaca8 Bug 36986: DBRev 23.12.00.023
Only commit message is a mistake, it is well 24.06.00.001 with db_revs/240600001.pl
Comment 19 Fridolin Somers 2024-06-06 08:46:21 UTC
Pushed to 24.05.x for 24.05.01
Comment 20 Fridolin Somers 2024-06-06 08:59:00 UTC
Pushed to 23.11.x for 23.11.06
Comment 21 Victor Grousset/tuxayo 2024-06-06 20:41:59 UTC
Nice, thanks Frido and Martin :D