Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MIG] stock_by_warehouse_sale: Migration to 17.0 T#79454 #1643

Merged

Conversation

xmglord
Copy link
Contributor

@xmglord xmglord commented Apr 8, 2024

No description provided.

@xmglord
Copy link
Contributor Author

xmglord commented Apr 9, 2024

@isaako34 and @rolandojduartem please review.

@rolandojduartem
Copy link
Contributor

@xmglord task in the MR description please

#
msgid ""
msgstr ""
"Project-Id-Version: Odoo Server 16.0+e\n"
Copy link
Contributor

Choose a reason for hiding this comment

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

Update the whole .po, please

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@rolandojduartem
Copy link
Contributor

Please, test your PR functionally. This error happened when I tried to open a sale order line

image

@rolandojduartem
Copy link
Contributor

Please, test your PR functionally. This error happened when I tried to open a sale order line

image

Actually, this error comes from stock_by_warehouse, it is required functional review previous merging cc @luisg123v @desdelinux

@rolandojduartem
Copy link
Contributor

rolandojduartem commented Apr 9, 2024

Use this as example https://github.com/odoo/odoo/blob/ce3b096de7a0dcbf2bc8387d0c17ac3d320a7ec4/addons/mail/static/src/js/onchange_on_keydown.js#L69, the fix in other PR and be sure to test in the both modules.

@xmglord xmglord changed the title [MIG] stock_by_warehouse_sale: Migration to 17.0 [MIG] stock_by_warehouse_sale: Migration to 17.0 T#79454 Apr 10, 2024
@xmglord xmglord force-pushed the 17.0-mig_v17_stock_warehouse_sale-xmglord branch 5 times, most recently from 3c2616f to d87b040 Compare April 26, 2024 02:30
@xmglord
Copy link
Contributor Author

xmglord commented Apr 26, 2024

@luisg123v please review but don't merge until @rolandojduartem give his functional review.

@luisg123v luisg123v self-requested a review April 26, 2024 02:41
@rolandojduartem
Copy link
Contributor

rolandojduartem commented Apr 26, 2024

No, the changes on stock_by_warehouse must be other PR, that is a fix, not a migration, you can rebase over that PR for functional review, but I do not think is a good idea mix changes in the same PR

@xmglord xmglord force-pushed the 17.0-mig_v17_stock_warehouse_sale-xmglord branch 2 times, most recently from 931ba79 to a0281b6 Compare May 2, 2024 22:24
@xmglord xmglord force-pushed the 17.0-mig_v17_stock_warehouse_sale-xmglord branch from a0281b6 to 0c43bdc Compare May 3, 2024 05:02
@xmglord xmglord requested a review from luisg123v May 3, 2024 05:02
@rolandojduartem
Copy link
Contributor

This MR is not rebased over !1651, so it fails


def _compute_get_warehouses_stock(self):
for line in self:
line.warehouses_stock = line.product_id.with_context(

This comment was marked as resolved.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It doesn't work since the field in the product is not yet calculated and is not stored so is need to call the compute directly.

Copy link
Contributor

Choose a reason for hiding this comment

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

True, sorry

@xmglord xmglord force-pushed the 17.0-mig_v17_stock_warehouse_sale-xmglord branch from 0c43bdc to 248ec07 Compare May 4, 2024 01:34
@xmglord
Copy link
Contributor Author

xmglord commented May 4, 2024

This MR is not rebased over !1651, so it fails

I already rebase but for some reason when installing this modules the widget fails.

@luisg123v luisg123v removed their request for review May 6, 2024 17:37
@desdelinux
Copy link

Hello, what are we missing here? @luisg123v @xmglord

@rolandojduartem
Copy link
Contributor

I am going to test it then

@rolandojduartem
Copy link
Contributor

I think it is not working as I expected, see the sale order, I did not activate the button and the widget was loaded at the beggining, could you check it?

Screenshot from 2024-05-16 12-16-05

Screenshot from 2024-05-16 12-14-39

def _compute_warehouse_stock(self):
for record in self:
record.warehouse_id = self.order_id.warehouse_id
record._compute_get_warehouses_stock()
Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, you forgot the condition of warehouses_stock_recompute

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done, thanks


def _compute_get_warehouses_stock(self):
for line in self:
line.warehouses_stock = line.product_id.with_context(
Copy link
Contributor

Choose a reason for hiding this comment

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

True, sorry

@rolandojduartem
Copy link
Contributor

Alsoo llok the button, the widget and the button are so close

hugho-ad and others added 14 commits May 22, 2024 01:02
…ad (Vauxoo#1358)

We need avoid using compute fields as possible because of the following issue:
 - odoo/odoo#30578
warehouses_stock is a computed field used just one time but re-computed too many times:
 - Opening sale order (one time by each line)
 - Saving sale order
 - Choosing a product_id
And it computation is more and more slow when the database grows.
It is making a big overhead saving sale.order.

With this change we have the same behaviour using it just for onchange
product_id or forcing from a seudo-button (field boolean)
A real button is not used since that it requires save the record to work

Using the technical of fields with store=False and onchange methods

Check more details from Vauxoo#1358
In order to get the real value, since that this field is not set when onchange method is used
The following is performed:
- Remove useless test that actually did nothing
- Remove data tags from views
- Remove encoding headers from Python files
- Remove superfluous readonly attribute from related fields
- Remove useless keys from the manifest
- Fix typos on the README
* [I18N] stock_by_warehouse_sale: Fix term names

Some term names are handled differently on v12.0, but those changes were
not taken into account when migrating this module from v11.0.

* [I18N] stock_by_warehouse_purchase: Fix term names

Some term names are handled differently on v12.0, but those changes were
not taken into account when migrating this module from v11.0.
This removes:
- Hencoding headers, fixing lint `utf8-coding-comment`
- Headers like `# Part of Odoo`
- And other ones
In previous Odoo versions, the form view was poped up when editing a
sale order line, as long as the option "Manage Product packaging" was
enabled. This is no longer the case [1], so we re-enable it manually,
in order to be able to see current inventory when filling a line.

[1] odoo/odoo@e813487e75f1
…able products

This functionality is for storable products to show the information in
the warehouses, now this field is invisible for products which are not
storable products.
@xmglord xmglord force-pushed the 17.0-mig_v17_stock_warehouse_sale-xmglord branch 2 times, most recently from dcf2f79 to 60c0afc Compare May 22, 2024 01:20
@xmglord
Copy link
Contributor Author

xmglord commented May 22, 2024

@rolandojduartem I already fix the space between the widget and the button, please review again.

@desdelinux
Copy link

Friendly Reminder @rolandojduartem

@rolandojduartem
Copy link
Contributor

Functionally and technically LGTM @luisg123v

- Simplify the way to define modifiers as states, required, readonly,
  invisible and column_invisible as part of [1].
- Adapt the stock by warehouse field to be computed.

[1] odoo/odoo#104741
@xmglord xmglord force-pushed the 17.0-mig_v17_stock_warehouse_sale-xmglord branch from 60c0afc to 80a9672 Compare May 23, 2024 22:42
@xmglord xmglord requested a review from luisg123v May 23, 2024 22:43
Copy link
Contributor

@luisg123v luisg123v left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@luisg123v luisg123v merged commit 19e80c2 into Vauxoo:17.0 May 23, 2024
3 checks passed
@luisg123v luisg123v deleted the 17.0-mig_v17_stock_warehouse_sale-xmglord branch May 23, 2024 23:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants