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_order_type: Migration to version 16.0 #826

Merged
merged 19 commits into from
Aug 22, 2023

Conversation

pilarvargas-tecnativa
Copy link
Contributor

cc @Tecnativa TT44380

@stefan-tecnativa @chienandalu please review :)

SimoRubi and others added 18 commits August 10, 2023 12:09
Without this patch, it's easy that the tour fails randomly due to a timeout when generating the sales order report. These assets can take a long time, increasing when more addons are installed.

In an integrated database, this patch makes the test go from this log:

    2020-11-06 10:34:53,941 34 INFO devel werkzeug: 127.0.0.1 - - [06/Nov/2020 10:34:53] "POST /payment/transfer/feedback HTTP/1.1" 302 - 730 4.368 11.800

To this one:

    2020-11-06 10:32:55,517 34 INFO devel werkzeug: 127.0.0.1 - - [06/Nov/2020 10:32:55] "POST /payment/transfer/feedback HTTP/1.1" 302 - 479 2.906 5.771

That's about 50% the time.

In the same patch, I remove the `self.env` patch that was only needed for Odoo v11 and lower.
With the old tour, timeout problems started to occur, even with 7830084

This partially reverts that, and instead uses a simpler tour to test the creation of the SO.

TT26399
If we don't trigger the onchanges, we won't get some of the default
values that the order type has defined

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

Translation: e-commerce-15.0/e-commerce-15.0-website_sale_order_type
Translate-URL: https://translation.odoo-community.org/projects/e-commerce-15-0/e-commerce-15-0-website_sale_order_type/
Currently translated at 100.0% (1 of 1 strings)

Translation: e-commerce-15.0/e-commerce-15.0-website_sale_order_type
Translate-URL: https://translation.odoo-community.org/projects/e-commerce-15-0/e-commerce-15-0-website_sale_order_type/es/
@pedrobaeza
Copy link
Member

This one shouldn't be needed if the field is computed writable (which I think it is), but if not, the patch should be to make it on the base module, like we have done with this one: OCA/sale-workflow@788e2bc

@chienandalu
Copy link
Member

This one shouldn't be needed if the field is computed writable (which I think it is), but if not, the patch should be to make it on the base module

Yes, this can be a good chance to fix that

@pilarvargas-tecnativa
Copy link
Contributor Author

Fix made in OCA/sale-workflow#2639

@pilarvargas-tecnativa
Copy link
Contributor Author

This migration PR has been reopened as the fix made in sale_order_type did not cover the e-commerce flow. Therefore this module is still needed.

@pedrobaeza can you check it too?

I have decided to extend the methods that set the values that are overwritten in the e-commerce flow instead of the method that prepares the dictionary and calls these methods so that in case they are called from anywhere else the intended effect of this module is still in effect.

@pedrobaeza
Copy link
Member

/ocabot migration website_sale_order_type

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Aug 21, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Aug 17, 2023
33 tasks
Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

OK for overriding these methods instead.

website_sale_order_type/models/website.py Outdated Show resolved Hide resolved
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.

👍

Just a minor comment:

website_sale_order_type/models/website.py Outdated Show resolved Hide resolved
@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). 🤖

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

_description should be removed.

Copy link
Member

@pedrobaeza pedrobaeza left a comment

Choose a reason for hiding this comment

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

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

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

@OCA-git-bot OCA-git-bot merged commit 4a49cd6 into OCA:16.0 Aug 22, 2023
4 checks passed
@OCA-git-bot
Copy link
Contributor

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

@pedrobaeza pedrobaeza deleted the 16.0-mig-website_sale_order_type branch August 22, 2023 17:03
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.