-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
[16.0][MIG] attachment_delete_restrict: Migration to 16.0 #2673
base: 16.0
Are you sure you want to change the base?
[16.0][MIG] attachment_delete_restrict: Migration to 16.0 #2673
Conversation
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.
@AungKoKoLin1997 I think some of the changes should be in separate commits to be backported to 14.0.
@@ -33,7 +33,7 @@ def _raise_delete_attachment_error(self, allowed_users): | |||
"You are not allowed to delete this attachment.\n\nUsers with " | |||
"the delete permission:\n%s" | |||
) | |||
% ("\n".join(allowed_users.mapped("name")) or "None") | |||
% ("\n".join(list(set(allowed_users.mapped("name")))) or "None") |
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 set()
should be done when allowed_users
is updated in _check_custom_delete_attachment()
instead. It should perhaps be in a separate commit to be backported.
) | ||
|
||
@api.onchange("restrict_delete_attachment") | ||
def _clear_groups_users(self): |
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 _clear_groups_users(self): | |
def _onchange_restrict_delete_attachment(self): |
I suppose this should also be in a separate commit to be backported.
"the models " | ||
"assigned here.", |
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.
"the models " | |
"assigned here.", | |
"the models assigned here. Restrict Attachment Deletion must be 'Custom' or 'Owner + Custom' for the model to be selected here.", |
def write(self, vals): | ||
res = super(ResGroups, self).write(vals) | ||
if "delete_attachment_model_ids" in vals: | ||
for group in self: | ||
for model in group.delete_attachment_model_ids: | ||
if group not in model.delete_attachment_group_ids: | ||
model.write( | ||
{ | ||
"delete_attachment_group_ids": [(4, group.id)], | ||
} | ||
) | ||
return res |
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.
Can you explain why this should be needed?
def write(self, vals): | ||
res = super(ResUsers, self).write(vals) | ||
if "delete_attachment_model_ids" in vals: | ||
for user in self: | ||
for model in user.delete_attachment_model_ids: | ||
if user not in model.delete_attachment_user_ids: | ||
model.write( | ||
{ | ||
"delete_attachment_user_ids": [(4, user.id)], | ||
} | ||
) | ||
return res |
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.
Why do we need this?
3840945
to
2fdfe5f
Compare
if self.env.user not in allowed_users: | ||
return self._raise_delete_attachment_error(allowed_users) | ||
|
||
def unlink(self): | ||
res_models = list(set(self.filtered("res_model").mapped("res_model"))) | ||
if res_models: | ||
models = self.env["ir.model"].search([("model", "in", res_models)]) | ||
models = self.env["ir.model"].sudo().search([("model", "in", res_models)]) |
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 should be part of the migration, due to the ACL change in 16.0 (or 15.0, I haven't checked).
2fdfe5f
to
ca37da1
Compare
Currently translated at 100.0% (37 of 37 strings) Translation: server-tools-14.0/server-tools-14.0-attachment_delete_restrict Translate-URL: https://translation.odoo-community.org/projects/server-tools-14-0/server-tools-14-0-attachment_delete_restrict/es_AR/
- Remove the display of duplicate allowed user names. - Correct the many2many field relation - Correct domain values and add the many2many_tags widget in views.
ca37da1
to
f0c88d3
Compare
hi @yostashiro. Thanks for your review. could you update it ? |
f0c88d3
to
434dc12
Compare
@OCA-server-tools-maintainers |
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.
Code review and functional test. LGTM.
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.
Code and Function : LGTM
@AungKoKoLin1997 |
- Add _onchange_restrict_delete_attachment() to clear groups and users when restrict_delete_attachment is not set to custom or owner_custom.
434dc12
to
dfd3a27
Compare
This PR has the |
@qrtl
This PR encompasses the following steps:
- Removal of the display of duplicate allowed user names.
- Correct the many2many field relation
- Correction of domain values and addition of the many2many_tags widget in views.
- Addition of _onchange_restrict_delete_attachment() to clear groups and users when restrict_delete_attachment is not set to custom or owner_custom.