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

[peluko00]

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

[victoralmau]

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]

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]

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`

dms/dms/models/dms_file.py

Line 247 in 5bcd966

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

  dms/dms/actions/file.xml

  Line 7 in 5bcd966

  <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

  dms/dms/models/dms_file.py

  Line 265 in 5bcd966

  "storage_id": dms_file.directory_id.storage_id.id,

  would not be necessary.
• We should change content_binary or content_file in

  dms/dms/models/dms_file.py

  Line 264 in 5bcd966

  "content": dms_file.with_context(**{}).content,

What do you think, @etobella ?

[peluko00]

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`

dms/dms/models/dms_file.py

Line 247 in 5bcd966

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

  dms/dms/actions/file.xml

  Line 7 in 5bcd966

  <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

  dms/dms/models/dms_file.py

  Line 265 in 5bcd966

  "storage_id": dms_file.directory_id.storage_id.id,

  
  would not be necessary.
• We should change content_binary or content_file in

  dms/dms/models/dms_file.py

  Line 264 in 5bcd966

  "content": dms_file.with_context(**{}).content,

What do you think, @etobella ?

The reason for this change is precisely to support that kind of use case: allowing files currently stored in the filestore to be migrated to another storage backend, such as S3, by changing the storage configuration and moving the existing file contents accordingly. So the goal is not only database ↔ file migration, but making storage backend transitions possible without requiring a separate manual action on dms.file.