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

[12.0][MIG] base_custom_info: Migration to 12.0 #1721

Merged
merged 17 commits into from Dec 18, 2019

Conversation

Tardo
Copy link
Member

@Tardo Tardo commented Nov 8, 2019

Cc @Tecnativa TT18593

@AdriaGForgeFlow
Copy link

Are you planning on finishing this? Do you need help?

@pedrobaeza
Copy link
Member

Yes, we need to check a problem with the onchanges. I'll check this week.

@AdriaGForgeFlow
Copy link

Yes, we need to check a problem with the onchanges. I'll check this week.

Okay, thanks!

@pedrobaeza pedrobaeza changed the title [WIP][12.0][MIG] base_custom_info: Migration to 12.0 [12.0][MIG] base_custom_info: Migration to 12.0 Dec 5, 2019
@pedrobaeza pedrobaeza added this to the 12.0 milestone Dec 5, 2019
@pedrobaeza
Copy link
Member

This should be now reviewable. @Tardo I think you can squash a bit commits.

@oca-clabot
Copy link

Hey @Tardo, thank you for your Pull Request.

It looks like some users haven't signed our Contributor License Agreement, yet.
You can read and sign our full Contributor License Agreement here: http://odoo-community.org/page/cla
Here is a list of the users:

Appreciation of efforts,
OCA CLAbot

carlosdauden and others added 14 commits December 5, 2019 16:36
Mostly no changes from 8.0 after all.
# Conflicts:
#	base_custom_info/__manifest__.py
* Now you can define properties types, and access rules are inherited from the model/record
  linked to the custom info record.
* Simplified version of computed value.
* Implement for res.partner.
* Add tests and fix bugs discovered in the meantime.
* Allow to disable partner custom info tab, and custom info menu.
* All of it can be set within general settings.
* Now, by default, this module does not display custom info for partners unless in demo mode.
  Better fit for a base module.
* You can disable the top menu entry too if it disturbs you, or enable it for everybody.
* Give a special form when editing in partner custom info tab.
* Sortable properties.
* Sort values at onchange time.
* Improve performance in onchange.
* Split in several model files.
…the write

We must avoid to rely on the order in which computed fields (including related fields) and constrains methods are applied.
Due to a recent change into the ORM, the contrains on the model_id into CustomInfoTemplate is now called AFTER the recompute of the related model field into property_ids.info_value_ids
As side effect, when the constrains is called, the model on the info value is already updated with the new value and we no more know the old value....
@Tardo
Copy link
Member Author

Tardo commented Dec 16, 2019

Ready for review

@pedrobaeza
Copy link
Member

@AdriaGForgeFlow can you review?

Copy link

@AdriaGForgeFlow AdriaGForgeFlow left a comment

Choose a reason for hiding this comment

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

Done a code review and functional review and overall looks good to me.

Just a small error when trying to access the Values menu in Custom Info / Basic.
image

Another comment below.


The *value* will always be one of these.

Recursive templates using options

Choose a reason for hiding this comment

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

Is this feature enabled? I don't seem to find how to use it.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that this refers to the feature of mix properties of different templates for a selection.

Copy link

Choose a reason for hiding this comment

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

Yes, but how do you use it? How do you specify in a selection property that links to a different template?
When I follow the instructions in the README and select "Needs videogames" in the Smart Partners template, nothing happens, no new properties appear in the list to be filled. Even though I see this in the property form view:
image

See:
image

Copy link
Member Author

Choose a reason for hiding this comment

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

Finally, it's an outdated readme... moved to roadmap. This feature it's not implemented yet. Thanks!

@Tardo
Copy link
Member Author

Tardo commented Dec 17, 2019

Change done! thanks for the review

Tardo and others added 3 commits December 18, 2019 18:49
- Put force_save=1 for allowing the web client to send readonly values to server
- Switch widget="selection" to "many2one" default with options because it's not
  supported anymore in tree view
- Move value_id to parent view.
@pedrobaeza
Copy link
Member

Let's merge it then!

/ocabot merge

@OCA-git-bot
Copy link
Contributor

What a great day to merge this nice PR. Let's do it!
Prepared branch 12.0-ocabot-merge-pr-1721-by-pedrobaeza-bump-no, awaiting test results.

OCA-git-bot added a commit that referenced this pull request Dec 18, 2019
Signed-off-by pedrobaeza
@OCA-git-bot OCA-git-bot merged commit 153e1d0 into OCA:12.0 Dec 18, 2019
@OCA-git-bot
Copy link
Contributor

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

None yet