Skip to content

feat: SITES-40623 - token system in Spacecat#1414

Merged
sandsinh merged 34 commits intomainfrom
SITES-40623
Mar 19, 2026
Merged

feat: SITES-40623 - token system in Spacecat#1414
sandsinh merged 34 commits intomainfrom
SITES-40623

Conversation

@sandsinh
Copy link
Contributor

@sandsinh sandsinh commented Mar 9, 2026


Summary

  • Add SuggestionGrant entity: New model, collection, schema, and TypeScript definitions for the suggestion_grants table. The collection provides splitSuggestionsByGrantStatus, isSuggestionGranted,
    grantSuggestions, and findBySuggestionIds — centralizing all grant-related data access previously spread across the Suggestion collection.
  • Add Token entity: New model, collection, and schema for the tokens table. TokenCollection.findBySiteIdAndTokenType resolves the current-cycle token (auto-creating if createIfNotFound: true), and
    Token.getRemaining() computes available tokens (total - used).
  • Bump data-service Docker image: Updated from v1.19.0 to v1.23.0 for integration tests (includes suggestion_grants, tokens tables, and grant_suggestions RPC).

Changes

New: SuggestionGrant (src/models/suggestion-grant/)

  • suggestion-grant.schema.js — Schema with suggestionId, grantId, siteId, tokenId, tokenType, grantedAt (all read-only). updatedAt/updatedBy marked postgrestIgnore (insert-only table).
  • suggestion-grant.collection.js — findBySuggestionIds (bulk lookup), splitSuggestionsByGrantStatus (partition into granted/not-granted), isSuggestionGranted (single-ID check), grantSuggestions
    (resolves token, calls grant_suggestions RPC), invokeGrantSuggestionsRpc (raw RPC call).
  • suggestion-grant.model.js — Minimal model extending BaseModel.
  • index.js / index.d.ts — Exports and TypeScript interface definitions.

New: Token (src/models/token/)

  • token.schema.js — Schema with siteId, tokenType, cycle, total, used. Composite index on (siteId, tokenType) + cycle.
  • token.collection.js — findBySiteIdAndTokenType(siteId, tokenType, options) resolves current-cycle token using getTokenGrantConfig from spacecat-shared-utils; creates token if createIfNotFound: true
    with total = min(options.total, config.tokensPerCycle).
  • token.model.js — getRemaining() returns max(0, total - used). Static TOKEN_TYPES enum.

Modified

  • entity.registry.js — Registered SuggestionGrant and Token entities.
  • models/index.js — Re-exported new entity modules.
  • postgrest.utils.js — Added SuggestionGrant: 'suggestion_grants' table override.
  • suggestion.schema.js — Added optional suggestionKey attribute.

Tests

  • Unit tests: suggestion-grant.collection.test.js (416 lines), suggestion-grant.model.test.js, token.collection.test.js, token.model.test.js, token.schema.test.js, suggestion.model.test.js (updated for
    suggestionKey).
  • Integration tests: suggestion-grant.test.js, suggestion.test.js (grant flow), token.test.js, all-collections-methods-coverage.test.js (updated for new entities). Docker image bumped to v1.23.0.

Test plan

  • Unit tests for all new collection methods, model methods, and schema validation
  • Integration tests against real PostgREST (Docker) for token lifecycle and grant flow
  • splitSuggestionsByGrantStatus correctly partitions granted vs. not-granted IDs
  • grantSuggestions consumes a token and inserts grant rows via RPC
  • findBySiteIdAndTokenType auto-creates tokens when createIfNotFound: true
  • Token.getRemaining() returns correct value and clamps to 0
  • Error paths: invalid inputs throw DataAccessError, RPC failures propagate correctly
  • All existing tests continue to pass

Related Issue


Motivation and Context

