Skip to content

[18.0][MIG] sale_blanket_order#3642

Closed
IsabelAForgeFlow wants to merge 66 commits intoOCA:18.0from
ForgeFlow:18.0-mig-sale_blanket_order
Closed

[18.0][MIG] sale_blanket_order#3642
IsabelAForgeFlow wants to merge 66 commits intoOCA:18.0from
ForgeFlow:18.0-mig-sale_blanket_order

Conversation

@IsabelAForgeFlow
Copy link
Copy Markdown
Contributor

@IsabelAForgeFlow IsabelAForgeFlow commented Mar 25, 2025

Migration to 18.0

Supersede: #3370

Copy link
Copy Markdown
Contributor

@AaronHForgeFlow AaronHForgeFlow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the supersede and include all the improvements!

It works fine, just some minor comments.

Comment on lines +31 to +23
<xpath expr="//field[@name='order_line']" position="before">
<field name="disable_adding_lines" invisible="1" />
</xpath>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is not necessary, it works without that in v18

Suggested change
<xpath expr="//field[@name='order_line']" position="before">
<field name="disable_adding_lines" invisible="1" />
</xpath>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During functional testing I can confirm this part is required.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@IsabelAForgeFlow can you add this part back? sorry I made you change it before.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, the forward port of this: #3620 does not work right away in v18. I am working in the adaptation.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It came out that the create attribute cannot be dynamically changed like it was, it has to be done using js patches, I just did.

Comment on lines +11 to +20
<xpath
expr="//field[@name='order_line']//list/field[@name='product_id']"
position="after"
>
<field
name="blanket_order_line"
context="{'from_sale_order': True}"
column_invisible="not parent.blanket_order_id"
/>
</xpath>
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<xpath
expr="//field[@name='order_line']//list/field[@name='product_id']"
position="after"
>
<field
name="blanket_order_line"
context="{'from_sale_order': True}"
column_invisible="not parent.blanket_order_id"
/>
</xpath>

I think this is not necessary, it works without that in v18

<field name="arch" type="xml">
<list create="false">
<field name="sequence" widget="handle" />
<field name="name" invisible="1" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<field name="name" invisible="1" />

I think this can be removed as well

name="product_id"
context="{'partner_id':parent.partner_id, 'quantity':original_uom_qty, 'company_id': company_id}"
/>
<field name="product_uom" invisible="1" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<field name="product_uom" invisible="1" />
<field name="product_uom" invisible="1" />

I think this is also not necessary

)
if blanket_order.state == "expired":
raise UserError(
_("You can't create a sale order from " "an expired blanket order!")
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_("You can't create a sale order from " "an expired blanket order!")
self.env._("You can't create a sale order from " "an expired blanket order!")

)
line_company_id = line.company_id and line.company_id.id or False
if company_id is not False and line_company_id != company_id:
raise UserError(_("You have to select lines " "from the same company."))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise UserError(_("You have to select lines " "from the same company."))
raise UserError(self.env._("You have to select lines " "from the same company."))

payment_term_id = 0
for line in self.line_ids.filtered(lambda line: line.qty != 0.0):
if line.qty > line.remaining_uom_qty:
raise UserError(_("You can't order more than the remaining quantities"))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise UserError(_("You can't order more than the remaining quantities"))
raise UserError(self.env._("You can't order more than the remaining quantities"))

payment_term_id = False

if not order_lines_by_customer:
raise UserError(_("An order can't be empty"))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise UserError(_("An order can't be empty"))
raise UserError(self.env._("An order can't be empty"))


