The last commit attempted to make the update idempotent, however, it missed a case. If the update is run, then a patron has the permission removed, they will get it again on a subsequent update. We should check if the permission exists in the permissions table, and only perform the first two updates if it does not
Created attachment 154702 [details] [review] Bug 34589: Only update users if permission is added
Can we get a test plan for this one?
Created attachment 155271 [details] [review] Bug 34589: Only update users if permission is added To test: 1 - Assign a user 'edit_items' permission 2 - Manually run the update perl -e 'use C4::Installer; warn Data::Dumper::Dumper(C4::Installer::run_db_rev("installer/data/mysql/db_revs/221200003.pl"));' 3 - Edit patron permissions 4 - Note they now have edit_any_item permission 5 - Remove that permission 6 - Run the update again 7 - Edit patron permissions 8 - They have edit_any_item again - remove it 9 - Apply patch 10 - Run the update 11 - Edit patron permissions 12 - No new permissions added! 13 - sudo koha-mysql kohadev DELETE FROM permissions WHERE code = 'edit_any_item' 14 - Run the update again 15 - Edit patron permissions 16 - They do have edit_any_item
Created attachment 155501 [details] [review] Bug 34589: Only update users if permission is added To test: 1 - Assign a user 'edit_items' permission 2 - Manually run the update perl -e 'use C4::Installer; warn Data::Dumper::Dumper(C4::Installer::run_db_rev("installer/data/mysql/db_revs/221200003.pl"));' 3 - Edit patron permissions 4 - Note they now have edit_any_item permission 5 - Remove that permission 6 - Run the update again 7 - Edit patron permissions 8 - They have edit_any_item again - remove it 9 - Apply patch 10 - Run the update 11 - Edit patron permissions 12 - No new permissions added! 13 - sudo koha-mysql kohadev DELETE FROM permissions WHERE code = 'edit_any_item' 14 - Run the update again 15 - Edit patron permissions 16 - They do have edit_any_item Signed-off-by: Émily-Rose Francoeur <emily-rose.francoeur@inLibro.com>
I wont object to this patch, but I think it is arguable. Insert in permissions, insert in userpermissions. Repeat does it again. Feels quite idempotent imo.
And another note: DBI::do does not promise a good return: Returns the number of rows affected or undef on error. A return value of -1 means the number of rows is not known, not applicable, or not available.
Created attachment 155634 [details] [review] Bug 34589: Only update users if permission is added To test: 1 - Assign a user 'edit_items' permission 2 - Manually run the update perl -e 'use C4::Installer; warn Data::Dumper::Dumper(C4::Installer::run_db_rev("installer/data/mysql/db_revs/221200003.pl"));' 3 - Edit patron permissions 4 - Note they now have edit_any_item permission 5 - Remove that permission 6 - Run the update again 7 - Edit patron permissions 8 - They have edit_any_item again - remove it 9 - Apply patch 10 - Run the update 11 - Edit patron permissions 12 - No new permissions added! 13 - sudo koha-mysql kohadev DELETE FROM permissions WHERE code = 'edit_any_item' 14 - Run the update again 15 - Edit patron permissions 16 - They do have edit_any_item Signed-off-by: Émily-Rose Francoeur <emily-rose.francoeur@inLibro.com> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Pushed to master for 23.11. Nice work everyone, thanks!
Pushed to 23.05.x for 23.05.04
Bug 202356 is not in 22.11.x, not backporting