[16.0] refactor quotations rest api

[nicolas-delbovier-acsone]

This PR introduces several refactorings to improve module isolation and extend the Shopinvader API:

1. sale_typology module creation: Isolates the sale order typology logic.
2. sale_quotation extension: Updates sale_quotation to depend on the new sale_typology module.
3. sale_cart_quotation module creation: Decouples sale_quotation from sale_cart.
4. shopinvader_api_quotation enhancement: Enables line creation, update, and deletion within the quotation API.
5. shopinvader_api_cart_quotation module extraction: Moves the cart-to-quotation transformation logic from shopinvader_api_quotation to its own dedicated module.

[nicolas-delbovier-acsone]

The last force push aimed at rebasing this PR to #1594.

[lmignon]

@paradoxxxzero fixed

[sebastienbeau]

Thanks for the refactor ! Quotation flow will be more powerfull !

[sebastienbeau]

Interesting code. Creating a specific error so we can catch it and return a clean http error code is smart.

In long term, maybe we should be able to extend the "convert_exception_to_status_body"
https://github.com/OCA/rest-framework/blob/34c06aab48512b6d7bda8ec797c45ae219a2fc59/fastapi/error_handlers.py#L20 so we can just extend this and it will raise the right http error without needed a wrapper.

The issue with the wrapper is when an other endpoint do an action and you didn't have set the decoration (because you didn't expect this case) you will not have the right http error.

Note a blocking point, it's just a comment to share the idea. If you think it's an interesting one we can to a PR in fastapi to implement it.

[sebastienbeau]

I always try to avoid function that call themself in both way (it's harder to read, and it can generate unexpected random issue)

For example if you have a custom module that do the following

def action_draft(self):
   for sale in self:
       sale._action_draft_count += 1
   super().action_draft()

Then depending on the order of loading the module, the increment code can be called before or after the inherit in sale_quotation.

In case that it's called before, if on a quotation you click on action_draft, it will increment a first time, then it will call "action_customer_reset_quotation_to_draft" that will call "action_draft" and increment a second time.

The crazy thing is that if the module are loaded in a different order (and the increment is call with the super), you will not have the issue, so have random bug (True bad experience).

Maybe we can simplify this logic and avoid the extra code of "bypass_customer_quotation"

Something like:

Suggested change

	def action_draft(self):
	other_quotations = self
	if not self.env.context.get("bypass_customer_quotation", False):
	customer_quotations = self.filtered("use_customer_quotation_workflow")
	other_quotations = self - customer_quotations
	if customer_quotations:
	customer_quotations.action_customer_reset_quotation_to_draft()
	return super(SaleOrder, other_quotations).action_draft()
	def action_customer_reset_quotation_to_draft(self):
	self._check_customer_action_allowed("reset_quotation_to_draft")
	return self.action_draft()
	def action_draft(self):
	customer_quotations = self.filtered("use_customer_quotation_workflow")
	customer_quotations.quotation_state = "draft"
	customer_quotations.filtered(lambda so: so.typology != "quote").typology = "quote"
	return super().action_draft()

What do you think ?

[lmignon]

With your approach, the logic of resetting a customer quotation to draft is no more isolated into a dedicated method.... The current implementation also ensure that everything stay consistent if you click on the 'main' button for action_draft or on the customer one....

[sebastienbeau]

Maybe we should have something like

    def _set_customer_reset_quotation_to_draft(self):
        self.quotation_state = "draft"
        self.filtered(lambda so: so.typology != "quote").typology = "quote"

    def action_customer_reset_quotation_to_draft(self):
        self._check_customer_action_allowed("reset_quotation_to_draft")
        self._set_customer_reset_quotation_to_draft()
        return self.with_context(skip_set_customer_reset_quotation_to_draft=True).action_draft()

    def action_draft(self):
        if not self._context.get("skip_set_customer_reset_quotation_to_draft"):
            self.filtered("use_customer_quotation_workflow")._set_customer_reset_quotation_to_draft()
        return super().action_draft()

Or

    def _set_customer_reset_quotation_to_draft(self):
        self.quotation_state = "draft"
        self.filtered(lambda so: so.typology != "quote").typology = "quote"

    def action_customer_reset_quotation_to_draft(self):
        self._check_customer_action_allowed("reset_quotation_to_draft")
        return self.action_draft()

    def action_draft(self):
        self.filtered("use_customer_quotation_workflow")._set_customer_reset_quotation_to_draft()
        return super().action_draft()

What do you think ?

[sebastienbeau]

setting draft here is useless as action_draft will do it

Suggested change

self.state = "draft"

[sebastienbeau]

This change with the previous behavior.
Indeed before this change an accepted quotation was staying in draft (as maybe we wait for extra action like the payment)

The two customer we have, for now make this difference, they want to know that the customer have accepted the quotation, but they wait for the payment to confirm the sale order.

Maybe we can add a general configuration option ? So we can activate or not this behavior.

[sebastienbeau]

Following comment is just an opinion so nothing blocking

I do not like a lot to add extra status bar, on previous version I hide the original one and try to be more integrated with existing odoo action.

I fear that this make the UI harder to undertand (many button and status bar)

I will show it to @thibaultrey and @dora-jurcevic to get more feedback

[paradoxxxzero]

Suggested change

	and any(rec.quotation_state != "waiting_acceptation" for rec in self)
	and any(rec.quotation_state != "waiting_acceptation" for rec in customer_quotations)