Sites need a capped number of granted suggestions per opportunity type and cycle. The shared library adds:

  • Token schema so clients can read/create token rows and resolve current-cycle allocation.
  • Grant config in utils so cycle and per-cycle limits are defined in one place and reused (e.g. by TokenCollection for creation and cycle).
  • Suggestion grants so UI/API can show which suggestions were granted and with which cycle/token.
  • grantSuggestion so callers can grant by IDs without touching RPC or token creation directly; token creation for the current cycle happens on first use.

How Has This Been Tested?

  • spacecat-shared-utils: token-grant-config.test.jsTOKEN_GRANT_CONFIG shape, tokensPerCycle/cycle/cycleFormat per type, getTokenGrantConfig returns correct values and currentCycle, unknown type returns undefined, and that config is immutable (e.g. copy, not reference).
  • spacecat-shared-data-access:
    • Token: token.schema.test.js, token.model.test.js, token.collection.test.js (e.g. findBySiteIdAndTokenType with create/upsert and config).
    • Suggestions: suggestion.collection.test.jsgrantSuggestion calls findBySiteIdAndTokenType, checks remaining, invokes RPC with correct params; returns no_tokens when remaining < count and does not call RPC; handles RPC error and RPC granted: false.
    • Integration: test/it/token/token.test.js and suggestion ITs where applicable.

Please ensure your pull request adheres to the following guidelines:

  • make sure to link the related issues in this description
  • when merging / squashing, make sure the fixed issue references are visible in the commits, for easy compilation of release notes

Related Issues

Thanks for contributing!

Made-with: Cursor

# Conflicts:
#	packages/spacecat-shared-data-access/src/models/entitlement/entitlement.collection.js
#	packages/spacecat-shared-data-access/test/unit/models/entitlement/entitlement.collection.test.js
@sandsinh sandsinh marked this pull request as draft March 9, 2026 13:12
@github-actions
Copy link

github-actions bot commented Mar 9, 2026

This PR will trigger a minor release when merged.

@sandsinh sandsinh marked this pull request as ready for review March 10, 2026 12:03
@Kanishkavijay39
Copy link
Contributor

verview

Adds two new entities (Token, SuggestionGrant) to implement a capped suggestion grant system. Tokens track per-site, per-cycle allocation; SuggestionGrant records
which suggestions were granted and via which token. Includes a grantSuggestions high-level method that checks token balance, calls a PostgREST RPC, and returns
results.


Critical Issues

  1. Production dependency pinned to a GitHub Gist tarball

"@adobe/spacecat-shared-utils": "https://gist.githubusercontent.com/sandsinh/e6c7d5895ece4f0ad3b32a64701faf3b/raw/..."

This is the most significant blocker. The Gist URL:

This PR must not be merged until #1420 is published to npm and the dependency is updated to a proper semver reference.


  1. grantSuggestions token check is wrong for batch grants

// suggestion-grant.collection.js
if (token.getRemaining() >= 1) { ... }

This only checks that at least 1 token remains. If suggestionIds.length > token.getRemaining(), the RPC is still called with all IDs. The check should be:

if (token.getRemaining() >= suggestionIds.length) { ... }

Unless the intent is that the entire batch consumes exactly 1 token (not 1 per suggestion), which is not documented anywhere in the code or PR description.


Design & Architecture Issues

  1. Cross-entity coupling inside a collection

SuggestionGrantCollection.grantSuggestions reaches into entityRegistry to resolve TokenCollection:

const tokenCollection = this.entityRegistry.getCollection('Token');

This is unusual for this codebase — collections typically don't resolve sibling collections. It creates hidden coupling and makes the method harder to test in
isolation. Consider moving this orchestration to the service layer, or at minimum documenting it as intentional.

  1. findBySuggestionIds and invokeGrantSuggestionsRpc return raw { data, error } PostgREST responses

These two methods bypass the standard entity-level error handling the rest of the layer uses (DataAccessError). If PostgREST returns an error, callers of
findBySuggestionIds get a raw error object rather than a thrown exception. This is inconsistent — grantSuggestions does handle the error from
invokeGrantSuggestionsRpc, but findBySuggestionIds callers must handle { data, error } themselves.

  1. invokeGrantSuggestionsRpc is public

