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

Unable to disable notification emails when adding drive permissions #103

Closed
cdwcriver opened this issue Oct 21, 2018 · 11 comments
Closed

Unable to disable notification emails when adding drive permissions #103

cdwcriver opened this issue Oct 21, 2018 · 11 comments
Assignees

Comments

@cdwcriver
Copy link

cdwcriver commented Oct 21, 2018

The -SendNotificationEmail parameter for the Add-GSDrivePermission permission is a switch parameter. However, this value defaults to true when adding permissions to users and groups, and adding the parameter does not switch this value to false. So, it doesn't seem to be possible to add permissions without sending a notification email.

Steps to reproduce the behavior:

Import-Csv ./testupdateperms.csv | % { Add-GSDrivePermission -User $_.user -FileId $_.fileid -Role $_.Role -Type "group" -EmailAddress $_.NewGroupAddress  }

Expected behavior
Notification email should not be sent for the added permission.

  • OS: Ubuntu
  • PowerShell Version: Core 6.0.2
  • PSGSuite Version: 2.15.1
@n-searle
Copy link

Hey,

I had similar behaviours with the -confirm switch which also defaults true, have you tried:
Import-Csv ./testupdateperms.csv | % { Add-GSDrivePermission -User $.user -FileId $.fileid -Role $.Role -Type "group" -EmailAddress $.NewGroupAddress -SendNotificationEmail:$false}

Cheers

@scrthq scrthq self-assigned this Oct 21, 2018
@scrthq
Copy link
Member

scrthq commented Oct 21, 2018

Hey @aitcriver - The default for the standard API is $true: https://developers.google.com/drive/api/v3/reference/permissions/create

It may be worth adding an additional parameter that's more explicit like -DisableNotificationEmail, but I try to at least have the same parameters across the various API's as a minimum unless more are needed.

For now, you can disable email notification as @n-searle mentioned by including -SendNotificationEmail:$false with your call to Add-GSDrivePermission.

@cdwcriver
Copy link
Author

Ah, yes, that is helpful, I wasn't exactly sure how to force that parameter to false. Right now, the behavior is that the -SendNotificationEmail parameter does nothing; it is true if the parameter is added of course, but it is also true when it is not added. The only way this parameter does anything is the not-obvious workaround posted here.

A DisableNotificationEmail parameter could work, but I think it would be better to just default to no notification for all cases and then require the -SendNotificationEmail if notifications are desired. Or, change it from a switch parameter to one that accepts a $true/$false value.

I typically use GAM so that's my frame of reference; GAM defaults to no notification emails unless you specify to send one.

@scrthq
Copy link
Member

scrthq commented Oct 22, 2018

Alright, GitHub is currently going through a data outage, but I have this in a PR ready. Specifically, I updated SendNotificationEmail parameter to default to $false unless specifically called and set to $true with -SendNotificationEmail OR if the permission action is a transfer of ownership (this is a hard requirement by Google for ownership transfers of Drive files). I also updated the doc for the SendNotificationEmail parameter to be more clear

Should hopefully have this out by tomorrow!

@cdwcriver
Copy link
Author

Sounds perfect! That was awfully quick, thank you!

scrthq added a commit that referenced this issue Oct 23, 2018
…103

## 2.17.0

* [Issue #102](#102)
  * Fixed: `$EncryptionKey` PSM1 parameter now stores the AES key correctly so SecureStrings are encrypted/decrypted as intended.
* [Issue #103](#103)
  * Updated: `SendNotificationEmail` parameter on `Add-GSDrivePermission` defaults to false for all User & Group permissions that are not ownership transfers.
  * Updated: Documentation for `SendNotificationEmail` parameter on `Add-GSDrivePermission` for clarity towards default Google API parameter values.
* Added: More unit testing for `Get-GSUser`
* Updated: `psake` build script
@scrthq
Copy link
Member

scrthq commented Oct 23, 2018

hey @aitcriver - v2.17.1 is out now with that update! let me know how it works for you =]

@cdwcriver
Copy link
Author

Thanks! I've verified that it doesn't send the notification email by default now. However, I don't seem to be able to set it to send a notification email. I get an "Add-GSDrivePermission : The property 'SendNotificationEmail' cannot be found on this object. Verify that the property exists and can be set." error:

Compare-ModuleVersion

ModuleName InstalledVersion GalleryVersion UpdateAvailable
---------- ---------------- -------------- ---------------
PSGSuite   2.17.1           2.17.1                   False


Add-GSDrivePermission -User source@example.com -FileId DriveFileId -Role writer -Type user -EmailAddress destination@example.com -SendNotificationEmail
Add-GSDrivePermission : The property 'SendNotificationEmail' cannot be found on this object. Verify that the property exists and can be set.
At line:1 char:1
+ Add-GSDrivePermission -User source@example.com -FileId DriveFile ...
+ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ CategoryInfo          : NotSpecified: (:) [Write-Error], WriteErrorException
+ FullyQualifiedErrorId : Microsoft.PowerShell.Commands.WriteErrorException,Add-GSDrivePermission

@scrthq
Copy link
Member

scrthq commented Oct 24, 2018 via email

scrthq added a commit that referenced this issue Oct 25, 2018
scrthq added a commit that referenced this issue Oct 25, 2018
scrthq added a commit that referenced this issue Oct 25, 2018
scrthq added a commit that referenced this issue Oct 25, 2018
## 2.17.2

* [Issue #103](#103)
  * Fixed: `SendNotificationEmail` is now correctly defaulting to `$false`, but attempting to actually send the notification email results in an error. This is now corrected.
@scrthq
Copy link
Member

scrthq commented Oct 25, 2018

@aitcriver - alright, update to 2.17.2 when you can and let me know if you're solid now!

@cdwcriver
Copy link
Author

I've updated, and it looks good now! I don't receive the notification when the -SendNotificationEmail parameter isn't included, and the notification is sent when it is included. Thanks!

@scrthq
Copy link
Member

scrthq commented Oct 25, 2018

Awesome! Thanks for validating!

Sent with GitHawk

@scrthq scrthq closed this as completed Oct 25, 2018
@ghost ghost removed the pending UAT label Oct 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants