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

[17.0][MIG] rma_scrap #514

Open
wants to merge 6 commits into
base: 17.0
Choose a base branch
from
Open

Conversation

AaronHForgeFlow
Copy link
Contributor

Supersedes #504

Respected original code & history

Reasons for supersede:

Merge 17.0 was broken after merging and reverting by mistake some pull requests: #483 & #500

History was incorrect showing merge commits within it.
Merge conflicts when trying to manually merge other PRs on top on that one.
Unable to see the changes in the migration commit, making hard for the reviewer to review the code

Sorry for the inconvenience.

https://github.com/ForgeFlow

@AaronHForgeFlow
Copy link
Contributor Author

Tested scrap operations with the scrap completion. LGTM

(I am not reviewing myself, the migration was done in the superseded PR)

@AaronHForgeFlow
Copy link
Contributor Author

There is only one issue with the rma_scrap view, with some fields not being displayed (I think they can be removed from the view)

image

@RLeeOSI
Copy link
Contributor

RLeeOSI commented Aug 2, 2024

@AaronHForgeFlow Found a minor issue with creating a scrap order from the RMA order.

If the product to be scrapped is serial tracked, the scrap form view makes the quantity field readonly. There's an onchange on the product_id field that is supposed to set the quantity to 1 if the product is serial tracked, but it is not triggered. The result is that the quantity is set to 0 and is readonly. Calling the onchange method after creating the scrap order should fix this.

@AaronHForgeFlow
Copy link
Contributor Author

@RLeeOSI thanks for the notice! I will fix that. Anyway, if you create a PR to my branch I would appreciate it.

@AaronHForgeFlow AaronHForgeFlow force-pushed the 17.0-mig-rma_scrap-good branch from d545bfe to 085d05f Compare August 8, 2024 08:50
@AaronHForgeFlow
Copy link
Contributor Author

@RLeeOSI I see that in the Scrap order the quantiy is readonly, but it seems that is standard Odoo. When the product is serial tracked then the quantity is readonly.

In the wizard, I can see I can put a different quantity even if the product is serial:

This quantity is propagated to the scrap order and can't be changed:
2024-08-22_14-36

So the solution I think is to force the quantity in the wizard to be 1. Same as Odoo does with manual scrap orders.

@AaronHForgeFlow AaronHForgeFlow force-pushed the 17.0-mig-rma_scrap-good branch from 085d05f to 94892e7 Compare August 22, 2024 15:14
@AaronHForgeFlow
Copy link
Contributor Author

Rebasing just to confirm tests fail and the module needs an amendment.

[FIX] rma_scrap: ensure scrapping serials will scrap just 1 unit & ensure scrapped quantity is always positve
[FIX] rma_scrap: scrap quantity should consider the quantity in the stock moves not the product_uom_qty
[IMP] rma_scrap: test serial case
@AaronHForgeFlow AaronHForgeFlow force-pushed the 17.0-mig-rma_scrap-good branch from 94892e7 to 97b62c0 Compare August 22, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants