Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RFR] OpenUpgrade framework patches #3

Conversation

StefanRijnhart
Copy link

No description provided.

@StefanRijnhart StefanRijnhart force-pushed the rfr/14.0/openupgrade_framework branch from 151fbf4 to 447b0e7 Compare December 6, 2020 12:55
Copy link

@yajo yajo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I love these kind of PRs:

imagen

There's one blocking point, the rest are suggestions/questions.

Comment on lines +9 to +55
def _drop_table(self):
""" Never drop tables """
for model in self:
if self.env.get(model.model) is not None:
openupgrade.message(
self.env.cr, "Unknown", False, False,
"Not dropping the table or view of model %s", model.model)


def _drop_column(self):
""" Never drop columns """
for field in self:
if field.name in models.MAGIC_COLUMNS:
continue
openupgrade.message(
self.env.cr, "Unknown", False, False,
"Not dropping the column of field %s of model %s", field.name,
field.model,
)
continue


IrModel._drop_column = _drop_column
IrModel._drop_table = _drop_table


@api.model
def _process_end(self, modules):
""" Don't warn about upgrade conventions from Odoo
('fields should be explicitely removed by an upgrade script')
"""
with mute_logger('odoo.addons.base.models.ir_model'):
return IrModelData._process_end._original_method(self, modules)


_process_end._original_method = IrModelData._process_end
IrModelData._process_end = _process_end


def _module_data_uninstall(self):
""" Don't delete many2many relation tables. Only unlink the
ir.model.relation record itself.
"""
self.unlink()


IrModelRelation._module_data_uninstall = _module_data_uninstall
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't you be using OdooPatch?

Comment on lines +38 to +41
_check_xml._original_method = View._check_xml
View._check_xml = _check_xml
handle_view_error._original_method = View.handle_view_error
View.handle_view_error = handle_view_error
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, shouldn't you use OdooPatch?

Comment on lines +20 to +28
savepoint = str(uuid4)
try:
self.env.cr.execute('SAVEPOINT "%s"' % savepoint)
return BaseModel.unlink._original_method(self)
except Exception as e:
self.env.cr.execute('ROLLBACK TO SAVEPOINT "%s"' % savepoint)
_logger.warning(
"Could not delete obsolete record with ids %s of model %s: %s",
self.ids, self._name, e)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You could just use with self.env.cr.savepoint(): ...

Comment on lines +32 to +33
unlink._original_method = BaseModel.unlink
BaseModel.unlink = unlink
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same point about OdooPatch

if (
"base" in self
and self["base"].dbdemo
and self["base"].installed_version < odoo.release.major_version
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 Beware! You have to use a smart version comparison algorithm. Check out odoo.tools.parse_version.

Comment on lines +20 to +21
update_from_db._original_method = Graph.update_from_db
Graph.update_from_db = update_from_db
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same about OdooPatch

@legalsylvain
Copy link
Owner

I'm fast merging all the PR, because it make more complex the developpement to have multiple PR, even if @yajo could be valid.

@legalsylvain legalsylvain merged commit a3c72ae into legalsylvain:14.0-upgrade-pocalypse Dec 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants