8000 [18.0][FIX] web_m2x_options: prevent crash on Many2OneReferenceField by sbejaoui · Pull Request #3191 · OCA/web · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

[18.0][FIX] web_m2x_options: prevent crash on Many2OneReferenceField #3191

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
Merged
merged 1 commit into from
Jun 30, 2025
Merged

[18.0][FIX] web_m2x_options: prevent crash on Many2OneReferenceField #3191

merged 1 commit into from
Jun 30, 2025
+54 −29

Conversation

sbejaoui
Copy link
Contributor
@sbejaoui sbejaoui commented Jun 9, 2025

Many2OneReferenceField inherits the props definition from Many2OneField but does not inherit the prototype methods added via patch on many2OneField, including m2o_options_props

without this patch, calling extractProps on Many2OneReferenceField leads to "this.m2o_options_props is not a function" errors because this method is undefined

this fix adds a no-op m2o_options_props implementation on Many2OneReferenceField to ensure compatibility and safely bypass the m2o_options_props logic since it's not relevant for reference fields

to reproduce this bug, open any view with many2one reference field, example attachment form view

image

@sbejaoui
Copy link
Contributor Author
sbejaoui commented Jun 9, 2025

cc/ @glitchov ,

@pedrobaeza pedrobaeza changed the title [FIX] web_m2x_options: prevent crash on Many2OneReferenceField [18.0][FIX] web_m2x_options: prevent crash on Many2OneReferenceField Jun 9, 2025
@sbejaoui sbejaoui force-pushed the 18.0-web_m2x_options-sbj branch from 4cb2dc9 to 868f851 Compare June 10, 2025 08:47
carlos-lopez-tecnativa
carlos-lopez-tecnativa suggested changes Jun 13, 2025
View reviewed changes
Copy link
Contributor
@carlos-lopez-tecnativa carlos-lopez-tecnativa left a comment

Choose a reason for hiding this comment

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

These changes prevent the error, but the Many2OneReferenceField is not working with the options.
Please check and add support to make it work correctly.

image

@sbejaoui
Copy link
Contributor Author

These changes prevent the error, but the Many2OneReferenceField is not working with the options. Please check and add support to make it work correctly.

I can't work on this right now. if someone else is available to take it over, I'd appreciate it
if not, I propose we merge it as-is with a FIXME comment in the code and document it as a known issue — otherwise, the view doesn't open at all in its current state

sbiosca-s73
sbiosca-s73 approved these changes Jun 19, 2025
View reviewed changes
Copy link
@sbiosca-s73 sbiosca-s73 left a comment

Choose a reason for hiding this comment

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

LGTM

@Botakely
Copy link

LGTM

@StefanRijnhart
Copy link
Member

@sbejaoui Please add the roadmap entry so this can be merged quickly, given the impact of the bug.

@sbejaoui
[FIX] web_m2x_options: prevent crash on Many2OneReferenceField
5d2726c 
Many2OneReferenceField inherits the props definition from Many2OneField but
does not inherit the prototype methods added via patch() on many2OneField,
including m2o_options_props()

without this patch, calling extractProps() on Many2OneReferenceField leads to
"this.m2o_options_props is not a function" errors because this method is undefined

this fix adds a no-op m2o_options_props() implementation on Many2OneReferenceField
to ensure compatibility and safely bypass the m2o_options_props logic
since it's not relevant for reference fields
@sbejaoui sbejaoui force-pushed the 18.0-web_m2x_options-sbj branch from 868f851 to 5d2726c Compare June 30, 2025 14:13
@sbejaoui
Copy link
Contributor Author

@sbejaoui Please add the roadmap entry so this can be merged quickly, given the impact of the bug.

done

StefanRijnhart
StefanRijnhart approved these changes Jun 30, 2025
View reviewed changes
Copy link
Member
@StefanRijnhart StefanRijnhart left a comment

Choose a reason for hiding this comment

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

@carlos-lopez-tecnativa can you live with this for now?

@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). 🤖

@StefanRijnhart
Copy link
Member

/ocabot merge patch

@OCA-git-bot
Copy link
8000
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 18.0-ocabot-merge-pr-3191-by-StefanRijnhart-bump-patch, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit 2148cb7 into OCA:18.0 Jun 30, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@StefanRijnhart StefanRijnhart StefanRijnhart approved these changes

@CasVissers-360ERP CasVissers-360ERP CasVissers-360ERP approved these changes

@sbiosca-s73 sbiosca-s73 sbiosca-s73 approved these changes

@carlos-lopez-tecnativa carlos-lopez-tecnativa carlos-lopez-tecnativa approved these changes

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

Successfully merging this pull request may close these issues.
7 participants
@sbejaoui @Botakely @StefanRijnhart @OCA-git-bot @CasVissers-360ERP @sbiosca-s73 @carlos-lopez-tecnativa
0