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

[14.0][MIG] stock_picking_quick #1088

Merged
merged 18 commits into from
Feb 10, 2023

Conversation

Kev-Roche
Copy link
Contributor

migration of module stock_picking_quick
cc @bealdav, @PierrickBrun

@rousseldenis
Copy link
Contributor

/ocabot migration stock_picking_quick

Copy link
Contributor

@rousseldenis rousseldenis 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

from odoo.tests.common import Form, TransactionCase


class TestQuickPicking(TransactionCase):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use SavepointCase

Copy link
Member

@bealdav bealdav Sep 26, 2022

Choose a reason for hiding this comment

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

In v15 TransactionCase Include SavepointCase feature, but yes, not in v14

self.env.context.get("parent_id")
)
if picking:
moves = self.env["stock.move"].search([("picking_id", "=", picking.id)])
Copy link
Contributor

Choose a reason for hiding this comment

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

You should do this to avoid a search():

Suggested change
moves = self.env["stock.move"].search([("picking_id", "=", picking.id)])
args.append(("move_ids.picking_id", "=", picking.id))

class ProductProduct(models.Model):
_inherit = "product.product"

move_ids = fields.One2many(
Copy link
Contributor

Choose a reason for hiding this comment

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

@api.depends("move_ids")
def _compute_process_qty(self):
res = super(ProductProduct, self)._compute_process_qty()
if self.env.context.get("parent_model", False) == "stock.picking":
Copy link
Contributor

Choose a reason for hiding this comment

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

You should add a depends_context for that entry

@Kev-Roche
Copy link
Contributor Author

Code review

@rousseldenis Thank you for your time, I apply your suggestions.

Copy link
Contributor

@rousseldenis rousseldenis left a comment

Choose a reason for hiding this comment

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

LGTM. A great addition as the module has now a quite long history, is to add test for product related code. @Kev-Roche

@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 Jan 29, 2023
@rousseldenis
Copy link
Contributor

/ocabot merge patch

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 14.0-ocabot-merge-pr-1088-by-rousseldenis-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 12365e7 into OCA:14.0 Feb 10, 2023
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 87f3147. 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
approved 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.

9 participants