if not currency_id:
raise UserError(
_(
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
_(
self.env._(

res.append(sale_order.id)
return {
"domain": [("id", "in", res)],
"name": _("Sales Orders"),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"name": _("Sales Orders"),
"name": self.env._("Sales Orders"),

@IsabelAForgeFlow IsabelAForgeFlow force-pushed the 18.0-mig-sale_blanket_order branch 2 times, most recently from 68781a9 to 836a13a Compare March 26, 2025 10:17
@rousseldenis
Copy link
Copy Markdown
Contributor

@IsabelAForgeFlow Could you move to https://github.com/OCA/sale-blanket ?

And consider checking behavior of new approach : OCA/sale-blanket#2

Thanks.

@IsabelAForgeFlow IsabelAForgeFlow force-pushed the 18.0-mig-sale_blanket_order branch from 836a13a to e5a2fb5 Compare April 2, 2025 06:39
<t t-set="current_subtotal" t-value="0" />
<t t-foreach="doc.line_ids" t-as="l">
<t
t-set="current_subtotal"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Noticed that the current_subtotal amount is being counted twice, which is wrong.
It should be counted only once:
<t t-set="current_subtotal" t-value="current_subtotal + l.price_subtotal"/>

Also remove the groups - it no longer exists

groups="account.group_show_line_subtotals_tax_included"
/>
<tr
t-att-class="'bg-200 fw-bold o_line_section' if l.display_type == 'line_section' else 'fst-italic o_line_note' if l.display_type == 'line_note' else ''"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe remove bg-200 - line sections does look better without the gray background imo

/>
</td>
</t>
<t t-if="l.display_type == 'line_section'">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You may use t-elif here

<t t-set="current_section" t-value="l" />
<t t-set="current_subtotal" t-value="0" />
</t>
<t t-if="l.display_type == 'line_note'">
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too
t-elif

_name = "sale.blanket.order.line"
_description = "Blanket Order Line"
_inherit = ["mail.thread", "mail.activity.mixin", "analytic.mixin"]

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add _order = "sequence, id" or _order = "order_id, sequence, id" otherwise changing the sequence of the order lines (e.g. line section or line note) will not correspond when printing the report pdf

@rousseldenis
Copy link
Copy Markdown
Contributor

/ocabot migration sale_blanket_order

@OCA-git-bot
Copy link
Copy Markdown
Contributor

The migration issue (#3344) has not been updated to reference the current pull request because a previous pull request (#3370) is not closed.
Perhaps you should check that there is no duplicate work.
CC @BertVGroenendael

@rousseldenis
Copy link
Copy Markdown
Contributor

@IsabelAForgeFlow Could you improve migration commit message ? Thanks

image

andreparames and others added 17 commits May 29, 2025 15:58
When duplicating a confirmed blanket order, the new copy shouldn't
keep the state nor the sequence number (name).
Steps to reproduce:

* create and confirm a blanket order (BO) with (product A, qty 30) and (product B, qty 20)
* from the BO create a SO with (product A, qty 10) and (product B, qty 10)
* from the BO create a SO with (product A, qty 20) and (product B, qty 0)
* from the BO create another SO with (product B, qty 10)

Current behavior:

It raises the exception "The sale has already been completed.".

Expected behavior:

No exception is raised.
Currently translated at 7.1% (11 of 154 strings)

Translation: sale-workflow-12.0/sale-workflow-12.0-sale_blanket_order
Translate-URL: https://translation.odoo-community.org/projects/sale-workflow-12-0/sale-workflow-12-0-sale_blanket_order/de/
@IsabelAForgeFlow IsabelAForgeFlow force-pushed the 18.0-mig-sale_blanket_order branch from 9d587c8 to b0e4c5a Compare May 29, 2025 14:03
@IsabelAForgeFlow
Copy link
Copy Markdown
Contributor Author

@IsabelAForgeFlow Could you improve migration commit message ? Thanks

image

Done!

Copy link
Copy Markdown

@yung-wang yung-wang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems to work fine

@mmequignon
Copy link
Copy Markdown
Member

@IsabelAForgeFlow Could you move to https://github.com/OCA/sale-blanket ?

And consider checking behavior of new approach : OCA/sale-blanket#2

Thanks.

@IsabelAForgeFlow what about this comment?

Copy link
Copy Markdown
Contributor

@jbaudoux jbaudoux left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, there have been a rewrite of this module in v16 and it is now in its own repo https://github.com/OCA/sale-blanket/tree/16.0

@AaronHForgeFlow
Copy link
Copy Markdown
Contributor

Hi @mmequignon @jbaudoux

It looks that it means to replace this module and adding some improvements on it. Could you point out the most important differences? If a new module means to replace another I think it should provide a list of what is better from the existing solution. I will test sale_order_blanket_order myself but I would save a lot of time if you help me :) Thank you.

@AaronHForgeFlow
Copy link
Copy Markdown
Contributor

Ok, I tested the sale-blanket module. You can deliver and invoice directly from the blanket order and also you can also do a call off order separately in order to deliver the blanket order partially. But basically both modules do the same.

About the transition, even if the sale-blanket module version take over, there are going to be at least 2 Odoo versions 16.0 & 17.0 where the 2 modules will coexist in OCA, so why not keeping also both versions in v18? Then in v19 a transition plan has to be made.

@AaronHForgeFlow AaronHForgeFlow force-pushed the 18.0-mig-sale_blanket_order branch from b0e4c5a to c052609 Compare July 9, 2025 13:09
@AaronHForgeFlow
Copy link
Copy Markdown
Contributor

It seems there is an agreement on keeping the 2 versions of the blanket orders, as discussed here: OCA/sale-blanket#6

Now I would rather like to keep this module in this current repo. Just to prevent confusion on the sale-blanket repo. The 2 modules have a very similar name.

@AaronHForgeFlow
Copy link
Copy Markdown
Contributor

I superseded this PR to sale-blanket repo: OCA/sale-blanket#8

There was some discussion in here: OCA/sale-blanket#6 and I want to move forward so it is not big deal to me to continue in the other repo.

Vicent-S73 pushed a commit to Studio73/e-commerce that referenced this pull request Aug 12, 2025
@MiquelRForgeFlow
Copy link
Copy Markdown
Contributor

This PR should be closed (as it was superseded by OCA/sale-blanket#8).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.