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

Master #327

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Master #327

wants to merge 2 commits into from

Conversation

comparecloud
Copy link

Per #53
Added functions to create GS contacts using the PeopleService API
Also modified the Contacts Get cmdlet adding the organization details (because I needed them)
A Contacts update cmdlet will come later

Chris O'Sullivan and others added 2 commits October 6, 2020 14:39
@scrthq
Copy link
Member

scrthq commented Nov 8, 2020

heya @comparecloud !!! Thank you so much for adding these in! Reviewing now and will get this merged in as soon as possible for the next release.

Copy link
Member

@scrthq scrthq left a comment

Choose a reason for hiding this comment

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

@comparecloud - Thanks for putting this together! Only feedback I would have are the following, everything else looks solid:

  1. It looks like the People API works in the context of the authenticated user and not at the organizational level, e.g. any contacts you create with New-GSContact would be added directly to the AdminEmail account only. If that's accurate, can you add the User parameter and pass that to New-GoogleService? An example here would be on Remove-GSDriveFile -- note the default value for $User is the AdminEmail, the parameter is not mandatory (e.g. we want to default to the AdminEmail, we use Resolve-Email to convert shortcuts like me into the full AdminEmail value or append the domain if only the email name part is passed, etc. We also would need that as the additional parameter passed to New-GoogleService.
  2. Check my comment on New-GSContact as it looks like an unset variable is used in the Verbose statement.

Once those are in, happy to get this merged! CC @FISHMANPET for visibility here.

$service = New-GoogleService @serviceParams
}
Process {
try {
Copy link
Member

Choose a reason for hiding this comment

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

Is $PrimaryEmail a remnant when testing? Not seeing it on the parameter list

@FISHMANPET
Copy link
Collaborator

Hey @comparecloud I realize it's been quite a while since you submitted this, but I'm pulling in old work to get ready for a new release. If you're still interested in this, could you look at the changes Nate requested?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants