feat: registry_admin_only_crud opt-out per action (#298)#302
feat: registry_admin_only_crud opt-out per action (#298)#302Tarekchehahde wants to merge 1 commit into
Conversation
OpenSPP#298) Honor bypass_registry_admin_only_crud on res.partner actions so one view can allow non-admin create/edit while the global setting stays on. Co-authored-by: Cursor <cursoragent@cursor.com>
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a bypassRegistryRestriction helper function to bypass registry restrictions on forms and lists when bypass_registry_admin_only_crud is enabled in the component's context. The review feedback highlights several opportunities to simplify the code by removing redundant !bypassRegistryRestriction(this) checks, as _registryRestricted is already falsy when bypassed. Additionally, it is recommended to combine two sequential if statements in the ListController setup into a single condition.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| if ( | ||
| this._registryRestricted && | ||
| !bypassRegistryRestriction(this) | ||
| ) { | ||
| params.config.mode = "readonly"; | ||
| } |
There was a problem hiding this comment.
Since this._registryRestricted is already set to false when the restriction is bypassed, we don't need to check !bypassRegistryRestriction(this) here. We can revert this to the simpler original check.
if (this._registryRestricted) {
params.config.mode = "readonly";
}| if ( | ||
| this._registryRestricted && | ||
| REGISTRY_MODELS.includes(this.props.resModel) && | ||
| !bypassRegistryRestriction(this) | ||
| ) { |
There was a problem hiding this comment.
Since this._registryRestricted is already set to false when the restriction is bypassed, we don't need to check !bypassRegistryRestriction(this) here. We can revert this to the simpler original check.
if (this._registryRestricted && REGISTRY_MODELS.includes(this.props.resModel)) {| if (!REGISTRY_MODELS.includes(modelName)) { | ||
| return; | ||
| } | ||
| if (bypassRegistryRestriction(this)) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
We can combine these two if statements into a single, cleaner condition.
| if (!REGISTRY_MODELS.includes(modelName)) { | |
| return; | |
| } | |
| if (bypassRegistryRestriction(this)) { | |
| return; | |
| } | |
| if (!REGISTRY_MODELS.includes(modelName) || bypassRegistryRestriction(this)) { | |
| return; | |
| } |
| if ( | ||
| this._registryRestricted && | ||
| REGISTRY_MODELS.includes(this.props.resModel) && | ||
| !bypassRegistryRestriction(this) | ||
| ) { |
There was a problem hiding this comment.
Since we return early in setup() when bypassRegistryRestriction(this) is true, this._registryRestricted will remain undefined (falsy). Therefore, we don't need to check !bypassRegistryRestriction(this) here. We can revert this to the simpler original check.
if (this._registryRestricted && REGISTRY_MODELS.includes(this.props.resModel)) {1 participant
Summary
bypass_registry_admin_only_crud: True.res.partner.Test plan
spp_starter.registry_admin_only_crudenabled and non-admin user: default partner views remain read-onlycontext={'bypass_registry_admin_only_crud': True}: New/save/edit available on that action onlyFixes #298
Made with Cursor