Fixed APIError: [403] when copying permissions

[wobYY]

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:

[alifeee]

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 :)

[wobYY]

Hey @lavigne958 I would like to help with fixing this and pushing it along. Could you share what should be tested and to what degree?

[alifeee]

hi :]

shame @lavigne958 is busy

I think the error has to do with the typing of the return value of this function(?), so mypy complains that you cannot use .get( as it is expecting a string

gspread/gspread/spreadsheet.py

Lines 546 to 548 in 6775c81

	def list_permissions(self) -> List[Dict[str, Union[str, bool]]]:
	"""Lists the spreadsheet's permissions."""
	return self.client.list_permissions(self.id)

do you think it is possible to add a test that can fail/pass this bug, so we do not re-introduce it in future?

[wobYY]

I think the error has to do with the typing of the return value of this function(?), so mypy complains that you cannot use .get( as it is expecting a string

Yup, had to add an additional Typing definition - Found, fixed and tested. Linting isn't screaming anymore with an error.
https://github.com/wobYY/gspread/blob/52d0e1a7c327da82175426df9929c0ec72fe5375/gspread/spreadsheet.py#L546

do you think it is possible to add a test that can fail/pass this bug, so we do not re-introduce it in future?

I was thinking of adding something like below:
https://github.com/wobYY/gspread/blob/52d0e1a7c327da82175426df9929c0ec72fe5375/tests/client_test.py#L64-L84

This would test both copying of permissions and then on top of that, that being done in the Shared Drive. I'm testing on my company drive so my test service account doesn't have the permission to delete the file which is done at the end of the test. I'm awaiting my manager's approval on account getting a temporary full access so it can delete the files in that folder in the Shared Drive and successfully complete the test.

[alifeee]

hi ! sorry for the rushed comment, but... will you push the code onto this branch so we can test the CI ? thanks for this effort ! :]

I may return

[wobYY]

Hey, no worries at all :) My plan is to merge to the master on my fork after I create the test just so the PR doesn't get spammed with the linting Github Action. If you prefer the test to be part of a separate PR let me know and I can merge the code without the test implementation.