-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
[18.0][MIG] base_view_inheritance_extension #3100
base: 18.0
Are you sure you want to change the base?
[18.0][MIG] base_view_inheritance_extension #3100
Conversation
Trivial changes: * Version in README changed * Version in manifest changed OCA Transbot updated translations from Transifex OCA Transbot updated translations from Transifex
…extension. (OCA#804) * Add list_add operation. * Add list_remove operation. OCA Transbot updated translations from Transifex
expression not match the node any more. Fixes OCA#885
* warning about dynamic context * import for safe_eval
Use https in URL to licence
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: server-tools-13.0/server-tools-13.0-base_view_inheritance_extension Translate-URL: https://translation.odoo-community.org/projects/server-tools-13-0/server-tools-13-0-base_view_inheritance_extension/
* Removed `list_add` and `list_remove`, they've been deprecated and implemented in Odoo core since several versions ago. * Removed `move`, as it has also already been implemented in core several versions ago. * Replaced `python_dict` by `update`, that performs an operation similar to :meth:`dict.update`, but on the ast.Dict.
Currently translated at 100.0% (3 of 3 strings) Translation: server-tools-16.0/server-tools-16.0-base_view_inheritance_extension Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-base_view_inheritance_extension/hr/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: server-tools-16.0/server-tools-16.0-base_view_inheritance_extension Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-base_view_inheritance_extension/
Currently translated at 100.0% (4 of 4 strings) Translation: server-tools-16.0/server-tools-16.0-base_view_inheritance_extension Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-base_view_inheritance_extension/es_AR/
Currently translated at 100.0% (4 of 4 strings) Translation: server-tools-16.0/server-tools-16.0-base_view_inheritance_extension Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-base_view_inheritance_extension/es/
…out previous domain defined
Currently translated at 100.0% (4 of 4 strings) Translation: server-tools-16.0/server-tools-16.0-base_view_inheritance_extension Translate-URL: https://translation.odoo-community.org/projects/server-tools-16-0/server-tools-16-0-base_view_inheritance_extension/it/
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate. Translation: server-tools-17.0/server-tools-17.0-base_view_inheritance_extension Translate-URL: https://translation.odoo-community.org/projects/server-tools-17-0/server-tools-17-0-base_view_inheritance_extension/
c57e742
to
c666397
Compare
c666397
to
3251ed6
Compare
@@ -104,7 +104,7 @@ def test_update_context_complex(self): | |||
] | |||
self.assertEqual( | |||
res.xpath('//field[@name="invoice_line_ids"]')[0].attrib["context"], | |||
"{%s}" % ", ".join(expected_items), | |||
"{%s}" % ", ".join(expected_items), # noqa: UP031 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is easy enough to avoid needing and we should try as hard as possible to only have absolutely necessary linting exceptions IMO. Odoo's own CI now fails if a linting exception is used after some high profile security issues introduced. That said I am not sure the current state of f-string vs format vs printf discussions in OCA. Last I knew it was f-string bad, printf bad, use format. Maybe someone who knows better can help here as a few f-string conversions up higher.
Anyway it won't stop me installing and testing so will give it a go now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested context update using https://github.com/odoonz/odoonz-addons/tree/18.0/stock_prodlot_qty - which has multiple upstream views with context - works as expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Make secure, also concerns around tests and applicability IRT attrs, which may explain lack of test coverage of all the main methods in module.
@api.model | ||
def _get_inheritance_handler_attributes(self, node): | ||
handler = super().apply_inheritance_specs | ||
if hasattr(self, f"inheritance_handler_attributes_{node.get('operation')}"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should check/call a private method
return handler | ||
|
||
@api.model | ||
def inheritance_handler_attributes_update(self, source, specs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be private
@api.model | ||
def _get_inheritance_handler_attributes(self, node): | ||
handler = super().apply_inheritance_specs | ||
if hasattr(self, f"inheritance_handler_attributes_{node.get('operation')}"): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if hasattr(self, f"inheritance_handler_attributes_{node.get('operation')}"): | |
if hasattr(self, f"_inheritance_handler_attributes_{node.get('operation')}"): |
handler = super().apply_inheritance_specs | ||
if hasattr(self, f"inheritance_handler_attributes_{node.get('operation')}"): | ||
handler = getattr( | ||
self, f"inheritance_handler_attributes_{node.get('operation')}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self, f"inheritance_handler_attributes_{node.get('operation')}" | |
self, f"_inheritance_handler_attributes_{node.get('operation')}" |
return handler | ||
|
||
@api.model | ||
def inheritance_handler_attributes_update(self, source, specs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def inheritance_handler_attributes_update(self, source, specs): | |
def _inheritance_handler_attributes_update(self, source, specs): |
self.var2str_domain_text(old_value.strip()) | ||
) | ||
new_domain = ast.literal_eval( | ||
self.var2str_domain_text(attribute_node.text.strip()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
self.var2str_domain_text(attribute_node.text.strip()) | |
self._var2str_domain_text(attribute_node.text.strip()) |
return re.sub(regex, r"'\1_is_a_var_to_replace'", domain_str) | ||
|
||
@api.model | ||
def str2var_domain_text(self, domain_str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def str2var_domain_text(self, domain_str): | |
def _str2var_domain_text(self, domain_str): |
return source | ||
|
||
@api.model | ||
def var2str_domain_text(self, domain_str): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def var2str_domain_text(self, domain_str): | |
def _var2str_domain_text(self, domain_str): |
new_value = str(expression.OR([old_domain, new_domain])) | ||
else: | ||
new_value = str(expression.AND([old_domain, new_domain])) | ||
new_value = self.str2var_domain_text(new_value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
new_value = self.str2var_domain_text(new_value) | |
new_value = self._str2var_domain_text(new_value) |
<form> | ||
<field | ||
name="ref" | ||
attrs="{'invisible': [('state', '=', 'draft')]}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test makes no sense? - attrs is gone in v18. Hmm, this might have big consequences for module - all these tests containing attrs I think will need changing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For being easily searchable and avoiding to duplicate efforts, please name the pull request following this pattern: [18.0][MIG] : Migration to 18.0.
and please check the CI
@xaviedoanhduy Maybe you could help him? I mean I've done this a long time, and I have no idea why a set of unrelated modules caused errors like that or what I would need to do to fix |
@gdgellatly,I'm going to modify to use a private method, thank you for your comments and I'm a little confused about the modifications needed for the CI. |
No description provided.