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.

Sign up for GitHub

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

Jump to bottom

[16.0][MIG] website_sale_coupon_page > website_sale_loyalty_page: Migration to version 16.0 #149

Merged
merged 10 commits into from
Sep 5, 2023
Merged

[16.0][MIG] website_sale_coupon_page > website_sale_loyalty_page: Migration to version 16.0 #149

merged 10 commits into from
Sep 5, 2023

Conversation

pilarvargas-tecnativa
Copy link
Contributor

@pilarvargas-tecnativa pilarvargas-tecnativa commented Aug 29, 2023 •
edited
Loading

Renamed to website_sale_loyalty_page + Migration to 16.0

cc @Tecnativa TT44378

@CarlosRoca13 @chienandalu please review

@pilarvargas-tecnativa pilarvargas-tecnativa marked this pull request as draft August 29, 2023 05:38
CarlosRoca13 and others added 9 commits August 29, 2023 10:18
@CarlosRoca13 @pilarvargas-tecnativa
[ADD] website_sale_coupon_page: New module to show the promotions on …
7bb3b94 
…website
@oca-transbot @pilarvargas-tecnativa
Update translation files
88cb8d5 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-promotion-13.0/sale-promotion-13.0-website_sale_coupon_page
Translate-URL: https://translation.odoo-community.org/projects/sale-promotion-13-0/sale-promotion-13-0-website_sale_coupon_page/
@pedrobaeza @pilarvargas-tecnativa
[IMP] website_sale_coupon_page: Switch public name to HTML
9c86d87 
This way, you can put richer text with multi-lines as description.

TT33564
@stefan-tecnativa @pilarvargas-tecnativa
[IMP] website_sale_coupon_page: black, isort, prettier
3d6eec2
@stefan-tecnativa @pilarvargas-tecnativa
[MIG] website_sale_coupon_page: Migration to 15.0
887d0dd
@pilarvargas-tecnativa
[UPD] Update website_sale_coupon_page.pot
f664a1a
@OCA-git-bot @pilarvargas-tecnativa
[UPD] README.rst
c4b115e
@weblate @pilarvargas-tecnativa
Update translation files
304a6de 
Updated by "Update PO files to match POT (msgmerge)" hook in Weblate.

Translation: sale-promotion-15.0/sale-promotion-15.0-website_sale_coupon_page
Translate-URL: https://translation.odoo-community.org/projects/sale-promotion-15-0/sale-promotion-15-0-website_sale_coupon_page/
@pilarvargas-tecnativa
[IMP] website_sale_coupon_page: pre-commit stuff
dd502a3
@pilarvargas-tecnativa pilarvargas-tecnativa marked this pull request as ready for review August 29, 2023 13:05
chienandalu
chienandalu approved these changes Aug 30, 2023
View reviewed changes
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.

👍

@pedrobaeza
Copy link
Member

Shouldn't this be renamed to website_loyalty_page or website_sale_loyalty_page?

@pilarvargas-tecnativa
Copy link
Contributor Author

Shouldn't this be renamed to website_loyalty_page or website_sale_loyalty_page?

Depends on website_sale_loyalty so I think website_sale_loyalty_page is correct.

@pilarvargas-tecnativa
Copy link
Contributor Author

Shouldn't this be renamed to website_loyalty_page or website_sale_loyalty_page?

Depends on website_sale_loyalty so I think website_sale_loyalty_page is correct.

But now that I check the dependencies I think it's more correct that it depends on loyalty

@pilarvargas-tecnativa
Copy link
Contributor Author

Shouldn't this be renamed to website_loyalty_page or website_sale_loyalty_page?

Depends on website_sale_loyalty so I think website_sale_loyalty_page is correct.

But now that I check the dependencies I think it's more correct that it depends on loyalty

I'm not sure, I'm making a mess with the changes that have been made in odoo, this is to show the promotions in the ecommerce, so it should depend on website_sale and loyalty and the bridge module is website_sale_loyalty, is it correct? In this case it would be fine renamed

@pedrobaeza
Copy link
Member

You have to check if the tools to discriminate the "coupons" to show are in loyalty or in website_sale_loyalty, and decide accordingly.

@chienandalu
Copy link
Member

Strictly it could be that this module only depended on loyalty and website, but it the practice that information is only usable on the eCommerce, so...

@pedrobaeza
Copy link
Member

A page can be rendered without website, so maybe the website dependency is not needed as well.

@chienandalu
Copy link
Member

Well, the website.published.mixin is being used so that dep can't be dropped

@pilarvargas-tecnativa
Copy link
Contributor Author

After analysing the dependencies, they should be loyalty and website. But now comes the dilemma of website_sale_loyalty, does it make sense for a site without e-commerce to show the promotions? I don't know if there will be any case in which, for example, without e-commerce they use the website as a presentation or catalogue and in that case it would be interesting to have this option available.

@pedrobaeza
Copy link
Member

If technically not needed, let's not put it for not forcing such installation. On mostly cases, they will install on their own, but not because we need it.

@pilarvargas-tecnativa
Copy link
Contributor Author

pilarvargas-tecnativa commented Aug 31, 2023 •
edited
Loading

I autocorrect myself:

The correct dependency is website_sale_loyalty, only the promotions published and associated to the website are shown, this is set by the "website_id" field of the mixin "website.multi.mixin" which is defined for the "loyalty.program" model in the website_sale_loyalty module. So the dependency is correct and it is well renamed.

ping @pedrobaeza

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@pedrobaeza pedrobaeza changed the title [16.0][MIG] website_sale_coupon_page: Migration to version 16.0 [16.0][MIG] website_sale_coupon_page > website_sale_loyalty_page: Migration to version 16.0 Sep 5, 2023
@pedrobaeza
Copy link
Member

/ocabot migration website_sale_coupon_page
/ocabot merge nobump

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Sep 5, 2023
@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-149-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot mentioned this pull request Jul 19, 2023
Open
14 tasks
@OCA-git-bot OCA-git-bot merged commit 6684e5e into OCA:16.0 Sep 5, 2023
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

pilarvargas-tecnativa pushed a commit to Tecnativa/sale-promotion that referenced this pull request Sep 21, 2023
@OCA-git-bot
Merge PR OCA#149 into 16.0
1e40029 
Signed-off-by pedrobaeza
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chienandalu chienandalu chienandalu approved these changes

@CarlosRoca13 CarlosRoca13 CarlosRoca13 approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@pilarvargas-tecnativa @pedrobaeza @chienandalu @OCA-git-bot @CarlosRoca13 @oca-transbot @stefan-tecnativa @weblate