-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[MIG] Migrate module attachment_base_synchronize to v10 #763
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,12 +1,10 @@ | ||
| <?xml version="1.0"?> | ||
| <openerp> | ||
| <data noupdate="1"> | ||
| <odoo noupdate="1"> | ||
|
|
||
| <record id="attachment_metadata" model="ir.attachment.metadata"> | ||
| <field name="datas">bWlncmF0aW9uIHRlc3Q=</field> | ||
| <field name="datas_fname">attachment_metadata.doc</field> | ||
| <field name="name">attachment_metadata.doc</field> | ||
| </record> | ||
|
|
||
| </data> | ||
| </openerp> | ||
| </odoo> |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,12 +2,13 @@ | |
| # @ 2015 Florian DA COSTA @ Akretion | ||
| # License AGPL-3.0 or later (http://www.gnu.org/licenses/agpl.html). | ||
|
|
||
| from openerp import models, fields, api, _ | ||
| from openerp.exceptions import Warning as UserError | ||
| import openerp | ||
| import hashlib | ||
| from base64 import b64decode | ||
| import hashlib | ||
| import logging | ||
| import odoo | ||
| from odoo import _, api, fields, models | ||
| from odoo.exceptions import UserError | ||
|
|
||
|
|
||
| _logger = logging.getLogger(__name__) | ||
|
|
||
|
|
@@ -56,8 +57,7 @@ def _compute_hash(self): | |
| @api.model | ||
| def run_attachment_metadata_scheduler(self, domain=None): | ||
| if domain is None: | ||
| domain = [] | ||
| domain.append(('state', '=', 'pending')) | ||
| domain = [('state', '=', 'pending')] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did you really want to change the behaviour here ? A unit test calling this method with a domain in arguments would be fine.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is wanted indeed.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with @florian-dacosta on this one. An alternative to changing the behavior is to use a context override to stop the hard coded domain, but IMO explicitly adding is better than explicitly removing. |
||
| attachments = self.search(domain) | ||
| if attachments: | ||
| return attachments.run() | ||
|
|
@@ -70,7 +70,7 @@ def run(self): | |
| """ | ||
| for attachment in self: | ||
| with api.Environment.manage(): | ||
| with openerp.registry(self.env.cr.dbname).cursor() as new_cr: | ||
| with odoo.registry(self.env.cr.dbname).cursor() as new_cr: | ||
| new_env = api.Environment( | ||
| new_cr, self.env.uid, self.env.context) | ||
| attach = attachment.with_env(new_env) | ||
|
|
@@ -86,7 +86,11 @@ def run(self): | |
| }) | ||
| attach.env.cr.commit() | ||
| else: | ||
| attach.write({'state': 'done'}) | ||
| vals = { | ||
| 'state': 'done', | ||
| 'sync_date': fields.Datetime.now(), | ||
| } | ||
| attach.write(vals) | ||
| attach.env.cr.commit() | ||
| return True | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,3 @@ | ||
| id,name,model_id:id,group_id:id,perm_read,perm_write,perm_create,perm_unlink | ||
| access_attachment_metadata_user,ir.attachment.metadata.user,model_ir_attachment_metadata,base.group_user,1,0,0,0 | ||
| access_attachment_metadata_user,ir.attachment.metadata.user,model_ir_attachment_metadata,,1,0,0,0 | ||
| access_attachment_metadata_user,ir.attachment.metadata.user,model_ir_attachment_metadata,base.group_no_one,1,1,1,1 | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They have same key, I've fixed that in #856 because it needed to pass tests.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is the change you are recommending that @florian-dacosta perform here? Edit: (sorry for the accidental ping @sylvain-garancher)
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @lasley There is a mistake because 2 security rules have the same id. @cmsalmeida fixed it in the PR of external_file_location migration. I guess it is enought, as for now external_file_location is the only module depending on this one.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the elaboration @florian-dacosta. Works for me too! |
||
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.
👍