Skip to content

feat(SITES-44690): add TaskManagementConnection, Ticket, and TicketSuggestion data models#1702

Open
prithipalpatwal wants to merge 15 commits into
mainfrom
feat/SITES-44690-data-models
Open

feat(SITES-44690): add TaskManagementConnection, Ticket, and TicketSuggestion data models#1702
prithipalpatwal wants to merge 15 commits into
mainfrom
feat/SITES-44690-data-models

Conversation

@prithipalpatwal

@prithipalpatwal prithipalpatwal commented Jun 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Adds TaskManagementConnection, Ticket, and TicketSuggestion data models to spacecat-shared-data-access (SITES-44690). Aligned to mysticat-data-service PR #720 Postgres schema and solution doc.

TaskManagementConnection

  • Represents an OAuth connection from an org to a task-management provider (e.g. Jira Cloud)
  • Status lifecycle: active → disabled / requires_reauth / error → disconnected (soft-delete)
  • Methods: isActive(), markRequiresReauth(), markDisabled(), markError(), markDisconnected()
  • Schema attributes aligned with PR fix(deps): update dependency @adobe/spacecat-helix-content-sdk to v1.3.72 #720 NOT NULL columns: provider, status, displayName, instanceUrl, connectedBy, metadata
  • metadata JSONB: { cloudId (required UUID), scopes (optional string[]) } — per PR fix(deps): update dependency @adobe/spacecat-helix-content-sdk to v1.3.72 #720; siteName/siteUrl are NOT in metadata, they live in displayName/instanceUrl columns
  • validateMetadata() exported for use by auth-service on connection INSERT/UPDATE

Ticket

  • Represents a Jira issue created by ASO via a TaskManagementConnection
  • Stores ticketId (Jira internal numeric ID), ticketKey, ticketUrl, ticketStatus, ticketProvider (denormalized), createdBy (IMS user ID)
  • Optional FK to Opportunity for single-suggestion queries without JOIN

TicketSuggestion

  • M:N bridge between Suggestions and Tickets (1:1 enforced in v1 via UNIQUE(suggestion_id) at DB level)
  • suggestionId is a logical TEXT reference — Suggestions live in DynamoDB, no Postgres FK
  • GSI on suggestionId powers findBySuggestionId() to detect duplicate-ticket attempts

Merge order

  1. mysticat-data-service #720 — SQL migrations (required first — integration tests pull mysticat-data-service Docker image)
  2. spacecat-shared #1701spacecat-shared-ticket-client package
  3. This PR — data models
  4. spacecat-api-service #2661 — controller + routes

CI note

Integration tests (test:it) fail until mysticat-data-service PR #720 merges and a new Docker image is published. Unit tests pass (2405 passing, 100% coverage).

v1 scope deviations

Deviation Reason Path to v2
STATUSES.DISCONNECTED added (not in spec) Soft-delete preserves audit history GC job hard-deletes disconnected rows in v2
lastUsedAt / errorMessage not in schema Nullable columns; owned by auth-service connection management PR Add attributes when auth-service PR lands
status_synced_at not in Ticket schema Webhook status sync is a v2 feature Add attribute when v2 ships

Related

  • mysticat-data-service #720 — SQL migrations (5 tables)
  • spacecat-shared #1701spacecat-shared-ticket-client
  • spacecat-infrastructure #637 — SM IAM policy
  • spacecat-api-service #2661 — controller + routes
  • SITES-44690

TaskManagementConnection:
- stores per-org Jira OAuth connection state (active / requires_reauth / disconnected)
- metadata jsonb carries cloudId, siteUrl, scopeKey without schema churn
- schema GSI on (organizationId, provider, status) powers the O(1) lookup
  the ticket-creation API needs before every request
- isActive() / markRequiresReauth() business-level methods encode the
  connection lifecycle in one place

Ticket:
- records each Jira issue created by ASO: ticketId (Jira numeric ID for
  future updates), ticketKey (human key), ticketUrl, ticketStatus
- optional opportunityId FK keeps door open for ticket-less future flows

