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

[15.0][IMP] stock_inventory_verification_request: compute involved lines and quant #2187

Open
wants to merge 3 commits into
base: 15.0
Choose a base branch
from

Conversation

JoanSForgeFlow
Copy link
Contributor

@JoanSForgeFlow JoanSForgeFlow commented Nov 5, 2024

If new moves are completed in that location for the specified product after confirming, they should automatically appear in the related lines.

Otherwise, it breaks traceability and hinders easy understanding of the process

Extra commit: do not show related inventory adjustments button if created_inventory_count is zero
2nd Extra commit: show lot_id for stock.group_production_lot group

@OCA-git-bot
Copy link
Contributor

Hi @LoisRForgeFlow,
some modules you are maintaining are being modified, check this out!

@JoanSForgeFlow JoanSForgeFlow changed the title [WIP][15.0][IMP] stock_inventory_verification_request: compute involved lines and quant [15.0][IMP] stock_inventory_verification_request: compute involved lines and quant Nov 6, 2024
@JoanSForgeFlow JoanSForgeFlow force-pushed the 15.0-imp-stock_inventory_verification_request-compute_involved_lines branch from a28c24c to 602dd99 Compare November 11, 2024 09:58
Copy link
Contributor

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

@api.depends("location_id", "product_id", "lot_id", "state")
def _compute_involved_move_lines(self):
for rec in self:
# Solo calcular si el estado es 'open'
Copy link
Contributor

Choose a reason for hiding this comment

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

Comments better in English

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sorry I missed this personal comment. Petition attended.

Thank you @AaronHForgeFlow

Copy link
Contributor

Choose a reason for hiding this comment

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

I would go one step further, why do you need a comment that states what the code does just right into the next line? I'm not a big friend of comments that explain what code does, an useful comment would be one that explains why something is done. So in this case for me the comment can be removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment attended

Thank you @LoisRForgeFlow

@JoanSForgeFlow JoanSForgeFlow force-pushed the 15.0-imp-stock_inventory_verification_request-compute_involved_lines branch 2 times, most recently from 0273b36 to 7ea8a15 Compare November 11, 2024 10:45
Copy link
Contributor

@JordiMForgeFlow JordiMForgeFlow 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 👍🏼

@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). 🤖

@JoanSForgeFlow JoanSForgeFlow force-pushed the 15.0-imp-stock_inventory_verification_request-compute_involved_lines branch 4 times, most recently from b455d96 to 605ac22 Compare November 11, 2024 11:13
@JoanSForgeFlow JoanSForgeFlow force-pushed the 15.0-imp-stock_inventory_verification_request-compute_involved_lines branch from 605ac22 to a270811 Compare November 11, 2024 11:16
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.

5 participants