Skip to content

[17.0][FIX] dms: Change save_type when migrate to another storage#467

Open
peluko00 wants to merge 1 commit intoOCA:17.0from
APSL:17.0-fix-dms
Open

[17.0][FIX] dms: Change save_type when migrate to another storage#467
peluko00 wants to merge 1 commit intoOCA:17.0from
APSL:17.0-fix-dms

Conversation

@peluko00
Copy link
Copy Markdown

@peluko00 peluko00 commented Mar 19, 2026

When migrating files from one storage to another, their save type needs to change to match the storage's save_type. After that, these files will no longer require migration.

cc https://github.com/APSL 38655
@miquelalzanillas @lbarry-apsl @javierobcn @mpascuall @BernatObrador @ppyczko please review

@peluko00 peluko00 changed the title f [17.0][FIX] dms: Change save_type when migrate to another storage Mar 19, 2026
@peluko00 peluko00 force-pushed the 17.0-fix-dms branch 2 times, most recently from 8fa2542 to 94cfc2a Compare March 24, 2026 07:16
Copy link
Copy Markdown
Member

@victoralmau victoralmau left a comment

Choose a reason for hiding this comment

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

IMP, this change isn't appropriate. Currently, the save_type field is set to compute (not store); perhaps it should be store, but in any case, the value shouldn't be defined this way. The proper approach is to set the appropriate value in the compute (_compute_save_type) based on different scenarios and/or add a depends clause if necessary.

To clarify, I have never used this migration action.

@peluko00
Copy link
Copy Markdown
Author

IMP, this change isn't appropriate. Currently, the save_type field is set to compute (not store); perhaps it should be store, but in any case, the value shouldn't be defined this way. The proper approach is to set the appropriate value in the compute (_compute_save_type) based on different scenarios and/or add a depends clause if necessary.

To clarify, I have never used this migration action.

When executes action_migrate they need change the save_type because if you change from filestore to Amazon s3 if you don't change this attribute the migration will be failed.

@victoralmau
Copy link
Copy Markdown
Member

Although I've never used the action_migrate() method, I do understand its purpose: to change the storage type from database to file or vice versa. That said, I believe the current approach is incorrect (and the changes in this PR are as well).

The`

def action_migrate(self, should_logging=True):
method should check whether all the storage_id of the selected records have the same save_type; if not, it shouldn’t be allowed. In any case, I don’t think this approach is the right one either.

In my opinion, the correct change should be:

  • Allow changing the save_type in dms.storage (except from or to attachment)
  • Define an inverse in the storage’s save_type field to perform the migration.
  • The
    <record id="action_dms_attachment_migrate" model="ir.actions.server">
    action would not be necessary.
  • The migration and require_migration fields in dms.file would not be necessary.
  • The change
    "storage_id": dms_file.directory_id.storage_id.id,
    would not be necessary.
  • We should change content_binary or content_file in
    "content": dms_file.with_context(**{}).content,

What do you think, @etobella ?

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.

2 participants