Both models follow monorepo conventions: SchemaBuilder, BaseModel/Collection,
index.js + index.d.ts re-exports, registered in EntityRegistry.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add JSDoc note explaining that DISCONNECTED unifies the architecture
spec's 'disabled' (admin action) and 'error' (irrecoverable failure)
into a single terminal state for v1 simplicity; the two-state
distinction is deferred to v2 when admin controls are added.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown

This PR will trigger a minor release when merged.

prithipalpatwal and others added 9 commits June 22, 2026 22:09
…tion tests

Two test fixes to unblock PR #1702 CI:

1. electrodb-service-construction.test.js: increase timeout from 2000ms
   (Mocha default) to 10000ms. Adding 2 entities (43 total, up from 41
   on main) pushed full-registry Service construction past 2000ms on
   CI's slower runners. The schemas are valid — the test passes with a
   realistic timeout (locally: ~1700ms per run).

2. Add task-management-connection.model+collection tests:
   - isActive() returns true for 'active', false for other statuses
   - markRequiresReauth() updates status and calls save(); propagates errors
   - findActiveByOrganizationAndProvider() delegates to the GSI lookup
     with status='active'; throws ValidationError on bad organizationId
     or missing provider

   These tests bring task-management-connection coverage to 100%
   (model.js + collection.js + index.js), matching the package threshold.

