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

fix/enterpriseportal: ListEnterpriseSubscriptions fixes#63412

Merged
bobheadxi merged 1 commit into
mainfrom
ep-fix-ListEnterpriseSubscriptions
Jun 20, 2024
Merged

fix/enterpriseportal: ListEnterpriseSubscriptions fixes#63412
bobheadxi merged 1 commit into
mainfrom
ep-fix-ListEnterpriseSubscriptions

Conversation

@bobheadxi

@bobheadxi bobheadxi commented Jun 20, 2024

Copy link
Copy Markdown
Member

Follow-ups to https://github.com/sourcegraph/sourcegraph/pull/63173 I'm running into while working on CORE-169:

  1. Intersect permission-allowed subscriptions, instead of always overwriting. I think this is probably desired behaviour, to still allow filtering by subscriptions. If no subscriptions are explicitly listed, then the allowed subscriptions become the set of subscriptions to list.
    1. If a permissions filter is used and the resulting subscription set is empty, we fast-exit with empty result
  2. If no subscription list is provided, list all subscriptions. This is safe because permission filter has special fast-path above
  3. Fix "is archived" support

Test plan

Tests, and a manual check - with sg start dotcom, exchange for a token:

sg sams create-client-token \
        -sams https://accounts.sgdev.org \
        -s 'enterprise_portal::subscription::read' \
        -s 'enterprise_portal::codyaccess::read'

Query for subscriptions without filters:

curl --header "Content-Type: application/json" --header 'authorization: bearer sams_at_...' --data '{}' http://localhost:6081/enterpriseportal.subscriptions.v1.SubscriptionsService/ListEnterpriseSubscriptions | jq

All subscriptions I have locally get returned ✅

@cla-bot cla-bot Bot added the cla-signed label Jun 20, 2024
@bobheadxi bobheadxi requested review from a team and unknwon June 20, 2024 22:59
@bobheadxi bobheadxi merged commit 31da9c2 into main Jun 20, 2024
@bobheadxi bobheadxi deleted the ep-fix-ListEnterpriseSubscriptions branch June 20, 2024 23:35
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