Skip to content

[13.0][ADD]product_attribute_value_menu#997

Merged
OCA-git-bot merged 1 commit into
OCA:13.0from
ForgeFlow:13.0-add-product_attribute_value
Feb 11, 2022
Merged

[13.0][ADD]product_attribute_value_menu#997
OCA-git-bot merged 1 commit into
OCA:13.0from
ForgeFlow:13.0-add-product_attribute_value

Conversation

@GuillemCForgeFlow
Copy link
Copy Markdown
Contributor

This new module it's based on this #984 which was proposing an improvement to the product_attribute_archive module. Instead, i'ts now a separate module which adds a Attribute Values menu item and tree view.
@ForgeFlow @JordiMForgeFlow @AaronHForgeFlow

Copy link
Copy Markdown
Contributor

@JordiMForgeFlow JordiMForgeFlow left a comment

Choose a reason for hiding this comment

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

Simple but really useful! Nice one @GuillemCForgeFlow 👍🏼

@JordiBForgeFlow
Copy link
Copy Markdown
Member

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.

Thanks @GuillemCForgeFlow
Is it possible to remove the dependency on product_attribute_archive?

@GuillemCForgeFlow
Copy link
Copy Markdown
Contributor Author

Thanks @GuillemCForgeFlow Is it possible to remove the dependency on product_attribute_archive?

I have added the field active on the tree view. For this reason it's added on the dependencies, but I could remove it from both sides. As you prefer.

@JordiBForgeFlow
Copy link
Copy Markdown
Member

@GuillemCForgeFlow product_attribute_archive should be the one depending on this one, not the other way around, I think.

@AaronHForgeFlow
Copy link
Copy Markdown
Contributor

AaronHForgeFlow commented Jan 25, 2022

@GuillemCForgeFlow product_attribute_archive should be the one depending on this one, not the other way around, I think.

Yes, then you can inherit the view on the product_attribute_archive module in order to add the active fields.

Sorry for the extra work, but I think this will improve extensibility a lot.

@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 13.0-add-product_attribute_value branch from 58044a1 to 8a40f3d Compare January 25, 2022 10:28
@GuillemCForgeFlow GuillemCForgeFlow changed the title [13.0][ADD]product_attribute_value [13.0][ADD]product_attribute_value_menu Jan 25, 2022
@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 13.0-add-product_attribute_value branch from 8a40f3d to 883ccf3 Compare January 25, 2022 10:39
@GuillemCForgeFlow
Copy link
Copy Markdown
Contributor Author

@AaronHForgeFlow @JordiBForgeFlow
Done! Now there's a new PR: #998 which adds a dependency to the product_attribute_archive module and inherits the tree view adding the active field. Also, the name of the module has been changed.

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.

👍 great! -thanks for attending all the suggestions

Comment thread product_attribute_value_menu/views/product_attribute_value.xml Outdated
Comment thread product_attribute_value_menu/views/product_attribute_value.xml
@GuillemCForgeFlow GuillemCForgeFlow force-pushed the 13.0-add-product_attribute_value branch from 883ccf3 to 925c886 Compare January 26, 2022 12:02
Copy link
Copy Markdown

@albariera albariera left a comment

Choose a reason for hiding this comment

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

LGTM 👍 😊

Copy link
Copy Markdown
Contributor

@Rad0van Rad0van left a comment

Choose a reason for hiding this comment

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

LGTM! Tested.

@OCA-git-bot
Copy link
Copy Markdown
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@dreispt
Copy link
Copy Markdown
Member

dreispt commented Feb 11, 2022

/ocabot merge nobump

@OCA-git-bot
Copy link
Copy Markdown
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 13.0-ocabot-merge-pr-997-by-dreispt-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit a25c28b into OCA:13.0 Feb 11, 2022
@OCA-git-bot
Copy link
Copy Markdown
Contributor

Congratulations, your PR was merged at 637f8a2. 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.

8 participants