rma icon indicating copy to clipboard operation
rma copied to clipboard

[18.0][MIG] rma_repair + rma_repair_lot: Migration to 18.0

Open victoralmau opened this issue 3 weeks ago • 10 comments

Migration to 18.0 + separate rma_repair into rma_repair_lot to avoid such a hard dependency (rma_lot).

Please @pilarvargas-tecnativa and @christian-ramos-tecnativa can you review it?

@Tecnativa TT57882

victoralmau avatar Nov 12 '25 16:11 victoralmau

I think the rma_lot dependency in https://github.com/OCA/rma/blob/18854fcc884e7d6b4926256f732b6469afdf97a5/rma_repair/models/rma.py#L58 is too strict (especially because of the dependencies that rma_lot has), so I can think of different possibilities:

  • Do not add rma_lot as a dependency of rma_repair and if the lot_id field exists, define its value.
  • Do not add rma_lot as a dependency of rma_repair and add an extra module (rma_repair_lot, for example) to add only that case.

What do you think, @pedrobaeza ?

victoralmau avatar Nov 12 '25 16:11 victoralmau

I would go for an extra module.

pedrobaeza avatar Nov 12 '25 19:11 pedrobaeza

/ocabot migration rma_repair

pedrobaeza avatar Nov 12 '25 19:11 pedrobaeza

I would go for an extra module.

On second thought, I don't think it's worth the effort to separate that part into a new module just for two lines of code. In the future, the separation will have to be justified, and anyone using this module in previous versions would not be able to use it and would have to add the new one.

In any case, it will be added to the ROADMAP.

victoralmau avatar Nov 13 '25 17:11 victoralmau

can you include these two commits please:

  • https://github.com/OCA/rma/pull/467/commits/c424d4f7fdf5a22af1acc2ef6ac577985a3d4755
  • https://github.com/OCA/rma/pull/467/commits/f30284a206ae6829aef23a464035e085a6cd3ec2

sbejaoui avatar Nov 14 '25 09:11 sbejaoui

https://github.com/OCA/rma/blob/18854fcc884e7d6b4926256f732b6469afdf97a5/rma_repair/models/rma.py#L58

IMO if this is the only reason to add rma_lot to the dependencies I would prefer checking if the field exists directly, as @victoralmau said, rma_lot adds many modules from stock-logistics-workflow unnecessary if you don't work with lots.

FWP of changes https://github.com/OCA/rma/pull/503#issuecomment-3531837234 created at 17.0: https://github.com/OCA/rma/pull/507

Commits added from 17.0 (https://github.com/OCA/rma/pull/507) to this PR

Separated the rma_lot part into a new rma_repair_lot module to avoid that hard dependency

victoralmau avatar Nov 18 '25 13:11 victoralmau

Shouldn't be rma_repair_lot autoinstalable?

rma_repair_lot

I'm not sure about auto-install. What do you think, @pedrobaeza ?

victoralmau avatar Nov 20 '25 07:11 victoralmau

Yeah, it sounds correct.

pedrobaeza avatar Nov 20 '25 07:11 pedrobaeza