Skip to content

[16.0][IMP] sale_elaboration: set products profile from its categories#3809

Merged
OCA-git-bot merged 1 commit intoOCA:16.0from
moduon:16.0-IMP-sale_elaboration
Aug 9, 2025
Merged

[16.0][IMP] sale_elaboration: set products profile from its categories#3809
OCA-git-bot merged 1 commit intoOCA:16.0from
moduon:16.0-IMP-sale_elaboration

Conversation

@Andrii9090-tecnativa
Copy link
Copy Markdown

I added new functionality to propagate the elaboration profile from the category to the products. When you set the elaboration profile in the category, all its products will have this profile.

MT-10786 @moduon @yajo @rafaelbn @Shide @chienandalu @u0f please review if you want 😄

@OCA-git-bot
Copy link
Copy Markdown
Contributor

Hi @yajo, @rafaelbn, @sergio-teruel, @CarlosRoca13,
some modules you are maintaining are being modified, check this out!

Copy link
Copy Markdown
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Thanks @Andrii9090 ! Some comments below:

  • The title of the PR should be: [16.0][IMP] sale_elaboration: add new functionality to propagate the elaboration profile from the category to the product
  • You can use a more concise title anyway: [16.0][IMP] sale_elaboration: set products profile from its categories
  • Check tests fails.

Comment on lines +28 to +32
<xpath expr="//tree" position="attributes">
<attribute name="multi_edit">1</attribute>
</xpath>
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add a comment on why you're doing this

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have added comment

@Andrii9090-tecnativa Andrii9090-tecnativa changed the title [IMP[16.0]] sale_elaboration: add new functionality to propagate the elaboration profile from the category to the products [16.0][IMP] sale_elaboration: set products profile from its categories Jul 11, 2025
@Andrii9090-tecnativa Andrii9090-tecnativa force-pushed the 16.0-IMP-sale_elaboration branch from 1050eed to 774a020 Compare July 11, 2025 15:49
Copy link
Copy Markdown
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Some more small changes ;)

comodel_name="product.elaboration.profile", string="Elaboration Profile"
)

def write(self, vals):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You should deal with the create as well

Comment on lines +14 to +19
#. Go to *Inventory > Configuration > Product Categories* and select one or you
can add a column of Elaboration Profiles in the Product Categories list view.
#. Set the elaboration profile you want to use for all products in this category.
It is used in the case if the category hasn't child category. When you
create/update a product, the elaboration profile is set from the product's
category if the elaboration profile isn't yet setted.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Keep some idention so it's easier to read it in code:

Suggested change
#. Go to *Inventory > Configuration > Product Categories* and select one or you
can add a column of Elaboration Profiles in the Product Categories list view.
#. Set the elaboration profile you want to use for all products in this category.
It is used in the case if the category hasn't child category. When you
create/update a product, the elaboration profile is set from the product's
category if the elaboration profile isn't yet setted.
#. Go to *Inventory > Configuration > Product Categories* and select one or you
can add a column of Elaboration Profiles in the Product Categories list view.
#. Set the elaboration profile you want to use for all products in this category.
It is used in the case if the category hasn't child category. When you
create/update a product, the elaboration profile is set from the product's
category if the elaboration profile isn't yet setted.

Comment on lines +14 to +15
#. Go to *Inventory > Configuration > Product Categories* and select one or you
can add a column of Elaboration Profiles in the Product Categories list view.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add that in an extra comment after this block like: You can also use multi-edit in the list view to set the profile in multiple categories at once. Just unhide it from the dots in the right.

Suggested change
#. Go to *Inventory > Configuration > Product Categories* and select one or you
can add a column of Elaboration Profiles in the Product Categories list view.
#. Go to *Inventory > Configuration > Product Categories* and select one.

Comment on lines +17 to +19
It is used in the case if the category hasn't child category. When you
create/update a product, the elaboration profile is set from the product's
category if the elaboration profile isn't yet setted.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Set this aside:

When you create product or update its category , the elaboration profile is set from the product's
category. Anyway, it's possible to change it afterwards.

Suggested change
It is used in the case if the category hasn't child category. When you
create/update a product, the elaboration profile is set from the product's
category if the elaboration profile isn't yet setted.
The profiles is just set in the category products and it won't be propagated to the child category products.

