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

Fixed APIError: [403] when copying permissions #1515

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

wobYY
Copy link

@wobYY wobYY commented Sep 24, 2024

This PR fixes the bug raised in the issue #1507.

tox test (replaced the first part of the path with local_dir to remove personal information):

>> tox -e py
.pkg: _optional_hooks> python local_dir\gspread\.venv\Lib\site-packages\pyproject_api\_backend.py True flit_core.buildapi
.pkg: get_requires_for_build_sdist> python local_dir\gspread\.venv\Lib\site-packages\pyproject_api\_backend.py True flit_core.buildapi
.pkg: build_sdist> python local_dir\gspread\.venv\Lib\site-packages\pyproject_api\_backend.py True flit_core.buildapi
py: install_package> python -I -m pip install --force-reinstall --no-deps local_dir\gspread\.tox\.tmp\package\5\gspread-6.1.2.tar.gz
py: commands[0]> pytest tests/
======================================================================= test session starts ========================================================================
platform win32 -- Python 3.12.4, pytest-8.3.3, pluggy-1.5.0
cachedir: .tox\py\.pytest_cache
rootdir: local_dir\gspread
configfile: pyproject.toml
plugins: vcr-1.0.2
collected 141 items

tests\cell_test.py .......                                                                                                                                    [  4%]
tests\client_test.py .............                                                                                                                            [ 14%]
tests\spreadsheet_test.py .................                                                                                                                   [ 26%]
tests\utils_test.py ........................                                                                                                                  [ 43%]
tests\worksheet_test.py ................................................................................                                                      [100%]

========================================================================= warnings summary ========================================================================= 
tests\worksheet_test.py:39
  local_dir\gspread\tests\worksheet_test.py:39: PytestRemovedIn9Warning: Marks applied to fixtures have no effect
  See docs: https://docs.pytest.org/en/stable/deprecations.html#applying-a-mark-to-a-fixture-function
    @pytest.fixture(autouse=True)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================================================== 141 passed, 1 warning in 3.87s ================================================================== 
.pkg: _exit> python local_dir\gspread\.venv\Lib\site-packages\pyproject_api\_backend.py True flit_core.buildapi
  py: OK (9.83=setup[4.83]+cmd[5.00] seconds)
  congratulations :) (9.97 seconds)

Also tested by copying a random spreadsheet in my workspace. For debugging I added a print in the if block:

                # .list_permissions() returns a list of permissions,
                # even the folder permissions if the file is in a shared folder.
                # We only want the permissions that are directly applied to the
                # spreadsheet file, i.e. 'writer', 'commenter' and 'reader'.
                perm_details = {
                    p_details.get("permissionType"): p_details.get("inherited")
                    for p_details in p.get("permissionDetails")
                }
                if p.get("role") in ("organizer", "fileOrganizer") and (
                    perm_details.get("file") or perm_details.get("member")
                ):
                    print(
                        f"`{p.get("id")[:3] + "..."}` has the `{p.get("role")}` role which is a folder permission, skipping..."
                    )
                    continue

                print(f"`{p.get("id")[:3] + "..."}` has the `{p.get("role")}` role, adding permission")

The result (screenshot):
011... has the organizer role which is a folder permission, skipping...
027... has the fileOrganizer role which is a folder permission, skipping...
029... has the commenter role, adding permission
033... has the fileOrganizer role which is a folder permission, skipping...
061... has the organizer role which is a folder permission, skipping...
083... has the fileOrganizer role which is a folder permission, skipping...
085... has the fileOrganizer role which is a folder permission, skipping...
087... has the organizer role which is a folder permission, skipping...
131... has the writer role, adding permission
134... has the fileOrganizer role which is a folder permission, skipping...
146... has the fileOrganizer role which is a folder permission, skipping...
152... has the organizer role which is a folder permission, skipping...

Correctly shared:
image

@alifeee
Copy link
Collaborator

alifeee commented Sep 24, 2024

hi ! thanks for the change :)

I have fixed a couple of linting issues. There is a typing issue that mypy does not like, that confuses me a little. @lavigne958 will be able to look at it

otherwise, I think your change makes sense. we will try to get the workflows fixed and then think about merging :)

@lavigne958 lavigne958 added the in progress Issue currently in progress by the assignee label Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in progress Issue currently in progress by the assignee
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants