shopinvader_api_delivery_carrier: hide delivery line in cart

[sebastienbeau]

Hide delivery line in cart
Depends on

• [16.0] shopinvader_schema_sale: allow to hide some line in the schema (like delivery...) odoo-shopinvader#1564

[sbidoul]

Can you elaborate the use case?

[lmignon]

@sebastienbeau I don't really see why we should hide lines from a SO. You always have the 'name' field, which gives information describing what the line represents (you can have lines that are notes, sections, delivery charges, ....). Even if the line refers to a technical product that may not be available in ES, this should not matter. The product life cycle will mean that a product purchased in the past will no longer be present in ES, and this should not block the display of an order in the UI. Taking this a step further, the fact that the product identifier may or may not be available to the consumer of the API should not be a factor in this discussion, since we are dealing here with the API and not with its use by a website. But maybe I miss something...

[sebastienbeau]

Hi @sbidoul @sbidoul sorry for the delay

The idea it to give a better experience for the front-end developer (like we had in version 14 see: https://github.com/shopinvader/odoo-shopinvader/blob/0b031e87c4a64e7f013598e2f8c76ac206c126cd/shopinvader_delivery_carrier/services/abstract_sale.py#L48).

In the schema of the cart / sale the delivery price is accessible with special key see here: https://github.com/shopinvader/odoo-shopinvader-carrier/blob/d681c1a788d886a0da5e37c8a73cea2cf05c6b98/shopinvader_api_delivery_carrier/schemas/amount.py

Before this PR we have a duplicated information

• the price of the delivery is accessible in the cart / sale with the key tax_without_shipping, tax_without_shipping, tax_without_shipping, total_without_shipping_without_discount
• and the also as a line in the cart / sale

In a front end point of view. It make no sense to have the delivery as a product and they always have to remove it (If the delivery cost exist in a line in odoo is only because of the implementation, so It should not design our API).

Note: I choose to add a computed field so It's easier to reuse this computed field when trying to read the sale.line directly using this API : shopinvader/odoo-shopinvader#1508 without creating too many glue module.

Thanks for your feedback

[sbidoul]

I sort of see your point, but visible_in_shopinvader_api does not sound right to me.

give a better experience for the front-end developer

How does the front end dev filter today? In a project I see this:

Indeed it does not seem very easy to detect shipping and reward lines.

Would a is_shipping or is_reward flag on sale lines help?

cc/ @marielejeune @qgroulard who may have an opinion on this too.

[marielejeune]

Indeed from the schema it is impossible to detect reward and delivery lines for now.
In our projects the prices computations (such as for total_wo_discount_wo_shipping_wo_reward which is a custom field) are done in the schemas, not letting the front-end dev dealing with that.
We thus used the is_delivery and is_reward_line flags that are present on a SOL. But we could expose these flags directly in the SOL schema.

[sebastienbeau]

Hi @sbidoul @marielejeune

After thinking twice I agree that the naming is not really good and the architecture also.
Adding a computing field on a model to modify a schema is weird.

In anycase I really think that filtering should not be done on the frontend. Thibault was totally crazy when he realize that we return duplicated information in the API (api should be as simple as possible, any time we introduce something not natural, frontend will need to spend time for understand the "weird" behaviour, we should try to give the best experience)

I see an other possibility

Filtering with a domain

In shopinvader_schema_sale/schemas/sale.py

    @classmethod
    def from_sale_order(cls, odoo_rec):
        line_domain = self.env["sale.order.line"]._get_domain_without_technical_line()
        return cls.model_construct(
            ...
            lines=[SaleLine.from_sale_order_line(line) for line in odoo_rec.order_line.filtered_domain(line_domain)],
            ....

Then in this module we can extend

class SaleOrderLine

    def _get_domain_without_technical_line(self):
         res = super()..
         return expression.AND([res, [('is_delivery', '=', False)])

And in this PR (that add an endpoint for exposing directly "sale.order.line") : https://github.com/shopinvader/odoo-shopinvader/pull/1508/files

We can extend the domain to include this technical domain : https://github.com/shopinvader/odoo-shopinvader/pull/1508/files#diff-2dd5f80d9018355d48ebc8580045cc8e6d85f12ef8d81842f08492892a1a7ee0R68

    def _get_domain_adapter(self):
        domain = self.env["sale.order.line"]._get_domain_without_technical_line()
        return expression.AND([domain, [
            ("order_id.partner_id", "=", self.partner.id),
            ("order_id.typology", "=", "sale"),
        ]]

Note:

• maybe we should found a better name for the method that return the domain "_get_domain_main_line"
• maybe this method should be on an other object (the schema of SaleOrderLine?)

What do you think ?

[sbidoul]

And what are the drawbakcs of @marielejeune's proposal in #11 (comment).

[sebastienbeau]

Maybe I didn't understand what @marielejeune have done exactly.
But what Thibault tell me is that it was done on odoo side in the custom.

I think we should do it by default in this module (if not we always have to hack the custom)
But maybe I miss something

[sbidoul]

Yes I agree we should do something by default.

But I mean:

We thus used the is_delivery and is_reward_line flags that are present on a SOL. But we could expose these flags directly in the SOL schema

would that be simple enough for the frontend dev?

[sebastienbeau]

I do not think that exposing the field "is_reward_line" and "is_delivery" is enough, it's better to filter this lines by default.
If not frontend will have to filter this line every where (quotation, cart, sale...)

The issue is that right now in the schema there is not hook to filter line based on this boolean

[sebastienbeau]

Note I have try to have only "one" place where we filter this line.
But if it's more understandable, we can have a simple hook '_filter_sale_line" in the schema of the SaleOrder.

And simple also inherit the "def _get_domain_adapter(self):" to filter the unwanted line when we expose the sale.order.line directly