@Andrii9090-tecnativa Andrii9090-tecnativa force-pushed the 16.0-IMP-sale_elaboration branch from 774a020 to e9c3de5 Compare July 18, 2025 13:53
@Andrii9090-tecnativa Andrii9090-tecnativa marked this pull request as draft July 30, 2025 07:03
@Andrii9090-tecnativa Andrii9090-tecnativa force-pushed the 16.0-IMP-sale_elaboration branch from e9c3de5 to 7cf2130 Compare July 31, 2025 13:46
@Andrii9090-tecnativa
Copy link
Copy Markdown
Author

This short video highlights the newest improvements to the module. Thank you @yajo 😄

16.0.IMP.sale_elaboration_.set.products.profile.from.its.categories.3809.mp4

@OCA-git-bot
Copy link
Copy Markdown
Contributor

Hi @sergio-teruel, @CarlosRoca13, @yajo, @rafaelbn,
some modules you are maintaining are being modified, check this out!

@Andrii9090-tecnativa Andrii9090-tecnativa marked this pull request as ready for review August 1, 2025 09:56
Copy link
Copy Markdown
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

I can't see the logic. Maybe you scrambled your commits? 😅

</field>
</field>
</record>

Copy link
Copy Markdown
Member

@yajo yajo Aug 4, 2025

Choose a reason for hiding this comment

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

issue: Please disable the automatic formatter that you probably have in your IDE for XML files, and that removes these lines. Then, leave them as they were.

These lines are not wrong. Just let prettier (the one in pre-commit) do the formatting. This way we avoid distractions when reviewing these unneeded stylistic changes.

@chienandalu chienandalu force-pushed the 16.0-IMP-sale_elaboration branch from 7cf2130 to 8ca723b Compare August 7, 2025 08:15
Copy link
Copy Markdown
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

@Andrii9090 I changed the orientation of your changes for the most simple logic that gets to the original goal.

In a summary

  • The module behaves as it was until now.
  • When we set a elaboration profile in the category and no profile is set in the product, that will be the default. Otherwise, no profile is applied and all the elaborations are available.
  • Sadly, it's hard to test this with the test.Form tools (I'll do some research) as the domain is evaluated in the view and it seems that it doesn't care about it at that level... O left some of your original test case declarations... although the go with no effect, so maybe I remove them for the sake of commit clarity...

@rafaelbn @sergio-teruel all yours if you want to review 🙂

Elaboration profiles can now be set from the category so if no profile is set in
the product it defaults to that one.

TODO:

- `test.Form` doesn't constrain the view domain for profiles... so the only
  way to test the flow would be either to use a helper domain field or a
 tour... Some tooling left in the test case anyway.

MT-10786

Co-authored-by: David Vidal <david@moduon.team>
@chienandalu chienandalu force-pushed the 16.0-IMP-sale_elaboration branch from 8ca723b to 0dc1f08 Compare August 7, 2025 08:37
@rafaelbn rafaelbn added this to the 16.0 milestone Aug 7, 2025
Copy link
Copy Markdown
Member

@rafaelbn rafaelbn left a comment

Choose a reason for hiding this comment

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

Tested functionaly 👍🏼

2 minutes video with a full explanation

https://www.loom.com/share/d0570c9362ce47e6abd853135aac6807?sid=8ae7d7f6-80f9-4df0-8cc7-85999139cedf

@rafaelbn rafaelbn requested a review from yajo August 8, 2025 10:02
Copy link
Copy Markdown

@fcvalgar fcvalgar left a comment

Choose a reason for hiding this comment

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

Functional review,

LGTM, thank you @Andrii9090

@rafaelbn
Copy link
Copy Markdown
Member

rafaelbn commented Aug 9, 2025

/ocabot merge minor

@OCA-git-bot
Copy link
Copy Markdown
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 16.0-ocabot-merge-pr-3809-by-rafaelbn-bump-minor, awaiting test results.

@rafaelbn
Copy link
Copy Markdown
Member

rafaelbn commented Aug 9, 2025

Please @chienandalu port to 17 and 18 in the same task 😄 ❤️ Thank you!

@OCA-git-bot OCA-git-bot merged commit 1874c7e into OCA:16.0 Aug 9, 2025
15 checks passed
@OCA-git-bot
Copy link
Copy Markdown
Contributor

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

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.

6 participants