Skip to content
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]dms_attachment_link: port to 16.0 version #311

Closed

Conversation

DemchukM
Copy link
Contributor

Migrate dms_attachment_link module from Odoo 15.0 to Odoo 16.0

@pedrobaeza
Copy link
Member

Thanks for the contribution.

Please preserve commit history following technical method explained in https://github.com/OCA/maintainer-tools/wiki/Migration-to-version-16.0.

If the jump is between several versions, you have to modify the source branch in the main command to accommodate it to this circumstance.

@R3D2 R3D2 mentioned this pull request Mar 19, 2024
6 tasks
@Aldeigja Aldeigja force-pushed the 16.0-t3432-dms_attachment_link-migrate_to_16 branch from e64392a to 26ad8b1 Compare June 18, 2024 11:11
victoralmau and others added 8 commits July 6, 2024 18:10
TT41512

[UPD] Update dms_attachment_link.pot

[UPD] README.rst
…selection.

TT41512

[UPD] Update dms_attachment_link.pot

dms_attachment_link 13.0.1.1.0

Update translation files

Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: dms-13.0/dms-13.0-dms_attachment_link
Translate-URL: https://translation.odoo-community.org/projects/dms-13-0/dms-13-0-dms_attachment_link/
Currently translated at 100.0% (6 of 6 strings)

Translation: dms-15.0/dms-15.0-dms_attachment_link
Translate-URL: https://translation.odoo-community.org/projects/dms-15-0/dms-15-0-dms_attachment_link/es/
Currently translated at 100.0% (6 of 6 strings)

Translation: dms-15.0/dms-15.0-dms_attachment_link
Translate-URL: https://translation.odoo-community.org/projects/dms-15-0/dms-15-0-dms_attachment_link/it/
@Aldeigja Aldeigja force-pushed the 16.0-t3432-dms_attachment_link-migrate_to_16 branch from 26ad8b1 to 75516c9 Compare July 6, 2024 16:41
Copy link

@angelinaanaki angelinaanaki left a comment

Choose a reason for hiding this comment

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

Code LGTM

@pedrobaeza
Copy link
Member

Formally seems OK, now reviewers should check the migration itself.

/ocabot migration dms_attachment_link

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Jul 8, 2024
Comment on lines 21 to 28
@api.model_create_multi
def create(self, vals_list):
"""Create attachments and link them to DMS files."""
attachments = super().create(vals_list)
for attachment, vals in zip(attachments, vals_list):
if "dms_file_id" in vals:
attachment.datas = attachment.dms_file_id.content
return attachments
Copy link
Member

Choose a reason for hiding this comment

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

Why is this change necessary? The _compute_datas() method already takes care of it (or should).

Copy link

@hoangtrann hoangtrann left a comment

Choose a reason for hiding this comment

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

In general code LGTM, agree with @victoralmau that extending _compute_datas() isn't neccessary


@api.depends("dms_file_id.content")
def _compute_datas(self):
"""Get the contents of the attachment directly from the DMS fiel."""

Choose a reason for hiding this comment

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

Suggested change
"""Get the contents of the attachment directly from the DMS fiel."""
"""Get the contents of the attachment directly from the DMS file."""

@mostafabarmshory
Copy link
Contributor

Dear all, Would you fix and merge with the main 16.0 branch.
tx

@mostafabarmshory
Copy link
Contributor

@DemchukM

Seams unit testes fail. This prevent the merge process?!

Copy link
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.

Code review OK.

The changes from commit [16.0][FIX]dms_attachment_link: refactoring code add them to [16.0][MIG]dms_attachment_link: port to 16.0 version

The errors in tests will already be solved when you run it again (it was related to OCA/oca-ci#78).

@mostafabarmshory
Copy link
Contributor

The branch 17.0 dose not contain this module. What the problem is?

@victoralmau
Copy link
Member

The branch 17.0 dose not contain this module. What the problem is?

The 17.0 branch does not have this module because it has not been migrated. If you want to migrate it you can do it (for example when this migration is finished and it is merged in v16).

@victoralmau
Copy link
Member

Superseed by #371

@pedrobaeza pedrobaeza closed this Oct 2, 2024
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.

10 participants