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][ADD] rma_product_exchange #528

Open
wants to merge 1 commit into
base: 16.0
Choose a base branch
from

Conversation

chafique-delli
Copy link
Contributor

@chafique-delli chafique-delli commented Jun 28, 2024

This module allows you to ship a product different from the one ordered.
@florian-dacosta , @AaronHForgeFlow

@chafique-delli chafique-delli marked this pull request as draft June 28, 2024 08:29
@chafique-delli chafique-delli force-pushed the 16.0-ADD-rma_product_exchange branch 2 times, most recently from 5f5ef85 to d1505d3 Compare July 1, 2024 17:40
@chafique-delli chafique-delli marked this pull request as ready for review July 11, 2024 09:08
@AaronHForgeFlow
Copy link
Contributor

For this use case we typically do a sales order free of cost, where we change the product for another. Anyway I think it is interesting to have this use case in the operation and avoid the sales order.

Copy link
Contributor

@florian-dacosta florian-dacosta left a comment

Choose a reason for hiding this comment

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

I did not test the module yet. It is hard to see if there won't be any issue having a different product in shipping than in RMA.
For the computation of the qty_received field for instance...

I think for a start, there should be a constraint somewhere to ensure that the new product has the same uom_id as the base product of the rma line.
Else I fear we would have quite some bug, starting with this qty computed fields.


@api.model
def run(self, procurements, raise_user_error=True):
if self.env.context.get("rma_item"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this stuff really usefull ?
You already override rma_make_picking.wizard._get_product() it is enough and you don't need to do this stuff with context, or maybe I am missing something

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@florian-dacosta , it's removed.

class RmaOrderLine(models.Model):
_inherit = "rma.order.line"

new_product_id = fields.Many2one(
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a help here to explain what will happen if the field is set ?
Explain that will impact the shipping

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@florian-dacosta , it's done.

<field name="model">rma.operation</field>
<field name="inherit_id" ref="rma.rma_operation_form" />
<field name="arch" type="xml">
<xpath expr="//group[@name='policies']" position="inside">
Copy link
Contributor

Choose a reason for hiding this comment

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

It should be invisible if the delivery_policy is set to "no", because it is only used for delivery, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@florian-dacosta , it's fixed.

Copy link
Contributor

@florian-dacosta florian-dacosta left a comment

Choose a reason for hiding this comment

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

One minor comment.
The rest seems ok to me, I'll approve already!

return values

@api.model
def _create_procurement(self, item, picking_type):
Copy link
Contributor

Choose a reason for hiding this comment

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

This whole override is useless for now, because rma_item context key is not used anywhere anymore, right ?
In if this the case, please, remove it!

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.

3 participants