[16.0][ADD] add rma_reason & sale_rma_reason #409

sbejaoui · 2024-07-15T19:14:23Z

Specifying the RMA reason when creating an RMA order is crucial for effective inventory management, quality control, and customer service. It helps categorize returned items accurately, identify recurring product issues, and improve overall product quality. This data enables customer service to address issues more effectively and enhances the customer experience.

This addon allows RMA managers to pre-configure possible reasons users can specify when creating the RMA order.

fixes #405

needs: #410

cc/ @jbaudoux , @lmignon , @rousseldenis

rma_reason/models/rma_reason.py

rousseldenis

LGTM. Apart the @lmignon remark, it should be great to add it to portal template too:

Thanks

Specifying the RMA reason when creating an RMA order is crucial for effective inventory management, quality control, and customer service. It helps categorize returned items accurately, identify recurring product issues, and improve overall product quality. This data enables customer service to address issues more effectively and enhances the customer experience. This addon allows RMA managers to pre-configure possible reasons users can specify when creating the RMA order.

rousseldenis

LGTM

pedrobaeza · 2024-07-19T06:39:07Z

rma_reason/__manifest__.py

+    "name": "Rma Reason",
+    "version": "16.0.1.0.0",
+    "license": "AGPL-3",
+    "author": "Raumschmiede GmbH,BCIM,ACSONE SA/NV,Odoo Community Association (OCA)",


I think Raumschmiede GmbH should go in the CREDITS file, not as author, being the paying customer, isn't it?

@jbaudoux , fine with you?

They are the original author of the module, not the paying customer of this OCA port

But you should not add them at the top of the file that were not there originally.

pedrobaeza · 2024-07-19T06:39:52Z

rma_sale/views/sale_portal_template.xml

-                            <th class="text-start">Requested operation</th>
+                            <th
+                                class="text-start"
+                                name="operation"


And you can take the occasion to name all the elements.

Suggested change

name="operation"

name="th_operation"

is it really needed? As in the xpath, we must already specify the node

<xpath expr="//th[@name='operation']" position="before">

Adding the same name several times is not a good practice IMO. It's true that selectors are done usually with the HTML tag, but to get an absolute element, better with unique names.

You are right @pedrobaeza in the case of the id element attribute since this one is expected to be unique across the full dom. But for the name, it seems to me to be redundant.

OK, it's not critical.

Just as reference, Odoo follows my same suggestion:

https://github.com/odoo/odoo/blob/9919961ffbc9d05d87f068ee8512728345d17bf1/addons/sale/report/ir_actions_report_templates.xml#L73

I would say it should be at developer's discretion as both approaches provide a unique way of xpath anchor.

pedrobaeza · 2024-07-19T06:39:59Z

rma_sale/views/sale_portal_template.xml

@@ -147,7 +150,7 @@
                                            t-att-value="data['picking'] and data['picking'].id"
                                        />
                                    </td>
-                                    <td class="text-start">
+                                    <td class="text-start" name="operation">


Suggested change

<td class="text-start" name="operation">

<td class="text-start" name="td_operation">

rma_reason/readme/CONTRIBUTORS.md

OCA-git-bot · 2024-07-20T20:00:02Z

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

jbaudoux · 2024-07-22T08:55:43Z

cc @raumschmiede-joshuaL

jbaudoux

LG, not tested yet

jbaudoux · 2024-08-02T13:49:45Z

@sbejaoui I would prefer to have the reason before the operation on the wizard as, in my opinion, it makes more sense to first choose a reason and then an operation. Some operations may not be valid for some reason

jbaudoux

Can you add the reason also in the stock picking return ? Like you did on SO wizard.
You can also create RMAs from there

sbejaoui · 2024-08-15T14:41:59Z

Can you add the reason also in the stock picking return ? Like you did on SO wizard. You can also create RMAs from there

done

rma_reason/wizards/stock_return_picking.xml

rma_sale_reason/wizards/sale_order_rma_wizard.xml

[IMP] rma_reason: add rma reason to stock return wizard

sbejaoui · 2024-10-18T07:18:38Z

@jbaudoux , can you plz update your review

jbaudoux

Tested, looks good

sbejaoui force-pushed the 16.0-rma_reason-sbj branch 2 times, most recently from 429d797 to 0ddd67f Compare July 15, 2024 19:27

lmignon suggested changes Jul 17, 2024

View reviewed changes

rma_reason/models/rma_reason.py Outdated Show resolved Hide resolved

rousseldenis added the needs review label Jul 17, 2024

rousseldenis added this to the 16.0 milestone Jul 17, 2024

rousseldenis linked an issue Jul 17, 2024 that may be closed by this pull request

RMA reason #405

Open

5 tasks

rousseldenis reviewed Jul 17, 2024

