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.

Sign up for GitHub

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

Jump to bottom

[16.0][MIG] attribute_set #141

Merged
merged 31 commits into from
Aug 18, 2023
Merged

[16.0][MIG] attribute_set #141

merged 31 commits into from
Aug 18, 2023

Conversation

sbejaoui
Copy link

@sbejaoui sbejaoui commented Feb 28, 2023 •
edited
Loading

@sbejaoui sbejaoui mentioned this pull request Feb 28, 2023
Merged
@sbejaoui sbejaoui force-pushed the 16.0-attribute_set-sbj branch 3 times, most recently from d8c776f to 294d8f5 Compare February 28, 2023 13:38
lmignon
lmignon suggested changes Mar 2, 2023
View reviewed changes
attribute_set/views/menu_view.xml Outdated Show resolved Hide resolved
@sbejaoui sbejaoui requested a review from lmignon March 2, 2023 19:38
lmignon
lmignon suggested changes Mar 3, 2023
View reviewed changes
Copy link
Contributor

@lmignon lmignon left a comment

Choose a reason for hiding this comment

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

@sbejaoui No menu to manage attribute groups, and it's not possible to create one from the attribute form since the field is declared with 'no create' into the form. Strange to see that views (form, tree and search) are declared but not available into the menu...

@sbejaoui sbejaoui requested a review from lmignon March 3, 2023 08:26
@sbejaoui
Copy link
Author

sbejaoui commented Mar 3, 2023

@sbejaoui No menu to manage attribute groups, and it's not possible to create one from the attribute form since the field is declared with 'no create' into the form. Strange to see that views (form, tree and search) are declared but not available into the menu...

thanks @lmignon for the review.

It's weird that this object didn't have a menu item in all previous versions. I added the menu and the view_mode form to attribute_attribute_sort_action.

lmignon
lmignon approved these changes Mar 3, 2023
View reviewed changes
Copy link
Contributor

@lmignon lmignon 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 @sbejaoui LGTM (Code review + functional tests)

@StevenSermeus StevenSermeus mentioned this pull request Apr 6, 2023
Closed
@acsonefho
Copy link

@sbejaoui Can you fix conflict please?

This was referenced Jul 17, 2023
Closed
Closed
@dsolanki-initos dsolanki-initos mentioned this pull request Jul 24, 2023
Closed
@cyrilmanuel
Copy link

@sbejaoui Can you fix conflict please or if someone can do the fix @acsonefho

@simahawk simahawk changed the title [16.0][MIG] - attribute_set [16.0][MIG] attribute_set Aug 8, 2023
simahawk
simahawk reviewed Aug 8, 2023
View reviewed changes
attribute_set/models/attribute_attribute.py Outdated Show resolved Hide resolved
attribute_set/models/attribute_attribute.py Outdated Show resolved Hide resolved
attribute_set/models/attribute_attribute.py Outdated Show resolved Hide resolved
attribute_set/models/attribute_attribute.py Outdated Show resolved Hide resolved
attribute_set/models/attribute_attribute.py Outdated Show resolved Hide resolved
attribute_set/models/attribute_set_owner.py Outdated Show resolved Hide resolved
attribute_set/readme/DESCRIPTION.rst Outdated Show resolved Hide resolved
attribute_set/tests/test_custom_attribute.py Outdated Show resolved Hide resolved
attribute_set/utils/orm.py Outdated Show resolved Hide resolved
test-requirements.txt Outdated Show resolved Hide resolved
@OCA-git-bot
Copy link
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). 🤖

@simahawk
Copy link
Contributor

simahawk commented Aug 9, 2023

/ocabot migration attribute_set

@OCA-git-bot OCA-git-bot added this to the 16.0 milestone Aug 9, 2023
@OCA-git-bot OCA-git-bot mentioned this pull request Aug 9, 2023
Open
3 tasks
@clementmbr @sbejaoui
[REF] rename modules based on attribute_set and pim
7dbaae2 
New organization :

- attribute_set (former base_custom_attribute)
- product_attribute_set (former pim_custom_attribute but without menus)
- pim (The "PIM application" former pim_base)
- pim_attrubute_set (depends on product_attribute_set and adds menus in
the PIM application)
Pierrick Brun and others added 8 commits August 15, 2023 14:22
@sbejaoui sbejaoui requested a review from simahawk August 15, 2023 12:37
@simahawk
Copy link
Contributor

Suggestions for commits' rewrite:

[IMP] - set context for native fields -> [IMP] attribute_set: set ctx for native fields
[FIX] - fix menu position -> [FIX] attribute_set: fix menu position
[IMP] - add menu item for attribute group - > [IMP] attribute_set: add group menu item
[IMP] - improve code and documentation -> [IMP] attribute_set: misc improvements

Also, I took over #132 and replaced it w/ #169 to speed it up.
Could you pls drop the commit from 132 and pick the one from 169? Merci!

kevinkhao and others added 5 commits August 18, 2023 11:26
@kevinkhao @sbejaoui
[FIX] attribute_set: fix create options w/ rel model
8aad94e 
When you define an attribute as a select field and add a related model,
you have the option to "Load attribute options".

On this wizard, an "option_ids" dummy field is created via
fields_view_get.

This change deletes option_ids from create (once the options are created) and read
@sbejaoui
[16.0][MIG] - attribute_set
0ca424d
@sbejaoui
[IMP] attribute_set: set ctx for native fields
740a757
@sbejaoui
[FIX] attribute_set: fix menu position
7054f5b 
menu items are visible only for two levels
@sbejaoui
[IMP] attribute_set: add group menu item
9c23050
@sbejaoui sbejaoui force-pushed the 16.0-attribute_set-sbj branch 2 times, most recently from 13f9495 to c3e034a Compare August 18, 2023 09:28
@sbejaoui
Copy link
Author

Suggestions for commits' rewrite:

[IMP] - set context for native fields -> [IMP] attribute_set: set ctx for native fields [FIX] - fix menu position -> [FIX] attribute_set: fix menu position [IMP] - add menu item for attribute group - > [IMP] attribute_set: add group menu item [IMP] - improve code and documentation -> [IMP] attribute_set: misc improvements

Also, I took over #132 and replaced it w/ #169 to speed it up. Could you pls drop the commit from 132 and pick the one from 169? Merci!

done,

@simahawk
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 16.0-ocabot-merge-pr-141-by-simahawk-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 6d83728 into OCA:16.0 Aug 18, 2023
6 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 42cbe9b. 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
Reviewers

@pedrobaeza pedrobaeza pedrobaeza left review comments

@rvalyi rvalyi rvalyi left review comments

@acsonefho acsonefho acsonefho approved these changes

@cyrilmanuel cyrilmanuel cyrilmanuel approved these changes

@lmignon lmignon lmignon approved these changes

@simahawk simahawk simahawk approved these changes

Assignees
No one assigned
Labels
approved merged 🎉 ready to merge
Projects
None yet
Milestone
16.0
Development

Successfully merging this pull request may close these issues.