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

[FIX] models: using unknown fields makes CRUD methods crash #37094

Closed
wants to merge 3 commits into from

Conversation

rco-odoo
Copy link
Member

The basic ORM methods should not silently discard unknown fields, as
they may be a sign of broken code.

@rco-odoo
Copy link
Member Author

@julienlegros @odony

@C3POdoo C3POdoo added the RD research & development, internal work label Sep 18, 2019
@rco-odoo rco-odoo changed the base branch from saas-12.5 to 13.0 October 11, 2019 09:55
@rco-odoo rco-odoo force-pushed the saas-12.5-unknown-fields-rco branch 2 times, most recently from d1004cc to 69a2a8b Compare October 14, 2019 08:28
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Oct 14, 2019
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Oct 21, 2019
The basic ORM methods should not silently discard unknown fields, as
they may be a sign of broken code.
@robodoo robodoo added CI 🤖 Robodoo has seen passing statuses and removed CI 🤖 Robodoo has seen passing statuses labels Oct 22, 2019
@@ -100,7 +99,6 @@ def _check_holidays_status(holiday_status, ml, lt, rl, vrl):
HolidayStatusManagerGroup.create({
'name': 'WithMeetingType',
'allocation_type': 'no',
'categ_id': self.env['calendar.event.type'].with_user(self.user_hrmanager_id).create({'name': 'NotLimitedMeetingType'}).id,
Copy link
Contributor

Choose a reason for hiding this comment

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

categ_id was removed by 22cd306 of #33233

Copy link
Contributor

@odony odony left a comment

Choose a reason for hiding this comment

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

Cool, this will make the behavior uniform for all cases, and definitely catch mistakes and typos earlier. Not much of a semantic change for 13.0 either, as the refactored ORM already blocked read() on non-existant fields.

@robodoo rebase-ff r+

@robodoo
Copy link
Contributor

robodoo commented Oct 22, 2019

Merge method set to rebase and fast-forward

@robodoo
Copy link
Contributor

robodoo commented Oct 22, 2019

Merged at 9492b01, thanks!

@rco-odoo rco-odoo deleted the saas-12.5-unknown-fields-rco branch October 22, 2019 15:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI 🤖 Robodoo has seen passing statuses RD research & development, internal work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants