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

DXCDT-434: Add new auth0_user_roles resource #579

Merged
merged 1 commit into from
May 12, 2023

Conversation

sergiught
Copy link
Contributor

🔧 Changes

In this PR we are adding a new auth0_user_roles resource (1:many) to manage the assigned roles on a user.

📚 References

🔬 Testing

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)


### User Roles

The `roles` field on the `auth0_user` resource will continue to be available for managing user roles. However, to ensure
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Adding a migration guide as we are deprecating the roles field on the user resource.

@@ -470,7 +473,7 @@ func validateNoPasswordAndEmailVerifiedSimultaneously() validateUserFunc {
}
}

func updateUserRoles(d *schema.ResourceData, api *management.Management) error {
func persistUserRoles(d *schema.ResourceData, api *management.Management) error {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was conflict with updateUserRoles func for the user_roles resource, so I just renamed it. However this logic will likely go inside the auth0_user_roles file after it is removed from here.

@sergiught sergiught force-pushed the feature/DXCDT-434-user-roles branch from 250caf5 to 80bf9cc Compare May 12, 2023 09:34
@codecov-commenter
Copy link

codecov-commenter commented May 12, 2023

Codecov Report

Merging #579 (7bdb93a) into main (577845c) will decrease coverage by 0.10%.
The diff coverage is 72.22%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #579      +/-   ##
==========================================
- Coverage   87.71%   87.62%   -0.10%     
==========================================
  Files          65       66       +1     
  Lines       10114    10183      +69     
==========================================
+ Hits         8872     8923      +51     
- Misses        936      950      +14     
- Partials      306      310       +4     
Impacted Files Coverage Δ
internal/provider/provider.go 100.00% <ø> (ø)
internal/auth0/user/resource.go 73.44% <33.33%> (ø)
internal/auth0/user/resource_roles.go 73.91% <73.91%> (ø)

@sergiught sergiught marked this pull request as ready for review May 12, 2023 09:42
@sergiught sergiught requested a review from a team as a code owner May 12, 2023 09:42
Copy link
Contributor

@willvedd willvedd left a comment

Choose a reason for hiding this comment

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

Overall good. Just a couple of small ideas.

internal/auth0/user/resource.go Outdated Show resolved Hide resolved
internal/auth0/user/resource.go Outdated Show resolved Hide resolved
internal/auth0/user/resource_roles_test.go Show resolved Hide resolved
internal/auth0/user/resource_roles_test.go Show resolved Hide resolved
Comment on lines +8 to +12
### Deprecations

- [User Roles](#user-roles)

### User Roles
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming this is the only notable migration component in this h2, we don't need to establish such a rigid hierarchy.

Suggested change
### Deprecations
- [User Roles](#user-roles)
### User Roles
### User Roles Deprecation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd keep it like this for now and change it if we release the next version without the role_permission deprecation. Wdyt? Otherwise that will have it's own h3 element as well.

@sergiught sergiught force-pushed the feature/DXCDT-434-user-roles branch from 80bf9cc to 7bdb93a Compare May 12, 2023 15:34
@sergiught sergiught requested a review from willvedd May 12, 2023 15:37
@sergiught sergiught merged commit 8ca33e0 into main May 12, 2023
@sergiught sergiught deleted the feature/DXCDT-434-user-roles branch May 12, 2023 16:13
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