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

temporary fix: disable deletion of existing permissions #540

Merged
merged 4 commits into from
Oct 5, 2020

Conversation

bobheadxi
Copy link
Member

Pull Request

Description

Give a brief description of your changes:

Disables the deletion of existing permissions to resolve #510 temporarily. Longer-term fix is being tracked in the actual problem, #497

Testing

If testing this change requires extra setup, please document it here:

Ticket(s)

Closes #510

(Create a copy of that line for each Github Issue affected,
and replace "Affects" with "Closes" if merging this will close the relevant ticket.)

chuck-sys
chuck-sys previously approved these changes Oct 5, 2020
@codecov
Copy link

codecov bot commented Oct 5, 2020

Codecov Report

Merging #540 into master will increase coverage by 0.31%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #540      +/-   ##
==========================================
+ Coverage   93.11%   93.43%   +0.31%     
==========================================
  Files          47       47              
  Lines        2673     2665       -8     
  Branches      363      360       -3     
==========================================
+ Hits         2489     2490       +1     
+ Misses        129      119      -10     
- Partials       55       56       +1     
Impacted Files Coverage Δ
app/controller/command/commands/team.py 86.90% <ø> (+2.10%) ⬆️
interface/gcp.py 82.69% <66.66%> (-3.03%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ca02676...47d3b3a. Read the comment docs.

@bobheadxi bobheadxi merged commit 4394c76 into master Oct 5, 2020
@bobheadxi bobheadxi deleted the temp-fix/permissions branch October 5, 2020 07:30
bobheadxi added a commit that referenced this pull request Oct 17, 2020
Drive shares frequently get recreated because we fail to page through all results (we likely have over 100 shares for large folders), and when deletion was enabled (#540) we would also erroneously delete inherited permissions. Both of these issues result in many users receiving duplicate invites whenever Rocket performs a sync (#510, #497). To amend this, we make the following changes:

* Permissions listing is now paginated
* Permissions listing now happens for direct parents of target folders - permissions discovered in parents are treated as "inherited", and for these permissions we skip both recreation and deletion

Co-authored-by: Cheuk Yin Ng <[email protected]>
@bobheadxi bobheadxi linked an issue Oct 17, 2020 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

drive invites seem to be spammed for a few people drive: permission invites get re-sent on occasion
2 participants