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: set fetch from values before checking permissions #15457

Merged
merged 1 commit into from Dec 27, 2021

Conversation

ruchamahabal
Copy link
Member

Problem:

In document.py:

  • Fetch from values are set in _validate_links
  • Permissions are checked in check_permissions which is called before _validate_links

Use Case

In the Task doctype:

  • User permissions are applied on a field, for eg: Company field
  • Company is fetched from Project
  • Apply Strict User Permissions is checked in System Settings
  • User tries to create a Project from Project Template (which creates the tasks). But before setting the company (via fetch from), permissions are checked by the system and it raises "Insufficient Permissions for Task" since the company is not set.

Fix:

Permissions should be checked after:

  • Setting defaults (_set_defaults)
  • Setting fetch from values and validating links (_validate_links)
  • Setting user and timestamp (set_user_and_timestamp )

Co-authored-by: Suraj Shetty <surajshetty3416@gmail.com>
@codecov
Copy link

codecov bot commented Dec 27, 2021

Codecov Report

Merging #15457 (97d049c) into develop (bace5cd) will increase coverage by 0.21%.
The diff coverage is 100.00%.

@@             Coverage Diff             @@
##           develop   #15457      +/-   ##
===========================================
+ Coverage    51.24%   51.45%   +0.21%     
===========================================
  Files          744      744              
  Lines        65305    65305              
  Branches      5409     5409              
===========================================
+ Hits         33464    33602     +138     
+ Misses       28283    28145     -138     
  Partials      3558     3558              
Flag Coverage Δ
server 57.68% <100.00%> (+0.32%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

@mergify mergify bot merged commit 047466b into frappe:develop Dec 27, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants