-
Notifications
You must be signed in to change notification settings - Fork 949
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
Protect/unprotect #1273
base: master
Are you sure you want to change the base?
Protect/unprotect #1273
Conversation
+Worksheet.protect +Worksheet.unprotect
Protecting/Unprotecting Worksheet
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for this PR! good stuff :)
We will have a think about how to get the email. I'm not sure how all the auth works, but... is there always an email? For example if you are using a different type of authentication.
Further to-dos:
- add tests
- for protecting entire worksheet
- for unprotecting entire worksheet
- for protection kwarg domain_users_can_edit
- for
worksheet.list_protected_ranges()
@@ -1910,6 +1910,7 @@ def add_protected_range( | |||
description=None, | |||
warning_only=False, | |||
requesting_user_can_edit=False, | |||
domain_users_can_edit=False, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should be added to the Sphinx docs below (in function docstring)
@@ -2964,3 +2966,26 @@ def cut_range( | |||
} | |||
|
|||
return self.spreadsheet.batch_update(body) | |||
|
|||
def column_count(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I separated this out into #1274. It can be removed from here
def protect(self): | ||
"""Protect all ranges in current Worksheet""" | ||
email = get_email_from_somewhere_tbd() # TODO ? | ||
last_cell = rowcol_to_a1(self.row_count, self.col_count) | ||
self.add_protected_range( | ||
f"A1:{last_cell}", | ||
email, | ||
description="LOCKED by gspread user", | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did we try to call the API to add a new protected range but with no actual range. Something like, instead of giving: sheet1!A1:Z99
what if we give it: sheet1
as "range" does the API understand that we want to cover everything and then protects everything?? 🙃
I removed the milestone from this PR, we won't finish it on time for the next release. |
closes #1208
Worksheet.protect
andWorksheet.unprotect
. I've suggested keeping the names 'protect' and 'unprotect' instead of 'protect_all' and 'unprotect_all' because the scope is already clear ie. the whole Worksheet, and this avoids extra typing. The lack of arguments should also make it abundantly clear this doesn't protect/unprotect specific ranges.client.auth._service_account_email
to send toWorksheet.add_protected_range()
. This attribute seems to only be created on initialisation and I wasn't able to work out how to fetch it eg. fromauth
. One approach might be to add.email
attribute toSpreadsheet
when object is first initialised?domain_users_can_edit=False
as parameter toWorksheet.add_protected_range()
. Without it, a range is created but other (non system) users can still edit ie. not "View only" mode.Worksheet.list_protected_ranges
as a helper method to matchSpreadsheet.list_protected_ranges
and avoid people having to know or guess code for the "round trip" ie. up to the parent spreadsheet, then back down to the original worksheet using id.Worksheet.column_count
as alias for.col_count
. A pet peeve for many non-English speakers are abbreviations where an explicit (full English) variable name would avoid them having to guess.