Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

chore/enterpriseportal: rename customer admin role in API#63501

Merged
bobheadxi merged 1 commit into
mainfrom
ep-rename-codyanalytics-role
Jun 27, 2024
Merged

chore/enterpriseportal: rename customer admin role in API#63501
bobheadxi merged 1 commit into
mainfrom
ep-rename-codyanalytics-role

Conversation

@bobheadxi

@bobheadxi bobheadxi commented Jun 26, 2024

Copy link
Copy Markdown
Member

Studying Joe's work a bit more in depth I noticed that our API representation of this role ("Cody Analytics admin") does not line up with our internal representation ("customer admin").

Since we're already here, it's probably better to just align on "customer admin" as the role everywhere, and figure out more granular roles if we need it later.

Once it's rolled out and usages are migrated (https://github.com/sourcegraph/cody-analytics/pull/83), we can remove the deprecated enum entirely (https://github.com/sourcegraph/sourcegraph/pull/63502)

Test plan

CI

@bobheadxi bobheadxi requested a review from a team June 26, 2024 22:01
@cla-bot cla-bot Bot added the cla-signed label Jun 26, 2024
@bobheadxi bobheadxi force-pushed the ep-rename-codyanalytics-role branch from a9b2747 to ca6a30f Compare June 26, 2024 22:07

Copy link
Copy Markdown
Member Author

@bobheadxi bobheadxi force-pushed the ep-rename-codyanalytics-role branch from ca6a30f to 02ee4f3 Compare June 27, 2024 00:13
@bobheadxi bobheadxi merged commit ddb1d9a into main Jun 27, 2024
@bobheadxi bobheadxi deleted the ep-rename-codyanalytics-role branch June 27, 2024 17:12
bobheadxi referenced this pull request Jun 27, 2024
…thoritative (#63502)

Closes https://linear.app/sourcegraph/issue/CORE-199. AIP generally
implies `Update` RPCs are authoritative, which means that we should be
deleting all roles memberships not provided to
`UpdateEnterpriseSubscriptionMembership`. Most important outcome here is
that we can actually remove roles from users by assigning them an empty
role set `[]`

Later we can add a "get roles" RPC to safely make these updates, and
introduce a purely additive RPC if needed. It's not a huge deal right
now because we only have 1 role ("customer admin")

Also removes the deprecated value from
https://github.com/sourcegraph/sourcegraph/pull/63501.

## Test plan

Unit tests, expanded with better table-driven cases and expanded
assertions
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants