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] website_sale_product_attachment: Migration to version 16.0 #829

Merged

Conversation

ernesto-garcia-tecnativa

@Tecnativa TT44231

@ernesto-garcia-tecnativa ernesto-garcia-tecnativa force-pushed the 16.0-mig-website_sale_product_attachment branch 2 times, most recently from 3868dd0 to 6c02d87 Compare August 21, 2023 07:30
@ernesto-garcia-tecnativa ernesto-garcia-tecnativa force-pushed the 16.0-mig-website_sale_product_attachment branch 2 times, most recently from c9f617c to cd58ba2 Compare August 21, 2023 07:46
Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

In my functional test the attachments didn't show up for the public user (ie: not logged in)

@ernesto-garcia-tecnativa
Copy link
Author

In my functional test the attachments didn't show up for the public user (ie: not logged in)

I tried and works fine, you sure that field "Is public document" has marked into attachment file?

Example...
Captura de pantalla 2023-08-21 a la(s) 1 55 37 a m

@chienandalu
Copy link
Member

In my functional test the attachments didn't show up for the public user (ie: not logged in)

I tried and works fine, you sure that field "Is public document" has marked into attachment file?

After your last changes it's the opposite. It shows up for public users but it doesn't for logged in users.

@ernesto-garcia-tecnativa
Copy link
Author

@chienandalu because the website_attachment_ids field of product.template have a dynamic domain with the filter ("public", "=", True) and others that filter internal files of Odoo

Please see this link:
https://github.com/OCA/e-commerce/pull/829/files#diff-e62ec0c8b3609baae7fb387ed2a30dc6a94ce7e7f5fe3ddb509127143856332cR27

@ernesto-garcia-tecnativa ernesto-garcia-tecnativa force-pushed the 16.0-mig-website_sale_product_attachment branch from cd58ba2 to e0ebfdb Compare August 27, 2023 01:47
@chienandalu
Copy link
Member

because the website_attachment_ids field of product.template have a dynamic domain with the filter ("public", "=", True) and others that filter internal files of Odoo

But that's not my complain. I know that the attachment has to be public. The issue was that only the public user could see the dropdown in the product page and that's not the behavior in previous versions

@pedrobaeza
Copy link
Member

/ocabot migration website_sale_product_attachment

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Sep 1, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Sep 1, 2023
33 tasks
Jairo Llopis and others added 16 commits September 5, 2023 08:31
… website

This module lets you publish downloadable attachments in a product page.

This is useful if you want to publish firmwares, manuals, specs, warranties,
or whatever document related to the product.

@Tecnativa TT20984 TT23657
… names

Without this fix, the order in which attachments were displayed couldn't be predictable. Now, it's as expected by user: by name.

Also the thumbnails got stretched when the attachment had a long name.

@Tecnativa TT24437
Currently translated at 100.0% (9 of 9 strings)

Translation: e-commerce-12.0/e-commerce-12.0-website_sale_product_attachment
Translate-URL: https://translation.odoo-community.org/projects/e-commerce-12-0/e-commerce-12-0-website_sale_product_attachment/es/
Currently translated at 33.3% (3 of 9 strings)

Translation: e-commerce-12.0/e-commerce-12.0-website_sale_product_attachment
Translate-URL: https://translation.odoo-community.org/projects/e-commerce-12-0/e-commerce-12-0-website_sale_product_attachment/fr/
Currently translated at 100.0% (9 of 9 strings)

Translation: e-commerce-12.0/e-commerce-12.0-website_sale_product_attachment
Translate-URL: https://translation.odoo-community.org/projects/e-commerce-12-0/e-commerce-12-0-website_sale_product_attachment/fr/
Currently translated at 33.3% (3 of 9 strings)

Translation: e-commerce-12.0/e-commerce-12.0-website_sale_product_attachment
Translate-URL: https://translation.odoo-community.org/projects/e-commerce-12-0/e-commerce-12-0-website_sale_product_attachment/nl/
Currently translated at 100.0% (9 of 9 strings)

Translation: e-commerce-12.0/e-commerce-12.0-website_sale_product_attachment
Translate-URL: https://translation.odoo-community.org/projects/e-commerce-12-0/e-commerce-12-0-website_sale_product_attachment/ca/
…sudo

In v13, there's no general read ACL for public files, so we need to get
filenames and file type using sudo. The download is performed normally
due to the public=True field in the attachments.
pedrobaeza and others added 4 commits September 5, 2023 08:31
On previous version, attachments had 2 fields for adding both attachment
name and file name.

Now on v13, there's only one, that is fille with the file name. On
initial migration, it was considered that this field is enough, but
putting file names on the website product page can be ugly, limited and
confusing, so we are adding here a new field to store the name we want
to give it for the website e-commerce product page.

It also includes migration script for recovering the old information if
coming from v12.
Currently translated at 100.0% (12 of 12 strings)

Translation: e-commerce-13.0/e-commerce-13.0-website_sale_product_attachment
Translate-URL: https://translation.odoo-community.org/projects/e-commerce-13-0/e-commerce-13-0-website_sale_product_attachment/ca/
@ernesto-garcia-tecnativa ernesto-garcia-tecnativa force-pushed the 16.0-mig-website_sale_product_attachment branch from e0ebfdb to 6ac6c6f Compare September 5, 2023 14:32
Copy link
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Tested in local 👍

@chienandalu
Copy link
Member

please review @CarlosRoca13

Copy link
Contributor

@CarlosRoca13 CarlosRoca13 left a comment

Choose a reason for hiding this comment

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

Code and functional review 👍

@pedrobaeza
Copy link
Member

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-829-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 627b7b1 into OCA:16.0 Sep 6, 2023
5 of 6 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 1a8d70e. Thanks a lot for contributing to OCA. ❤️

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

Successfully merging this pull request may close these issues.