stytchauth / stytch-go

Official Stytch Backend SDK for Go
MIT License
47 stars 10 forks source link

`members.UpdateParams` omits empty Roles #177

Closed mlandry closed 5 months ago

mlandry commented 5 months ago

As a result, it is seemingly impossible to "clear" the roles on a member. You can't send in ["stych_member"] as setting an implicit role gives an error.

I think this is just a matter of removing the omitempty here: https://github.com/stytchauth/stytch-go/blob/3dcc5207b3c47c58584a3a5993f4aed7663b4201/stytch/b2b/organizations/members/types.go#L218

I figured I would open an issue to check in case this was an intentional choice.

logan-stytch commented 5 months ago

Hey @mlandry, thanks for the bug report. I see -- this is certainly annoying. I believe the issue with removing the omitempty would be that if someone didn't specify the roles for someone, it would clear them. As an example, if someone did this:

resp, err := stytchClient.Organizations.Members.Update(members.UpdateParams{
    OrganizationID: "organization-live-123",
    MemberID: "member-live-123",
    Name: "New Name",
})

Then I think it would unintentionally remove all existing roles from that member. Let me discuss with the team internally to see if we have an idea of how to fix this.

mlandry commented 5 months ago

Thanks @logan-stytch! That's a great point that I thought of as well after sending. Maybe something like *[]string would allow the tri-state (not set, empty slice, populated slice)?

logan-stytch commented 5 months ago

Yeah, I think that's what we're going to do. It'll technically be a MAJOR update since it's a breaking change, but it should fix things up. I can try to get a PR up for that today!

logan-stytch commented 5 months ago

Okay, pushed v14.0.0 which uses *[]string now and should make this possible!

mlandry commented 5 months ago

Awesome, thanks for the quick turnaround!

mlandry commented 5 months ago

Hello again! Would you mind bumping the version in your go.mod to v14 so I update our dependency and try out the fix?