[MIG][12.0] account_invoice_margin: Migration to v12.0

[sergio-teruel]

cc @Tecnativa TT20158
@carlosdauden @chienandalu Canyou reviw?

[sergio-teruel]

Squashed administrative commits

[legalsylvain]

Could you make a dependency to the module product_replenishment_cost and replace the line
purchase_price = self.product_id.standard_price
by
purchase_price = self.product_id.replenishment_cost

https://github.com/OCA/margin-analysis/tree/12.0/product_replenishment_cost

It will allow easy overload.

Thanks !

Otherwise, LGTM. Thanks for porting this usefull module.

[pedrobaeza]

@legalsylvain sorry, but we don't want to introduce new dependencies for something not useful for us. There are 2 options:

• Do a little hack looking if that fields is present and then using it, fallbacking to the other.
• That you create a new glue module.

[legalsylvain]

I understand, even if it is the objective of this OCA module is to harmonize, the use of standard_price.

Well, at least, could you just add a function to allow easily overload, avoiding to rewrite functions ?

Thanks.

Add :

def _get_standard_price(self):
    self.ensure_one()
    return self.product_id.standard_price

And :

purchase_price = self._get_standard_price()

[sergio-teruel]

@legalsylvain Hi, I think that this changes would be done in a base module that returns the field or method to read price standard_price and other fields for analysis purposes and the other modules must denpend of this...
But I think that this PR is not the place to do that...
Maybe can you open an isuue to discuss it??

[legalsylvain]

@sergio-teruel said :

and the other modules must denpend of this...

@pedrobaeza said :

sorry, but we don't want to introduce new dependencies

I see it a little bit contradictory.

@sergio-teruel : what about to implement my last proposal. I see it as very light change, without impact, and let developpers to overload the function as they want. If you have no time, I can do this little change, no problem.

[pedrobaeza]

@legalsylvain it's not contradictory, because you are quoting just part of the reply. My colleague is proposing to open an issue for discussing the best way to implement it (as you know we always want the best in the ecosystem), but do it in the future (probably on v13), not now, due to time constraints.

The problem I see analyzing your proposal is that it's not possible to have a method (or not performant) on the invoice report: https://github.com/OCA/margin-analysis/pull/66/files#diff-d561cc05dc73b10d983544aff45e51e7R14

[legalsylvain]

Hi, I think that I didn't explain correctly my request.
here is the PR : Tecnativa#1

i think that it is harmless, and allow customization, regarding purchase_price computation.

Is it OK for you ?

[carlosdauden]

@sergio-teruel the module product_replenishment_cost seems pretty basic to me.
Can you reinspect the module?

[pedrobaeza]

@legalsylvain yes, I have mixed things about fields. I have merged your PR.

[pedrobaeza]

@sergio-teruel this is missing the recent patch #64. Or is not needed anymore?

[legalsylvain]

thanks !

LGTM. Code review. No test.

[carlosdauden]

@pedrobaeza #64 is now in commit history

[pedrobaeza]

#64 is removed again in the migration commit: ecb81bf

[pedrobaeza]

Fix missing

[sergio-teruel]

Now yes....

[pedrobaeza]

/ocabot merge

[OCA-git-bot]

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-66-by-pedrobaeza-bump-no, awaiting test results.

[OCA-git-bot]

Congratulations, your PR was merged at 6ea3bee. Thanks a lot for contributing to OCA. ❤️

[legalsylvain]

Seems that my commit (Tecnativa#1) has been lost in the last rebase. I don't understand why ?

[pedrobaeza]

@sergio-teruel you always have to do git pull before doing any other operation! I'm restoring the commit manually.

[legalsylvain]

Thanks for your quick answer !

I'm restoring the commit manually.

No need. I'm currently working on a PR. I'll do it.

[pedrobaeza]

OK, I'll wait then.