fix: Bug where the repo count does not reset on a reingestion (#119)

Signed-off-by: Yoriyasu Yano <430092+yorinasub17@users.noreply.github.com>

fskconfig/loader_github.ts

@@ -345,7 +345,11 @@ function validateRepoLimits(
     maybeLimit = planRepoLimits[""];
   }
 
-  const totalRepoCount = configRepoCount + ghorg.subscription.repoCount;
+  let existingRepoCount = 0;
+  for (const k in ghorg.subscription.repoCount) {
+    existingRepoCount += ghorg.subscription.repoCount[k];
+  }
+  const totalRepoCount = configRepoCount + existingRepoCount;
   if (totalRepoCount > maybeLimit) {
     throw new FensakConfigLoaderUserError(
       `the config file for \`${ghorg.name}\` exceeds or causes the org to exceed the repo limit for the org (limit is ${maybeLimit})`,

fskconfig/loader_github_test.ts

@@ -24,7 +24,7 @@ Deno.test("fetchAndParseConfigFromDotFensak for fensak-test example repo", async
     id: "sub_asdf",
     mainOrgName: "fensak-test",
     planName: "pro",
-    repoCount: 0,
+    repoCount: {},
     cancelledAt: 0,
   };
   const testOrg: GitHubOrgWithSubscription = {
@@ -96,7 +96,9 @@ Deno.test("fetchAndParseConfigFromDotFensak checks repo limits", async () => {
       id: "sub_asdf",
       mainOrgName: "fensak-test",
       planName: "pro",
-      repoCount: 5,
+      repoCount: {
+        "yanosan": 5,
+      },
       cancelledAt: 0,
     },
   };

mgmt/subscription_events.ts

@@ -86,7 +86,7 @@ async function handleSubscriptionCreatedEvent(
     id: data.id,
     mainOrgName: data.mainOrgName,
     planName: data.planName,
-    repoCount: 0,
+    repoCount: {},
     cancelledAt: 0,
   };
   const stored = await storeSubscription(newSub, maybeSub);

svcdata/models.ts

@@ -17,16 +17,15 @@ export interface Lock {
  * @property mainOrgName The main organization that manages the subscription. Owners of this Org can manage the
  *                       subscription.
  * @property planName The name of the subscription plan.
- * @property repoCount A convenient counter of the number of active repos on the subscription. This is a sum across all
- *                     associated orgs.
+ * @property repoCount A convenient counter of the number of active repos for each Org in the subscription.
  * @property cancelledAt The timestamp (in milliseconds after epoch in UTC) when the subscription will be cancelled.
  *                       Used to record a future cancellation event for subscription management.
  */
 export interface Subscription {
   id: string;
   mainOrgName: string;
   planName: string;
-  repoCount: number;
+  repoCount: Record<string, number>;
   cancelledAt: number;
 }
 

svcdata/store.ts

@@ -167,7 +167,28 @@ export async function storeSubscription(
 export async function getSubscription(
   id: string,
 ): Promise<Deno.KvEntryMaybe<Subscription>> {
-  return await mainKV.get<Subscription>([TableNames.Subscription, id]);
+  const key = [TableNames.Subscription, id];
+  // deno-lint-ignore no-explicit-any
+  const obj = await mainKV.get<any>(key);
+
+  // TODO: figure out a better way to do this.
+  // Check and migrate to new repoCount field.
+  if (obj.value && Number.isInteger(obj.value.repoCount)) {
+    const updated = { ...obj.value };
+    updated.repoCount = {};
+    const ok = await mainKV.atomic()
+      .check(obj)
+      .set(key, updated)
+      .commit();
+    if (!ok) {
+      throw new Error(
+        `Transaction error while migrating subscription ${id} to new data format.`,
+      );
+    }
+  }
+
+  // At this point, we can be certain the object is in the right format.
+  return await mainKV.get<Subscription>(key);
 }
 
 /**
@@ -211,9 +232,27 @@ export async function deleteGitHubOrg(
   orgName: string,
   existingOrgRecord?: Deno.KvEntryMaybe<GitHubOrg>,
 ): Promise<void> {
-  let staged = mainKV.atomic();
-  if (existingOrgRecord) {
-    staged = staged.check(existingOrgRecord);
+  if (!existingOrgRecord) {
+    existingOrgRecord = await getGitHubOrgRecord(orgName);
+  }
+  if (!existingOrgRecord.value) {
+    // Do nothing since the org is already deleted.
+    return;
+  }
+
+  let staged = mainKV.atomic()
+    .check(existingOrgRecord);
+
+  // If there is a subscription associated, then we also need to null out the counter.
+  const maybeSubID = existingOrgRecord.value?.subscriptionID;
+  if (maybeSubID) {
+    const existingSubscription = await getSubscription(maybeSubID);
+    if (existingSubscription.value) {
+      const updatedSub = { ...existingSubscription.value };
+      delete updatedSub.repoCount[orgName];
+      staged = staged.check(existingSubscription)
+        .set([TableNames.Subscription, maybeSubID], updatedSub);
+    }
   }
 
   const ok = await staged
@@ -340,7 +379,8 @@ export async function storeComputedFensakConfig(
     );
   }
   const sub = { ...existingSub.value };
-  sub.repoCount += Object.keys(cfg.orgConfig.repos).length;
+  const orgRepoCount = Object.keys(cfg.orgConfig.repos).length;
+  sub.repoCount[orgName] = orgRepoCount;
   const { ok } = await mainKV.atomic()
     .check(existingSub)
     .set(existingSub.key, sub)