It's a raw implementation detail exposed on the collection. It should be private (rename to #invokeGrantSuggestionsRpc or document as internal-only). Exposing it
invites callers to bypass grantSuggestions's token-check logic.


Code-Level Issues

  1. Token.getRemaining() uses fragile optional chaining

getRemaining() {
const total = this.getTotal?.() ?? this.total ?? 0;
const used = this.getUsed?.() ?? this.used ?? 0;
return Math.max(0, total - used);
}

this.getTotal?.() with ?. implies getTotal might not exist. In this codebase, BaseModel generates getters from the schema — they'll always exist. The fallback to
this.total is unnecessary and potentially masks a bug if the schema generation changes. Should be simply:

return Math.max(0, this.getTotal() - this.getUsed());

  1. token.schema.js — total is readOnly but created by the collection

readOnly on total prevents updates, which is intentional — total is set at creation time. But the name is slightly misleading. A comment clarifying "set at creation,
never updated" would help.

  1. docker-compose.yml — pull_policy: always

pull_policy: ${MYSTICAT_DATA_SERVICE_PULL_POLICY:-always}

Defaults to always, meaning every local npm run test:it run re-pulls the Docker image even if it's already present. This significantly slows local dev. The default
should be missing (only pull if not cached locally), with always available for CI via the env var override. The previous behaviour had no pull_policy, which Docker
Compose defaults to missing.


Test Coverage

Strengths:

  • Good unit test coverage for all new methods including error paths
  • Integration tests cover token exhaustion (no_tokens) scenario
  • Mutation guards and undefined input cases are tested

Minor Issues

  • suggestion-grant.schema.js marks updatedAt/updatedBy as postgrestIgnore — fine for an insert-only table, but a comment explaining why would help future readers
  • PR description mentions integration test coverage for grantSuggestions via Suggestion, but SuggestionGrantCollection.grantSuggestions appears to live in
    suggestion-grant, not suggestion — the PR description mixes these up

.from(this.tableName)
.select('suggestion_id,grant_id')
.in('suggestion_id', suggestionIds);
return { data: data ?? [], error };
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can throw DataAccessError in case of failure

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

throw new DataAccessError('grantSuggestions: tokenType is required', this);
}

const tokenCollection = this.entityRegistry.getCollection('TokenCollection');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Design concern: Collections typically don't call into other collections via entityRegistry. This cross-entity coupling is atypical for this codebase orchestration usually lives in the service layer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using it in some collections. So it should be good.

const tokenCollection = this.entityRegistry.getCollection('TokenCollection');
const token = await tokenCollection.findBySiteIdAndTokenType(siteId, tokenType);

if (!token || token.getRemaining() < 1) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical bug: token.getRemaining() >= 1 only checks if at least one token remains. If you pass 5 suggestionIds but only 2 tokens remain, it still proceeds. Should be

= suggestionIds.length (assuming 1 token per suggestion) or the semantics need documenting.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

1 token could grant multiple suggestions in some cases, no need for check on suggestionIds.

@sandsinh sandsinh merged commit 9c540ba into main Mar 19, 2026
7 checks passed
@sandsinh sandsinh deleted the SITES-40623 branch March 19, 2026 11:54
solaris007 pushed a commit that referenced this pull request Mar 19, 2026
## [@adobe/spacecat-shared-data-access-v3.24.0](https://github.com/adobe/spacecat-shared/compare/@adobe/spacecat-shared-data-access-v3.23.0...@adobe/spacecat-shared-data-access-v3.24.0) (2026-03-19)

### Features

* SITES-40623 - token system in Spacecat ([#1414](#1414)) ([9c540ba](9c540ba))
@solaris007
Copy link
Member

🎉 This PR is included in version @adobe/spacecat-shared-data-access-v3.24.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants