Migrate off deprecated trait profile/status attributes - #57
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| user.ID, | ||
| userTraitOptions, | ||
| sdkResource.WithResourceProfile(profile), | ||
| sdkResource.WithResourceStatus(v2.Status_RESOURCE_STATUS_ENABLED, ""), |
There was a problem hiding this comment.
🟡 Suggestion: The profile/status emission path changed shape here (trait-level → resource-level), but user_test.go only covers accountRole, so nothing asserts what userResource actually produces. Consider a small table test asserting sdkResource.GetProfile(res) contains user_role/user_id and sdkResource.GetStatus(res).GetStatus() == v2.Status_RESOURCE_STATUS_ENABLED — that pins the behavior the Grants path depends on. (confidence: high that coverage is absent; the migration itself reads correct)
| roleTraitOptions := []resource.RoleTraitOption{ | ||
| resource.WithRoleProfile(profile), | ||
| } | ||
| roleTraitOptions := []resource.RoleTraitOption{} |
There was a problem hiding this comment.
🟡 Suggestion: Now that the profile moved to WithResourceProfile, this slice is always empty — the local variable is dead scaffolding. Passing nil directly to resource.NewRoleResource(...) is clearer. Same applies to project.go:40 and team.go:43.
Connector PR Review: Migrate off deprecated trait profile/status attributesBlocking Issues: 0 | Suggestions: 1 | Threads Resolved: 0 Review SummaryScanned the full PR diff for security and correctness. This is a behavior-preserving migration of Security IssuesNone found. Correctness IssuesNone found. Suggestions
Prompt for AI agents |
ca27f66 to
a772c25
Compare
baton-sdk v0.20.6 moved `profile`, `status`, and `created_at` off the trait
messages onto attributes on `Resource`, deprecating the trait-level options and
getters. staticcheck flags every remaining call with `SA1019`, so `verify / lint`
is red on `main`.
This migrates the connector to the resource-level API:
- `With{User,Group,Role,App}Profile` -> `WithResourceProfile`
- `WithStatus` / `WithDetailedStatus` -> `WithResourceStatus`
- `WithCreatedAt` / `WithSecretCreatedAt` -> `WithResourceCreatedAt`
- trait `GetProfile()` / `GetStatus()` reads -> the equivalent read on the resource
The option type changes from a `*TraitOption` to a `ResourceOption`, so the calls
move out of the trait slice and into the variadic tail of the `New*Resource` call.
The two status enums are numerically identical, so the values map 1:1. Non-deprecated
trait data (login, aliases, emails, secret type/expiry) is untouched.
No behavioural change intended: the deprecated options already populated the
resource-level fields. `golangci-lint run ./...` reports 0 issues after this
change, and the package tests pass.
a772c25 to
37b45fc
Compare
| return nil, "", nil, fmt.Errorf("list-grants: Failed to get user trait from user: %w", err) | ||
| } | ||
| userProfile := userTrait.GetProfile() | ||
| userProfile := sdkResource.GetProfile(resource) |
There was a problem hiding this comment.
🟡 Suggestion: The read path for the role profile key moved from the user trait to the resource, which is the one place this refactor could silently regress (Grants would fail with "user role was not present on profile"). user_test.go only covers CreateAccount/Delete — consider a small test that builds userResource(...) and asserts Grants still emits the role membership grant, so the write/read pairing is locked in.
1 participant
baton-sdk v0.20.6 moved
profile,status, andcreated_atoff the traitmessages onto attributes on
Resource, deprecating the trait-level options andgetters. staticcheck flags every remaining call with
SA1019, soverify / lintis red on
main.This migrates the connector to the resource-level API:
With{User,Group,Role,App}Profile->WithResourceProfileWithStatus/WithDetailedStatus->WithResourceStatusWithCreatedAt/WithSecretCreatedAt->WithResourceCreatedAtGetProfile()/GetStatus()reads -> the equivalent read on the resourceThe option type changes from a
*TraitOptionto aResourceOption, so the callsmove out of the trait slice and into the variadic tail of the
New*Resourcecall.The two status enums are numerically identical, so the values map 1:1. Non-deprecated
trait data (login, aliases, emails, secret type/expiry) is untouched.
No behavioural change intended: the deprecated options already populated the
resource-level fields.
golangci-lint run ./...reports 0 issues after thischange, and the package tests pass.