fix/enterpriseportal: ListEnterpriseSubscriptions fixes (#63412)

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:

```sh
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:

```sh
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 ✅

Author: bobheadxi

cmd/enterprise-portal/internal/database/subscriptions.go

@@ -42,8 +42,9 @@ type ListEnterpriseSubscriptionsOptions struct {
 	IDs []string
 	// InstanceDomains is a list of instance domains to filter by.
 	InstanceDomains []string
-	// OnlyArchived indicates whether to only list archived subscriptions.
-	OnlyArchived bool
+	// IsArchived indicates whether to only list archived subscriptions, or only
+	// non-archived subscriptions.
+	IsArchived bool
 	// PageSize is the maximum number of subscriptions to return.
 	PageSize int
 }

cmd/enterprise-portal/internal/dotcomdb/dotcomdb.go

@@ -464,17 +464,30 @@ type SubscriptionAttributes struct {
 	ArchivedAt *time.Time
 }
 
+type ListEnterpriseSubscriptionsOptions struct {
+	SubscriptionIDs []string
+	IsArchived      bool
+}
+
 // ListEnterpriseSubscriptions returns a list of enterprise subscription
 // attributes with the given IDs. It silently ignores any non-existent
 // subscription IDs. The caller should check the length of the returned slice to
 // ensure all requested subscriptions were found.
-func (r *Reader) ListEnterpriseSubscriptions(ctx context.Context, subscriptionIDs ...string) ([]*SubscriptionAttributes, error) {
-	if len(subscriptionIDs) == 0 {
-		return []*SubscriptionAttributes{}, nil
+//
+// If no IDs are given, it returns all subscriptions.
+func (r *Reader) ListEnterpriseSubscriptions(ctx context.Context, opts ListEnterpriseSubscriptionsOptions) ([]*SubscriptionAttributes, error) {
+	query := `SELECT id, created_at, archived_at FROM product_subscriptions WHERE true`
+	namedArgs := pgx.NamedArgs{}
+	if len(opts.SubscriptionIDs) > 0 {
+		query += "\nAND id = ANY(@ids)"
+		namedArgs["ids"] = opts.SubscriptionIDs
+	}
+	if opts.IsArchived {
+		query += "\nAND archived_at IS NOT NULL"
+	} else {
+		query += "\nAND archived_at IS NULL"
 	}
 
-	query := `SELECT id, created_at, archived_at FROM product_subscriptions WHERE id = ANY(@ids)`
-	namedArgs := pgx.NamedArgs{"ids": subscriptionIDs}
 	rows, err := r.db.Query(ctx, query, namedArgs)
 	if err != nil {
 		return nil, errors.Wrap(err, "query subscription attributes")

cmd/enterprise-portal/internal/dotcomdb/dotcomdb_test.go

@@ -74,6 +74,7 @@ type mockedData struct {
 	targetSubscriptionID  string
 	accessTokens          []string
 	createdLicenses       int
+	createdSubscriptions  int
 	archivedSubscriptions int
 }
 
@@ -92,6 +93,7 @@ func setupDBAndInsertMockLicense(t *testing.T, dotcomdb database.DB, info licens
 		require.NoError(t, err)
 		sub, err := subscriptionsdb.Create(ctx, u.ID, u.Username)
 		require.NoError(t, err)
+		result.createdSubscriptions += 1
 		_, err = licensesdb.Create(ctx, sub, t.Name()+"-barbaz", 2, license.Info{
 			CreatedAt: info.CreatedAt,
 			ExpiresAt: info.ExpiresAt,
@@ -108,6 +110,7 @@ func setupDBAndInsertMockLicense(t *testing.T, dotcomdb database.DB, info licens
 		require.NoError(t, err)
 		sub, err := subscriptionsdb.Create(ctx, u.ID, u.Username)
 		require.NoError(t, err)
+		result.createdSubscriptions += 1
 		_, err = licensesdb.Create(ctx, sub, t.Name()+"-archived", 2, license.Info{
 			CreatedAt: info.CreatedAt,
 			ExpiresAt: info.ExpiresAt,
@@ -127,6 +130,7 @@ func setupDBAndInsertMockLicense(t *testing.T, dotcomdb database.DB, info licens
 		require.NoError(t, err)
 		sub, err := subscriptionsdb.Create(ctx, u.ID, u.Username)
 		require.NoError(t, err)
+		result.createdSubscriptions += 1
 		_, err = licensesdb.Create(ctx, sub, t.Name()+"-not-dev", 2, license.Info{
 			CreatedAt: info.CreatedAt,
 			ExpiresAt: info.ExpiresAt,
@@ -140,6 +144,7 @@ func setupDBAndInsertMockLicense(t *testing.T, dotcomdb database.DB, info licens
 	require.NoError(t, err)
 	subid, err := subscriptionsdb.Create(ctx, u.ID, u.Username)
 	require.NoError(t, err)
+	result.createdSubscriptions += 1
 	result.targetSubscriptionID = subid
 	// Insert a rubbish license first, CreatedAt is not used (creation time is
 	// inferred from insert time) so we need to do this first
@@ -171,6 +176,7 @@ func setupDBAndInsertMockLicense(t *testing.T, dotcomdb database.DB, info licens
 		require.NoError(t, err)
 		sub, err := subscriptionsdb.Create(ctx, u.ID, u.Username)
 		require.NoError(t, err)
+		result.createdSubscriptions += 1
 		_, err = licensesdb.Create(ctx, sub, t.Name()+"-foobar", 2, license.Info{
 			CreatedAt: info.CreatedAt,
 			ExpiresAt: info.ExpiresAt,
@@ -458,3 +464,28 @@ func TestListEnterpriseSubscriptionLicenses(t *testing.T) {
 		})
 	}
 }
+
+func TestListEnterpriseSubscriptions(t *testing.T) {
+	db, dotcomreader := newTestDotcomReader(t)
+	info := license.Info{
+		ExpiresAt: time.Now().Add(30 * time.Minute),
+		UserCount: 321,
+		Tags:      []string{licensing.PlanEnterprise1.Tag(), licensing.DevTag},
+	}
+	mock := setupDBAndInsertMockLicense(t, db, info, nil)
+
+	// Just a simple sanity test
+	ss, err := dotcomreader.ListEnterpriseSubscriptions(
+		context.Background(),
+		dotcomdb.ListEnterpriseSubscriptionsOptions{})
+	require.NoError(t, err)
+	assert.Len(t, ss, mock.createdSubscriptions-mock.archivedSubscriptions)
+	var found bool
+	for _, s := range ss {
+		if s.ID == mock.targetSubscriptionID {
+			found = true
+			break
+		}
+	}
+	assert.True(t, found)
+}

cmd/enterprise-portal/internal/subscriptionsservice/BUILD.bazel

@@ -17,6 +17,7 @@ go_library(
         "//cmd/enterprise-portal/internal/database",
         "//cmd/enterprise-portal/internal/dotcomdb",
         "//cmd/enterprise-portal/internal/samsm2m",
+        "//internal/collections",
         "//internal/trace",
         "//lib/enterpriseportal/subscriptions/v1:subscriptions",
         "//lib/enterpriseportal/subscriptions/v1/v1connect",

cmd/enterprise-portal/internal/subscriptionsservice/adapters.go

@@ -47,6 +47,12 @@ func convertLicenseAttrsToProto(attrs *dotcomdb.LicenseAttributes) *subscription
 }
 
 func convertSubscriptionToProto(subscription *database.Subscription, attrs *dotcomdb.SubscriptionAttributes) *subscriptionsv1.EnterpriseSubscription {
+	// Dotcom equivalent missing is surprising, but let's not panic just yet
+	if attrs == nil {
+		attrs = &dotcomdb.SubscriptionAttributes{
+			ID: subscription.ID,
+		}
+	}
 	conds := []*subscriptionsv1.EnterpriseSubscriptionCondition{
 		{
 			Status:             subscriptionsv1.EnterpriseSubscriptionCondition_STATUS_CREATED,