fix: address code reviews

Author: sandsinh

packages/spacecat-shared-data-access/src/models/suggestion-grant/suggestion-grant.collection.js

@@ -17,8 +17,9 @@ import DataAccessError from '../../errors/data-access.error.js';
 
 /**
  * SuggestionGrantCollection - Manages SuggestionGrant records (suggestion_grants table).
- * Table is insert-only; inserts happen via grant_suggestions RPC. This collection
- * provides read-only lookup by suggestion IDs.
+ * Inserts happen via the grant_suggestions RPC and deletes via the revoke_suggestion_grant
+ * RPC. This collection provides read-only lookup by suggestion IDs as well as grant and
+ * revoke operations.
  *
  * @class SuggestionGrantCollection
  * @extends BaseCollection
@@ -119,51 +120,6 @@ class SuggestionGrantCollection extends BaseCollection {
     return grantedIds.length > 0;
   }
 
-  /**
-   * Invokes the revoke_suggestion_grant RPC. Deletes suggestion_grants rows for the given
-   * grant ID and decrements tokens.used by 1.
-   * RPC name and parameter shape live in this collection (suggestion_grants).
-   *
-   * @async
-   * @param {string} grantId - Grant ID to revoke.
-   * @returns {Promise<{ data: Array|null, error: object|null }>}
-   */
-  async invokeRevokeSuggestionGrantRpc(grantId) {
-    return this.postgrestService.rpc('revoke_suggestion_grant', {
-      p_grant_id: grantId,
-    });
-  }
-
-  /**
-   * Revokes a suggestion grant by grant ID. Calls the revoke_suggestion_grant RPC to
-   * atomically delete suggestion_grants rows and refund the consumed token.
-   *
-   * @async
-   * @param {string} grantId - The grant ID to revoke.
-   * @returns {Promise<{ success: boolean, reason?: string, revokedCount?: number }>}
-   * @throws {DataAccessError} - On missing inputs or RPC failure.
-   */
-  async revokeSuggestionGrant(grantId) {
-    if (!hasText(grantId)) {
-      throw new DataAccessError('revokeSuggestionGrant: grantId is required', this);
-    }
-
-    const rpcResult = await this.invokeRevokeSuggestionGrantRpc(grantId);
-    const { data, error } = rpcResult;
-
-    if (error) {
-      this.log.error('revokeSuggestionGrant: RPC failed', error);
-      throw new DataAccessError('Failed to revoke suggestion grant (revoke_suggestion_grant)', this, error);
-    }
-
-    const row = Array.isArray(data) && data.length > 0 ? data[0] : null;
-    if (!row || !row.success) {
-      return { success: false, reason: row?.reason || 'rpc_no_result' };
-    }
-
-    return { success: true, revokedCount: row.revoked_count };
-  }
-
   /**
    * Grants one or more suggestions by consuming a single token for the given token type.
    * Resolves the current cycle token via TokenCollection#findBySiteIdAndTokenType
@@ -217,6 +173,55 @@ class SuggestionGrantCollection extends BaseCollection {
 
     return { success: true, grantedSuggestions: row.granted_suggestions };
   }
+
+  /**
+   * Invokes the revoke_suggestion_grant RPC. Deletes suggestion_grants rows for the given
+   * grant ID and decrements tokens.used by 1.
+   * RPC name and parameter shape live in this collection (suggestion_grants).
+   *
+   * @async
+   * @param {string} grantId - Grant ID to revoke.
+   * @returns {Promise<{ data: Array|null, error: object|null }>}
+   * @throws {DataAccessError} - On missing grantId.
+   */
+  async invokeRevokeSuggestionGrantRpc(grantId) {
+    if (!hasText(grantId)) {
+      throw new DataAccessError('invokeRevokeSuggestionGrantRpc: grantId is required', this);
+    }
+    return this.postgrestService.rpc('revoke_suggestion_grant', {
+      p_grant_id: grantId,
+    });
+  }
+
+  /**
+   * Revokes a suggestion grant by grant ID. Calls the revoke_suggestion_grant RPC to
+   * atomically delete suggestion_grants rows and refund the consumed token.
+   *
+   * @async
+   * @param {string} grantId - The grant ID to revoke.
+   * @returns {Promise<{ success: boolean, reason?: string, revokedCount?: number }>}
+   * @throws {DataAccessError} - On missing inputs or RPC failure.
+   */
+  async revokeSuggestionGrant(grantId) {
+    if (!hasText(grantId)) {
+      throw new DataAccessError('revokeSuggestionGrant: grantId is required', this);
+    }
+
+    const rpcResult = await this.invokeRevokeSuggestionGrantRpc(grantId);
+    const { data, error } = rpcResult;
+
+    if (error) {
+      this.log.error('revokeSuggestionGrant: RPC failed', error);
+      throw new DataAccessError('Failed to revoke suggestion grant (revoke_suggestion_grant)', this, error);
+    }
+
+    const row = Array.isArray(data) && data.length > 0 ? data[0] : null;
+    if (!row || !row.success) {
+      return { success: false, reason: row?.reason || 'rpc_no_result' };
+    }
+
+    return { success: true, revokedCount: row.revoked_count };
+  }
 }
 
 export default SuggestionGrantCollection;

packages/spacecat-shared-data-access/test/unit/models/suggestion-grant/suggestion-grant.collection.test.js

@@ -455,6 +455,13 @@ describe('SuggestionGrantCollection', () => {
 
       expect(result).to.deep.equal(rpcResult);
     });
+
+    it('throws DataAccessError when grantId is missing', async () => {
+      await expect(instance.invokeRevokeSuggestionGrantRpc(''))
+        .to.be.rejectedWith(DataAccessError, 'grantId is required');
+      await expect(instance.invokeRevokeSuggestionGrantRpc(null))
+        .to.be.rejectedWith(DataAccessError, 'grantId is required');
+    });
   });
 
   describe('invokeGrantSuggestionsRpc', () => {