gspread icon indicating copy to clipboard operation
gspread copied to clipboard

Regression on API V3 Migration (#1060)

Open crimoniv opened this issue 3 years ago • 1 comments

Describe the bug Since our last gspread upgrade (from 5.4.0 to 5.5.0), we started experiencing file sharing errors when using perm_type='domain'.

To Reproduce Steps to reproduce the behavior:

  1. Create a new spreadsheet
  2. Share the spreadsheet using perm_type='domain'
  3. An exception is raised. It reads as follows:
gspread.exceptions.APIError: {'code': 400, 'message': "The domain field is required for permissions of type 'domain'.", 'errors': [{'message': "The domain field is required for permissions of type 'domain'.", 'domain': 'global', 'reason': 'required', 'location': 'permission.domain', 'locationType': 'other'}]}

Expected behavior As in gspread==5.4.0, we expect the spreadsheet to be successfully shared with no exception raised.

Code example

client = gspread.authorize(credentials)
sh = client.create(title)
sh.share('domain.es', perm_type='domain', role='writer')  # <- raises gspread.exceptions.APIError

Environment info:

  • Operating System [e.g. Linux, Windows, macOS]: Debian GNU/Linux 10 (buster)
  • Python version: 3.8.13 (Docker image python:3.8-slim-buster)
  • gspread version: 5.5.0

Stack trace or other output that would be helpful

Traceback (most recent call last):
  File "/app/src/spreadsheet.py", line 38, in get_or_create
    spreadsheet.share("domain.com", perm_type="domain", role="writer")
  File "/usr/local/lib/python3.8/site-packages/gspread/spreadsheet.py", line 512, in share
    self.client.insert_permission(
  File "/usr/local/lib/python3.8/site-packages/gspread/client.py", line 508, in insert_permission
    return self.request("post", url, json=payload, params=params)
  File "/usr/local/lib/python3.8/site-packages/gspread/client.py", line 92, in request
    raise APIError(response)
gspread.exceptions.APIError: {'code': 400, 'message': "The domain field is required for permissions of type 'domain'.", 'errors': [{'message': "The domain field is required for permissions of type 'domain'.", 'domain': 'global', 'reason': 'required', 'location': 'permission.domain', 'locationType': 'other'}]}

Additional context

As per API V3 documentation, the domain property is expected to be provided when granting permissions with type=domain. In the V2 to V3 migration, the value property was renamed to emailAddress, which only applies to user and group types.

crimoniv avatar Sep 12 '22 07:09 crimoniv

I'm experiencing this too. There aren't any tests for domains I assume.

I started a PR #1115 as an attempt at a fix.

Note that it's not just a problem as @crimoniv describes above. In addition, the "sendNotificationEmail" and "emailMessage" params are only relevant for users and groups, so they can't be present for domains.

NickCrews avatar Sep 23 '22 00:09 NickCrews