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

Allow "warning" type protected ranges #1439

Merged
merged 6 commits into from
Mar 17, 2024
Merged

Conversation

alifeee
Copy link
Collaborator

@alifeee alifeee commented Mar 15, 2024

closes #1437

They were not allowed before because:

It seems that when add_protected_range was added it was assumed that editor_users_emails was always required. However, warning ranges do not require (and in fact, cannot contain) emails.

To fix this issue, we:

  • change editor_users_emails from and argument to a keyword argument (if it is not changed in position, it should still be usable as an argument)
  • add some logic in the code to not pass "editors" if warning_only is set

Copy link
Collaborator

@lavigne958 lavigne958 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great change ! Oddly enough the API allows a user to set a protected range with no editors (when providing an actual empty list of editors). 🤔

@alifeee
Copy link
Collaborator Author

alifeee commented Mar 16, 2024

yes I wasn't sure what email to put in the test, since it would be different for whoever ran the test.

maybe the email is fetchable from an API somewhere?

@lavigne958
Copy link
Collaborator

yes I wasn't sure what email to put in the test, since it would be different for whoever ran the test.

maybe the email is fetchable from an API somewhere?

I don't think it's a problem. Leaving it like this endures it works with no emails too 🙄

@alifeee alifeee merged commit 3e5ca62 into master Mar 17, 2024
10 checks passed
@alifeee alifeee deleted the fix/protected-range-warnings branch March 17, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

worksheet.add_protected_range() - impossible to create the warning_only=True range
2 participants