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

[MIG] project_purchase_link: Migration to 16. #1061

Merged
merged 13 commits into from
Jun 13, 2023

Conversation

AnizR
Copy link
Contributor

@AnizR AnizR commented Feb 8, 2023

Migration of module project_purchase_link

Some modifications had to be done due to the fact that we now have analytic_distribution instead of just an anlaytic_account.

@AnizR
Copy link
Contributor Author

AnizR commented Feb 8, 2023

/ocabot migration project_purchase_link

@OCA-git-bot
Copy link
Contributor

Sorry @AnizR you are not allowed to mark the addon tobe migrated.

To do so you must either have push permissions on the repository, or be a declared maintainer of all modified addons.

If you wish to adopt an addon and become it's maintainer, open a pull request to add your GitHub login to the maintainers key of its manifest.

@AnizR AnizR mentioned this pull request Feb 8, 2023
38 tasks
@pedrobaeza
Copy link
Member

/ocabot migration project_purchase_link

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Feb 8, 2023
@pedrobaeza
Copy link
Member

Please squash together these commits:

Selección_071

@AnizR AnizR force-pushed the 16.0-mig-project_purchase_link branch from e1bd026 to 102a340 Compare February 8, 2023 17:43
@AnizR
Copy link
Contributor Author

AnizR commented Feb 8, 2023

Please squash together these commits:

Selección_071

Done

@@ -23,84 +21,205 @@ class ProjectProject(models.Model):
)

def _compute_purchase_info(self):
# read group
query = self.env["purchase.order.line"]._search(
Copy link
Member

Choose a reason for hiding this comment

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

Why removing the read_group? I don't think the complexity of the code deserves the few juice on the performance (if any).

Copy link
Contributor Author

@AnizR AnizR Feb 8, 2023

Choose a reason for hiding this comment

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

In fact, since now on a PO line we have an analytic_distribution (and an analytic distribution is a json such as:
{analytic_account_id1:rate1, analytic_account_id2:rate2...}).
I think that it is not possible to use a read group as it was done in previous versions because I am trying to find PO lines that have the analytic account of the project within their analytic distribution:
query.add_where( "purchase_order_line.analytic_distribution ?| array[%s]", [str(project.analytic_account_id.id) for project in self], )

But correct me if I am wrong

Copy link
Member

Choose a reason for hiding this comment

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

OK, I see... Everything gets complicated with the new analytic system. I let users of this module to judge, as I don't use it and only doing some administrative collaboration.

@AnizR AnizR force-pushed the 16.0-mig-project_purchase_link branch from 102a340 to 40b7c1a Compare February 10, 2023 13:13
@AnizR AnizR requested a review from acsonefho February 10, 2023 13:13
@AnizR AnizR force-pushed the 16.0-mig-project_purchase_link branch 2 times, most recently from b14cf03 to 14c0eec Compare February 10, 2023 13:33
@AnizR AnizR marked this pull request as draft February 10, 2023 13:52
@AnizR AnizR force-pushed the 16.0-mig-project_purchase_link branch from 14c0eec to be6fe51 Compare February 10, 2023 15:00
@AnizR AnizR marked this pull request as ready for review February 10, 2023 15:03
@github-actions
Copy link

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Jun 11, 2023
@acsonefho acsonefho force-pushed the 16.0-mig-project_purchase_link branch from be6fe51 to aa3831d Compare June 13, 2023 13:12
@acsonefho
Copy link

acsonefho commented Jun 13, 2023

@pedrobaeza can you retry your merge please? 🙏
Sorry I miss read your previous command.

Do you have super-power to merge this please?

@pedrobaeza
Copy link
Member

There's a missing review, but well, I think it's enough with the arguments given and your review:

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-1061-by-pedrobaeza-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 328ff98 into OCA:16.0 Jun 13, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 09c3ded. 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
Labels
merged 🎉 migration needs review stale PR/Issue without recent activity, it'll be soon closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants