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-363: Fix connection docs #471

Merged
merged 4 commits into from
Feb 8, 2023
Merged

Conversation

sergiught
Copy link
Contributor

@sergiught sergiught commented Feb 8, 2023

🔧 Changes

This PR aims at fixing some small issues with the connection resource and data source docs.

However, the issue where computed properties don't have descriptions set on the data source docs, e.g. https://registry.terraform.io/providers/auth0/auth0/latest/docs/data-sources/client#nested-schema-for-addonssamlp is not caused by the schema.TransformResourceToDataSource helper func, but instead originates from hashicorp/terraform-plugin-docs#66.

📚 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)

@@ -29,8 +29,6 @@ Data source to retrieve a specific Auth0 connection by `connection_id` or `name`
- `realms` (List of String) Defines the realms for which the connection will be used (e.g., email domains). If not specified, the connection name is added as the realm.
- `show_as_button` (Boolean) Display connection as a button. Only available on enterprise connections.
- `strategy` (String) Type of the connection, which indicates the identity provider.
- `strategy_version` (String)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These 2 properties shouldn't have been neither in the data source or the resource connection docs. These are part of the current state upgraders only and are old remnants of past provider versions.

@@ -774,7 +776,7 @@ var resourceSchema = map[string]*schema.Schema{
}

func connectionSchemaV0() *schema.Resource {
s := resourceSchema
s := internalSchema.Clone(resourceSchema)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because the resourceSchema is a map (pointer), we were modifying the same current schema with the old schema version attributes causing the generated docs as well to display these properties that were removed in v2. We're making a clone of the resource schema instead and then safely modifying it.

@@ -48,3 +48,17 @@ func SetExistingAttributesAsOptional(schema map[string]*schema.Schema, keys ...s
schema[attribute].Required = false
}
}

// Clone returns a shallow clone of m.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

For our use case a shallow clone is perfectly fine as the attributes are top-level.

@codecov-commenter
Copy link

Codecov Report

Base: 87.16% // Head: 87.17% // Increases project coverage by +0.01% 🎉

Coverage data is based on head (7b14a34) compared to base (ac61dfe).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files
@@                             Coverage Diff                              @@
##           feature/DXCDT-363-connection-data-source     #471      +/-   ##
============================================================================
+ Coverage                                     87.16%   87.17%   +0.01%     
============================================================================
  Files                                            48       48              
  Lines                                          9387     9396       +9     
============================================================================
+ Hits                                           8182     8191       +9     
  Misses                                          921      921              
  Partials                                        284      284              
Impacted Files Coverage Δ
internal/auth0/connection/schema.go 77.27% <100.00%> (ø)
internal/schema/schema.go 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@sergiught sergiught marked this pull request as ready for review February 8, 2023 13:07
@sergiught sergiught requested a review from a team as a code owner February 8, 2023 13:07
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.

Good catch 👍

Base automatically changed from feature/DXCDT-363-connection-data-source to main February 8, 2023 17:53
@sergiught sergiught merged commit 758e4ba into main Feb 8, 2023
@sergiught sergiught deleted the patch/fix-docs-transform-schema branch February 8, 2023 18:00
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.

4 participants