OCA / server-ux

GNU Affero General Public License v3.0
163 stars 531 forks source link

[13.0][FIX] mass_editing: fix upgrade on empty db #897

Closed maisim closed 4 months ago

maisim commented 5 months ago

Fix potential sql error when upgrading an db wich have no mass editing record

maisim commented 5 months ago

cc @legalsylvain

rvalyi commented 4 months ago

Hello, I confirm the bug and I agree with the fix.

However initially I fixed it on my side a bit more cleanly using openupgrade.table_exists. I'm posting my own fix here for the record, but IMHO it doesn't matter much as this migration code is not propagated to more recent versions. So I'll even approve this PR.

diff --git a/mass_editing/migrations/13.0.1.0.0/post-migration.py b/mass_editing/migrations/13.0.1.0.0/post-migration.py
index a4c098fe..e775d76d 100644
--- a/mass_editing/migrations/13.0.1.0.0/post-migration.py
+++ b/mass_editing/migrations/13.0.1.0.0/post-migration.py
@@ -1,6 +1,7 @@
 # Copyright (C) 2019 - Today: GRAP (http://www.grap.coop)
 # @author: Sylvain LE GAL (https://twitter.com/legalsylvain)
 # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html).
+from openupgradelib import openupgrade

 def migrate(cr, version):
@@ -8,7 +9,7 @@ def migrate(cr, version):
         return
     # Don't execute if already migrated in v12
     cr.execute("SELECT COUNT(*) FROM mass_editing_line")
-    if cr.fetchone()[0] > 0:
+    if cr.fetchone()[0] > 0 or not openupgrade.table_exists(cr, "mass_field_rel"):
         return
     # Rename table for consistency reason
     cr.execute(
legalsylvain commented 4 months ago

hi @maisim : that's a duplicate of https://github.com/OCA/server-ux/pull/835 Don't you think ?

thanks.

rvalyi commented 4 months ago

hi @maisim : that's a duplicate of #835 Don't you think ?

thanks.

I'll test with https://github.com/OCA/server-ux/pull/835 in a few minutes and confirm here.

rvalyi commented 4 months ago

I confirm #835 already fixed the issue (that's crazy: I git the bug and fixed it yesterday, using the code from just a fez hours before the fix was merged). So closing. Feel free to re-open if you don't agree.

maisim commented 4 months ago

hi @maisim : that's a duplicate of #835 Don't you think ?

Oh I didn't see that sorry