View reviewed changes

sbejaoui force-pushed the 16.0-rma_reason-sbj branch 2 times, most recently from b361a86 to 324b5a8 Compare July 17, 2024 18:49

sbejaoui requested review from rousseldenis and lmignon July 17, 2024 18:49

sbejaoui force-pushed the 16.0-rma_reason-sbj branch 3 times, most recently from 3223b9c to 18c0169 Compare July 18, 2024 07:03

sbejaoui force-pushed the 16.0-rma_reason-sbj branch 6 times, most recently from b32941b to 7886fb6 Compare July 18, 2024 13:05

sbejaoui changed the title ~~[16.0][ADD] add rma_reason~~ [16.0][ADD] add rma_reason & sale_rma_reason Jul 18, 2024

rousseldenis approved these changes Jul 18, 2024

View reviewed changes

lmignon approved these changes Jul 19, 2024

View reviewed changes

OCA-git-bot added the approved label Jul 19, 2024

pedrobaeza reviewed Jul 19, 2024

View reviewed changes

rousseldenis reviewed Jul 19, 2024

View reviewed changes

rma_reason/readme/CONTRIBUTORS.md Outdated Show resolved Hide resolved

OCA-git-bot added the ready to merge label Jul 20, 2024

sbejaoui force-pushed the 16.0-rma_reason-sbj branch 4 times, most recently from 94908ba to b0f2031 Compare July 22, 2024 08:53

jbaudoux approved these changes Jul 22, 2024

View reviewed changes

jbaudoux reviewed Aug 2, 2024

View reviewed changes

sbejaoui force-pushed the 16.0-rma_reason-sbj branch from 1696b0a to a8ef693 Compare August 15, 2024 14:41

jbaudoux requested changes Aug 16, 2024

View reviewed changes

rma_reason/wizards/stock_return_picking.xml Outdated Show resolved Hide resolved

rma_sale_reason/wizards/sale_order_rma_wizard.xml Outdated Show resolved Hide resolved

rma_sale_reason/wizards/sale_order_rma_wizard.xml Outdated Show resolved Hide resolved

OCA-git-bot removed approved ready to merge labels Aug 16, 2024

sbejaoui force-pushed the 16.0-rma_reason-sbj branch from a8ef693 to 694fb4d Compare August 16, 2024 10:20

[ADD] add rma_sale_reason

9a3a65b

[IMP] rma_reason: add rma reason to stock return wizard

sbejaoui force-pushed the 16.0-rma_reason-sbj branch from 694fb4d to 9a3a65b Compare August 18, 2024 18:08

[IMP] rma_reason: filter operation by reason

618c595

sbejaoui requested a review from jbaudoux August 19, 2024 07:19

jbaudoux approved these changes Nov 12, 2024

View reviewed changes

OCA-git-bot added the approved label Nov 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[16.0][ADD] add rma_reason & sale_rma_reason #409

[16.0][ADD] add rma_reason & sale_rma_reason #409

sbejaoui commented Jul 15, 2024 •

edited

Loading

rousseldenis left a comment

rousseldenis left a comment

pedrobaeza Jul 19, 2024

sbejaoui Jul 19, 2024

jbaudoux Jul 22, 2024

jbaudoux Jul 22, 2024

pedrobaeza Jul 19, 2024

sbejaoui Jul 19, 2024

pedrobaeza Jul 19, 2024

lmignon Jul 19, 2024

pedrobaeza Jul 19, 2024

pedrobaeza Jul 20, 2024

rousseldenis Jul 20, 2024

pedrobaeza Jul 19, 2024

OCA-git-bot commented Jul 20, 2024

jbaudoux commented Jul 22, 2024

jbaudoux left a comment

jbaudoux commented Aug 2, 2024

jbaudoux left a comment

sbejaoui commented Aug 15, 2024

sbejaoui commented Oct 18, 2024

jbaudoux left a comment

	<td class="text-start" name="operation">
	<td class="text-start" name="td_operation">

[16.0][ADD] add rma_reason & sale_rma_reason #409

Are you sure you want to change the base?

[16.0][ADD] add rma_reason & sale_rma_reason #409

Conversation

sbejaoui commented Jul 15, 2024 • edited Loading

rousseldenis left a comment

Choose a reason for hiding this comment

rousseldenis left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

OCA-git-bot commented Jul 20, 2024

jbaudoux commented Jul 22, 2024

jbaudoux left a comment

Choose a reason for hiding this comment

jbaudoux commented Aug 2, 2024

jbaudoux left a comment

Choose a reason for hiding this comment

sbejaoui commented Aug 15, 2024

sbejaoui commented Oct 18, 2024

jbaudoux left a comment

Choose a reason for hiding this comment

sbejaoui commented Jul 15, 2024 •

edited

Loading