Skip to content

PR #129 breaks attachments_to_filesystem #135

Closed

Description

@luc-demeyer

in this PR the code has been incorrectly moved to the new api:

in models/ir_attachment.py, line 83:

    ir_attachment = self.env['ir.attachment']
    attachment_ids = ir_attachment.search(
        [('db_datas', '!=', False)], limit=limit)

Since it's now the new api, the attachments are returned and no longer the attachment_ids.
The rest of the code still believes that a list of ids is returned and hence it breaks,
cf. line 89
attachment_data = ir_attachment.read(
[attachment_id], ['datas', 'res_model']

an quick fix could be to change [attachment_id] into [attachment_id.id] but it's probably cleaner to go the new api all the way (hence change 'attachment_ids' in line 84 into 'attachments' and skip the 'read' in line 89 since we have already the records in browse.

Activity

  1. hbrunn commented on May 2, 2017

    @hbrunn
    Member

    @pedrobaeza I won't have time to work on this any time soon, is it a good idea to assign the issue to me then? Probably this will keep other people from picking this up? (generally, I prefer to assign issues to myself as a pledge to work on it some time soon...)

  2. pedrobaeza commented on May 2, 2017

    @pedrobaeza
    Member

    OK, @hbrunn, I have created a quick patch in #136

  3. luc-demeyer commented on May 8, 2017

    @luc-demeyer
    Author

    The 'quick' fix in PR #136 was a bit too quick and since this module doesn't have unit testing, bugs are not detected: the read still fails because new api read assumes the id in the recordset, not via the [ids] parameter.

    I have now created another quick fix (cf. PR #138) on top of PR #136 and I have TESTED this one. Please merge asap because a breaking fix on top of a breaking fix is not good for the OCA reputation.

  4. hbrunn commented on May 9, 2017

    @hbrunn
    Member

    well, that's why I only work on stuff when I've got time to do it properly. Now I burned most of this week's OCA time for #139 to get this branch back on track. Consider turning on mandatory CI on this repo, I'll do this for 'my' repos as soon as I get permission to do so (cf https://odoo-community.org/groups/contributors-15/contributors-57793).
    As the issue itself is fixed, I'll close it. Next time, add the appropriate string in your commit message to have this happen automatically

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions