Skip to content

Add Organisation and Project invitations #560

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 19 commits into from
Nov 13, 2021
Merged

Add Organisation and Project invitations #560

merged 19 commits into from
Nov 13, 2021

Conversation

beergeek
Copy link
Collaborator

@beergeek beergeek commented Sep 9, 2021 •
edited
Loading

Description

This update provides both resource and data source for invitations at the Organisation and Project levels.

Link to any related issue(s):

Type of change:

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Documentation fix/enhancement

Required Checklist:

  • I have signed the MongoDB CLA
  • I have read the Terraform contribution guidelines
  • I have added tests that prove my fix is effective or that my feature works per HashiCorp requirements
  • I have added any necessary documentation (if appropriate)
  • I have run make fmt and formatted my code

Further comments

gssbzn
gssbzn requested changes Sep 13, 2021
View reviewed changes
Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

Thanks for the contribution, I have made an initial pass of the PR mostly some small style recommendations, I mostly singled out a line but some of the recommendations apply to multiple cases

Also a this is a big blocker, it seems this PR is lacking some unit testing at least, once those are done we can take another look

@themantissa themantissa requested a review from gssbzn September 23, 2021 00:10
coderGo93
coderGo93 suggested changes Sep 23, 2021
View reviewed changes
Copy link
Contributor

@coderGo93 coderGo93 left a comment

Choose a reason for hiding this comment

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

It's looking good but I left some suggestions related to validation

gssbzn
gssbzn reviewed Sep 24, 2021
View reviewed changes
Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

Thanks for all the work here 💯 and thanks for adding those tests, I left a couple of comments on things that we could simplify also not reviewing docs, delegating that to @themantissa

gssbzn
gssbzn reviewed Sep 24, 2021
View reviewed changes
Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

almost there, some places were you were doing replace instead of join are still missing, also thanks for removing the dependency on log but the linter is reporting you still import it so we should fix that, other than that LGTM from me

@beergeek
Copy link
Collaborator Author

Thanks @gssbzn I ran out of time last night. I will get this done on Monday morning Oz time.

@beergeek
Copy link
Collaborator Author

Well I guess I found time when having breakfast :-)

@themantissa
Copy link
Collaborator

@beergeek I will review the docs soon. FYI this will not go in our next release (1.0.2) but in 1.1.0 since it's a major feature add. Let me know if any questions on this.

gssbzn
gssbzn approved these changes Sep 27, 2021
View reviewed changes
Copy link
Collaborator

@gssbzn gssbzn left a comment

Choose a reason for hiding this comment

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

LGTM thanks for all the work here 💯

@themantissa
Copy link
Collaborator

@coderGo93 I just re-requested your review as you had requested changes that @beergeek has made. Can you take a look and confirm and approve please?

coderGo93
coderGo93 suggested changes Sep 29, 2021
View reviewed changes
Copy link
Contributor

@coderGo93 coderGo93 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Left some comments related to schema of roles in datasource(s)

themantissa
themantissa requested changes Sep 29, 2021
View reviewed changes
Copy link
Collaborator

@themantissa themantissa left a comment

Choose a reason for hiding this comment

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

Mostly docs comments but one on the omission of teams for org. Bad copy/paste comments all a bit jokey but please do check we don't have any of these entries from db user ;) Thanks!

coderGo93
coderGo93 approved these changes Oct 7, 2021
View reviewed changes
Copy link
Contributor

@coderGo93 coderGo93 left a comment

Choose a reason for hiding this comment

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

LGTM if the tests passes, thanks for the changes.

themantissa
themantissa requested changes Oct 20, 2021
View reviewed changes
Copy link
Collaborator

@themantissa themantissa left a comment

Choose a reason for hiding this comment

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

Looking really good - just a few more nits in the docs. I'll also have one of our doc team members taking a look as they will be able to start helping us with the documentation PRs!

website/docs/d/org_invitation.html.markdown Outdated
## Argument Reference

* `org_id` - (Required) The unique ID for the project to create the database user.
* `username` - (Required) The Atlas user's email address.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I still think it wouldn't hurt to clarify this more but am happy to hear any reason why we should not do so.

website/docs/r/org_invitation.html.markdown Outdated
resource "mongodbatlas_org_invitation" "test" {
username = "test-acc-username"
org_id = "<ORG-ID>"
roles = [ "GROUP_DATA_ACCESS_READ_WRITE" ]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Still a project role in example.

@themantissa
Copy link
Collaborator

@jwilliams-mongo first review of a PR containing docs. If you have any questions let me know. Thanks!

jwilliams-mongo
jwilliams-mongo reviewed Oct 22, 2021
View reviewed changes
Copy link
Collaborator

@jwilliams-mongo jwilliams-mongo left a comment

Choose a reason for hiding this comment

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

some suggestions for clarity, for grammar, and to bring the documentation more in line with the Atlas API documentation.

themantissa
themantissa approved these changes Oct 29, 2021
View reviewed changes
Copy link
Collaborator

@themantissa themantissa left a comment

Choose a reason for hiding this comment

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

Thank you for all the work on this! LGTM

jwilliams-mongo
jwilliams-mongo approved these changes Oct 29, 2021
View reviewed changes
Copy link
Collaborator

@jwilliams-mongo jwilliams-mongo left a comment

Choose a reason for hiding this comment

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

LGTM % suggestions in two resolved comments that don't appear to have made it in to the PR.

@coderGo93 coderGo93 merged commit 60bc5e3 into mongodb:master Nov 13, 2021
abner-dou pushed a commit that referenced this pull request Nov 15, 2021
@beergeek @themantissa
@coderGo93 @abner-dou
Add Organisation and Project invitations (#560)
9f76280 
* Add Organisation and Project invitations

* Fix linting issues

* Change to American spelling

* Apply fixes and suggested modifications

* Fix the linting issues

* Fix the linting issues

* Add tests

* Refactor portions of the code to reduce the amount of code required

* Changes as requested

* Changes as requested

* Changes to code as requested, plus doc updates

* Fix conflict

* Fix context and spelling

* Fix spelling and grammar as advised

* Further spelling and grammatical fixes

* refactor: fixed many bugs that couldn't be tested locally

Co-authored-by: beergeek <[email protected]>
Co-authored-by: Melissa Plunkett <[email protected]>
Co-authored-by: Edgar López <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gssbzn gssbzn gssbzn approved these changes

@themantissa themantissa themantissa approved these changes

@coderGo93 coderGo93 coderGo93 approved these changes

@jwilliams-mongo jwilliams-mongo jwilliams-mongo approved these changes

@abner-dou abner-dou Awaiting requested review from abner-dou

@andreaangiolillo andreaangiolillo Awaiting requested review from andreaangiolillo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@beergeek @themantissa @gssbzn @coderGo93 @jwilliams-mongo