Co-authored-by: Cursor <cursoragent@cursor.com>
Both fields are required by the architecture spec (PR #150) and were
previously omitted without justification:

- ticketProvider (readOnly): denormalized from the connection so the
  audit record is self-contained even if the connection is later deleted
- createdBy (readOnly): IMS user ID of the person who created the ticket,
  sourced from the JWT sub claim at request time

Also updates ticket.model.js JSDoc to document the two new fields and
the remaining intentional v1 deviations (no status_synced_at, no
TicketSuggestion bridge model).

TypeScript declarations updated with getTicketProvider() and getCreatedBy().

Co-authored-by: Cursor <cursoragent@cursor.com>
Adds the ticket_suggestions bridge table model per architecture spec
(PR #150). Links a specific Suggestion to the Ticket created for it.

Key design decisions:
- UNIQUE (suggestion_id) enforced at the DB level (SQL migration #3)
  prevents the same suggestion from being ticketed twice in v1.
  In v2, relax to UNIQUE (suggestion_id, ticket_id) for grouped mode.
- suggestionId is a logical TEXT reference — suggestions live in
  DynamoDB/ElectroDB, not PostgreSQL, so no Postgres FK is declared.
- GSI on suggestionId auto-generates findBySuggestionId() for the
  "has this suggestion already been ticketed?" check.
- opportunityId is denormalized on the bridge row to avoid a JOIN
  through the suggestion store for opportunity-scoped queries.

Co-authored-by: Cursor <cursoragent@cursor.com>
…or/markDisconnected

Aligns TaskManagementConnection with the full spec (PR #150) status enum:
  active | disabled | requires_reauth | error | disconnected

Previously only active, requires_reauth, disconnected were defined, missing
spec's 'disabled' (admin-disabled) and 'error' (repeated API failures).

'disconnected' is a v1 soft-delete extension — the spec hard-deletes the row;
v1 keeps it with status='disconnected' for audit history.

New methods: markDisabled(), markError(), markDisconnected().
Tests updated to cover all five statuses and new mark* methods.

Co-authored-by: Cursor <cursoragent@cursor.com>
Implements spec §Metadata Validation Strategy:
- metadata-validator.js: per-provider schemas for jira_cloud (cloudId UUID,
  siteName required, siteUrl optional https) and jira_corp (baseUrl https).
  Enforces additionalProperties: false. Throws ValidationError on any violation.
- Unknown providers are rejected — no silent passthrough.
- validateMetadata() exported from the task-management-connection index so
  auth-service (connection creation path) can call it before any DB write.
- Unit tests: 10 cases covering required fields, UUID format, https scheme,
  extra properties, and unknown provider.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ion doc

- Fix index.d.ts: add missing `export type * from './ticket-suggestion'`
- Fix TaskManagementConnection index.d.ts: add markDisabled/markError/markDisconnected
  method signatures and new getConnectedBy/getDisplayName/getInstanceUrl getters
- Fix metadata-validator: jira_cloud metadata is now { cloudId, scopes? } per
  PR #720 (mysticat-data-service) — siteName/siteUrl moved to dedicated
  display_name and instance_url columns, not stored in metadata JSONB
- Fix metadata-validator: remove dead v === undefined branch in jira_corp
  projectCategory validator; add tests for all uncovered branches (100% coverage)
- Fix task-management-connection.schema.js: add instanceUrl, displayName,
  connectedBy attributes matching NOT NULL columns in PR #720 Postgres schema
- Fix ticket.model.js: remove stale "No TicketSuggestion bridge model" comment —
  TicketSuggestion IS included in this PR
- Update model test fixture to use aligned metadata { cloudId, scopes }

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…nection

metadata JSONB is { cloudId, scopes } per PR #720, not { cloudId, siteUrl, scopeKey }.
Display fields live in instanceUrl/displayName columns.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@prithipalpatwal prithipalpatwal changed the title feat(SITES-44690): add TaskManagementConnection and Ticket data models feat(SITES-44690): add TaskManagementConnection, Ticket, and TicketSuggestion data models Jun 24, 2026

@MysticatBot MysticatBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey @prithipalpatwal,

Verdict: Request changes - four structural issues to address before merge; the code quality and patterns are solid otherwise.
Complexity: HIGH - large diff (1300+ lines, 22 files); database model schema with multiple entity relationships.
Changes: Adds TaskManagementConnection, Ticket, and TicketSuggestion data models with status lifecycle, metadata validation, and cascade relationships (22 files).
Note: CI checks are currently failing - expected per PR body (integration tests require mysticat-data-service PR #720).

Must fix before merge

  1. [Important] Ticket schema missing has_many TicketSuggestions with removeDependents: true - orphan records on cascade delete - ticket/ticket.schema.js (details inline)
  2. [Important] jira_corp metadata schema defined but unreachable through model layer - metadata-validator.js / task-management-connection.model.js (details inline)
  3. [Important] validateMetadata not wired into schema validation - invalid metadata can reach DB - task-management-connection.schema.js (details inline)
  4. [Important] metadata attribute default: {} conflicts with provider validation contracts - task-management-connection.schema.js (details inline)
Non-blocking (5): minor issues and suggestions
  • nit: instanceUrl uses weak startsWith('https://') check while ticketUrl uses the stronger isValidUrl() from shared-utils - inconsistent validation strength - task-management-connection.schema.js
  • nit: Ticket.DEFAULT_STATUS = 'To Do' uses a display-label string with spaces while all other status enums use snake_case identifiers - ticket/ticket.model.js:6
  • suggestion: ticketProvider on Ticket is unconstrained string while provider on TaskManagementConnection uses enum validation - consider type: Object.values(TaskManagementConnection.PROVIDERS) - ticket/ticket.schema.js
  • suggestion: scopes array in metadata validator has no length limit on the array or individual strings - add reasonable bounds - metadata-validator.js
  • suggestion: Consider adding markActive() convenience method for the re-auth reconnection flow - task-management-connection.model.js

Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 2m 42s | Cost: $8.82 | Commit: 1eadf949525929b3f5f245daf2e448bc4df71106
If this code review was useful, please react with 👍. Otherwise, react with 👎.

* Unless required by applicable law or agreed to in writing, software distributed under
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
* OF ANY KIND, either express or implied. See the License for the specific language
* governing permissions and limitations under the License.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (blocking): The TicketSuggestion schema declares belongs_to Ticket, creating a FK relationship. However, this Ticket schema does not declare has_many TicketSuggestions with removeDependents: true. When a Ticket is deleted (via TaskManagementConnection cascade), TicketSuggestion records will be orphaned.

The cascade path is: Connection removal -> Tickets deleted -> TicketSuggestion rows left dangling. findBySuggestionId() would then return a TicketSuggestion whose getTicket() fails or returns null, making it appear a suggestion was ticketed when the ticket no longer exists.

Fix: Add .addReference('has_many', 'TicketSuggestions', ['createdAt'], { removeDependents: true }) to this schema.

// enforced as UUID format by a DB CHECK constraint.
// - scopes (optional) is the array from the Atlassian accessible-resources response;
// stored so permission gaps can be detected without re-calling Atlassian (e.g. missing
// manage:jira-webhook when v2 webhooks land).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (blocking): The jira_corp metadata schema here is architecturally unreachable. The provider attribute in the connection schema constrains valid values to Object.values(TaskManagementConnection.PROVIDERS) which is only ['jira_cloud']. A record with provider: 'jira_corp' cannot be created through the model path.

This creates dead code that implies runtime coverage of a path that cannot execute. It will confuse the next engineer who touches this.

Fix: Either (a) remove the jira_corp entry and its tests until PROVIDERS is extended, or (b) add JIRA_CORP: 'jira_corp' to PROVIDERS now if it is genuinely needed for the auth-service pre-validation path (and document that the validator is called independently of the model layer).

})
// display_name column (PR #720): human-readable site name from Atlassian accessible-resources.
// Set by auth-service at OAuth callback time; never user-provided.
.addAttribute('displayName', {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (blocking): The metadata attribute uses type: 'any' with no validate callback. The exported validateMetadata() is never called from within the model/schema layer, meaning a TaskManagementConnection can be persisted with arbitrary metadata that violates the provider contract.

The current design relies on caller discipline with zero enforcement at the data layer. One missed call site and invalid metadata reaches the DB.

Fix: Override save() in the model to call validateMetadata(this.getProvider(), this.getMetadata()) before persisting. This follows the pattern used by other models in this codebase for co-presence invariants. Alternatively, wire it into the schema validate callback if the framework supports cross-field access.

// Set by auth-service at OAuth callback time; never user-provided.
.addAttribute('displayName', {
type: 'string',
required: true,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (blocking): The default: {} on metadata conflicts with the validation contract. For jira_cloud, an empty object is invalid (missing required cloudId). If a connection is ever created relying on this default, it will pass schema validation but silently store invalid state.

Since required: true already prevents null/undefined, the default serves no purpose other than masking an omission that should be a hard error.

Fix: Remove default: {} so that omitting metadata triggers a validation failure at creation time.

@MysticatBot MysticatBot added ai-reviewed Reviewed by AI complexity:high High complexity PR labels Jun 24, 2026

@MysticatBot MysticatBot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Hey @prithipalpatwal,

Verdict: Request changes - four data-integrity and validation issues remain unaddressed since the prior review, plus one newly identified cascade gap.
Complexity: HIGH - large diff (1300+ lines, 22 files); database model schema with multiple entity relationships.
Changes: Adds TaskManagementConnection, Ticket, and TicketSuggestion data models with status lifecycle, metadata validation, and cascade relationships (22 files).
Note: CI checks are currently failing - expected per PR body (integration tests require mysticat-data-service PR #720).

Must fix before merge

  1. [Important] Ticket schema missing has_many TicketSuggestions with removeDependents: true - orphan records on cascade delete - ticket/ticket.schema.js:20 (details inline)
  2. [Important] Organization schema missing has_many TaskManagementConnections - cascade from org deletion will orphan connections and their child tickets - organization/organization.schema.js (not in diff - one-line addition needed: .addReference('has_many', 'TaskManagementConnections', ['updatedAt'], { removeDependents: true }) following the existing Sites/Projects/Entitlements pattern)
  3. [Important] instanceUrl uses weak startsWith('https://') while sibling ticketUrl uses isValidUrl() from shared-utils - inconsistent validation strength - task-management-connection.schema.js:48 (details inline)
  4. [Important] metadata attribute default: {} conflicts with validateMetadata('jira_cloud', {}) which requires cloudId - creates schema-valid but business-invalid state - task-management-connection.schema.js:60 (details inline)
Non-blocking (5): minor issues and suggestions
  • nit: jira_corp metadata schema in metadata-validator.js is unreachable through model layer (PROVIDERS enum only has jira_cloud) - add a comment noting it is forward-looking, or remove until PROVIDERS is extended
  • suggestion: Wire validateMetadata into the schema validate callback or add a prominent JSDoc on the metadata attribute noting that callers MUST call it externally - the current implicit contract is invisible to future contributors - task-management-connection.schema.js:57
  • nit: scopes array in metadata validator has no length limit on the array or individual strings - add reasonable bounds (e.g., max 100 entries, max 256 chars each) - metadata-validator.js:48
  • nit: validateMetadata does not guard against non-object input (null/undefined/string) - will throw TypeError instead of clean ValidationError - metadata-validator.js:73
  • suggestion: Add the 3 new entities to the README's "Current exported entities" list - packages/spacecat-shared-data-access/README.md

Skill: pr-review | Model: us.anthropic.claude-opus-4-6-v1[1m] | Duration: 3m 3s | Cost: $8.14 | Commit: 9b00cf55343c95dd6c9bc628a44fb8c76b584255
If this code review was useful, please react with 👍. Otherwise, react with 👎.

import SchemaBuilder from '../base/schema.builder.js';
import Ticket from './ticket.model.js';
import TicketCollection from './ticket.collection.js';

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (blocking): The cascade chain is incomplete. TaskManagementConnection correctly declares has_many Tickets with removeDependents: true, but this Ticket schema has no corresponding has_many TicketSuggestions. When a Ticket is deleted (via Connection cascade), TicketSuggestion rows are orphaned. The findBySuggestionId() GSI will then return bridge records pointing at deleted tickets, making it appear a suggestion is still ticketed.

The Opportunity schema in this codebase uses the identical pattern: .addReference("has_many", "Suggestions", ["updatedAt"], { removeDependents: true }).

Fix: Add .addReference('has_many', 'TicketSuggestions', ['updatedAt'], { removeDependents: true }) to this schema.

// calls route through the fixed Atlassian gateway keyed on cloudId from metadata).
.addAttribute('instanceUrl', {
type: 'string',
required: true,

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (blocking): instanceUrl validates with value.startsWith("https://") which accepts malformed values like https://, https:// , or strings with control characters. The Ticket schema uses the stricter isValidUrl() from @adobe/spacecat-shared-utils for ticketUrl. Since instanceUrl is stored permanently and rendered in UIs, it should use the same validation.

Fix:

import { isValidUrl } from '@adobe/spacecat-shared-utils';
// ...
validate: (value) => isValidUrl(value),

})
// metadata JSONB (PR #720): provider-specific structured data.
// jira_cloud: { cloudId (required UUID), scopes (optional string array) }.
// siteName and siteUrl are NOT stored here — they live in displayName/instanceUrl above.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

issue (blocking): metadata has default: {} but validateMetadata('jira_cloud', {}) throws because cloudId is required. Since required: true already ensures callers must provide the field, this default only fires when metadata is omitted - creating a record that passes schema validation but violates the business invariant. Any reader trusting metadata.cloudId on such a record will encounter undefined.

Fix: Remove default: {}. The required: true constraint already enforces that callers provide metadata explicitly.

prithipalpatwal and others added 3 commits June 24, 2026 16:56
… schema

Adds the externalInstanceId attribute (required, readOnly) to align with the
new external_instance_id column in mysticat-data-service PR #720.

- Add externalInstanceId attribute to schema (string, required, readOnly)
- Add externalInstanceId to mockRecord in model test
- Add schema test file covering the new attribute

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…sues

- Add OAuthNonce model (schema, model, collection, index) with custom
  delete({ nonce }) method for atomic single-use replay prevention
- Register OAuthNonce in EntityRegistry and models/index.js
- Fix TicketSuggestion.opportunityId: required: false → required: true
  (matches DB NOT NULL constraint; avoids opaque 500 on create)
- Fix Ticket.ticketStatus: required: true → required: false, default: null
  (DB column is nullable; status is set by async sync job, not on create)
- Remove Ticket.DEFAULT_STATUS = 'To Do' (Jira-specific constant no
  longer referenced; provider defaults belong in the client layer)
- Add TODO tracking lastUsedAt/errorMessage missing from
  TaskManagementConnection schema (follow-up)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ai-reviewed Reviewed by AI complexity:high High complexity PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants