Skip to content

feat(subscriptions): foundation + read methods (PR 4/7)#516

Open
sarthak688 wants to merge 1 commit into
mainfrom
feat/subscriptions-sdk
Open

feat(subscriptions): foundation + read methods (PR 4/7)#516
sarthak688 wants to merge 1 commit into
mainfrom
feat/subscriptions-sdk

Conversation

@sarthak688

@sarthak688 sarthak688 commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

PR 4/7 in the Notification SDK stack. Rebased onto main (foundation #512, mark-read #513, and delete #514 are now merged); this branch is a single squashed commit.

Adds the Subscriptions service alongside the existing Notifications service. Both live under @uipath/uipath-typescript/notifications as discrete classes (no shared state). This PR ships the 3 read methods + subscription model foundation.

All three methods are tagged @internal (the NotificationService scope is internal), so no docs/oauth-scopes.md or mkdocs.yml nav entries are added — matching #512#514.

Methods Added

Layer Method Signature
Service subscriptions.getAll() getAll(tenantId: string, options?: SubscriptionGetAllOptions): Promise<SubscriptionGetResponse>
Service subscriptions.getPublishers() getPublishers(tenantId: string, options?: SubscriptionGetPublishersOptions): Promise<SubscriptionGetPublishersResponse>
Service subscriptions.getSupportedChannels() getSupportedChannels(tenantId: string): Promise<SubscriptionGetSupportedChannelsResponse>

Endpoints Called

Method HTTP Endpoint
subscriptions.getAll() GET notificationservice_/usersubscriptionservice/api/v1/UserSubscription
subscriptions.getPublishers() GET notificationservice_/usersubscriptionservice/api/v1/UserSubscription/GetPublishers
subscriptions.getSupportedChannels() GET notificationservice_/usersubscriptionservice/api/v1/UserSubscription/GetSupportedChannelStatus

Response Modeling

getAll() and getPublishers() populate genuinely different field sets (verified against the backend GetUserSubscriptionHandlers vs GetPublisherDetailsHandler), so each has its own explicit response type:

  • getAll()SubscriptionGetResponse — full per-publisher, per-topic, per-channel subscription state (SubscriptionPublisher[]).
  • getPublishers()SubscriptionGetPublishersResponse — discovery-only catalogue (SubscriptionPublisherBase[]) carrying only identity fields + topic list, no subscription state.
  • The full SubscriptionPublisher / SubscriptionTopic extend the base shapes SubscriptionPublisherBase / SubscriptionTopicBase, so the discovery fields are declared once. Accessing state-only fields (isSubscribed, modes, isUserOptin, …) on a getPublishers() result is a compile error, steering callers to getAll().
  • getSupportedChannels() returns Email/Slack/Teams with isEnabled. InApp is implicitly always available and not included in the response.
  • Adds the NotificationMode delivery-channel enum (InApp/Email/Slack/Teams) to notifications.types.ts, shared by the subscription channel/mode shapes.
  • All three methods carry @track(...) telemetry decorators and forward the acting tenantId as the first argument.

Example Usage

import { Subscriptions, NotificationMode } from '@uipath/uipath-typescript/notifications';

const subscriptions = new Subscriptions(sdk);
const tenantId = '<tenant-guid>';

// Full subscription state
const { publishers } = await subscriptions.getAll(tenantId);

// Filter to specific publishers
const orchestrator = await subscriptions.getAll(tenantId, {
  publishers: ['Orchestrator', 'Actions'],
});

// Discovery (no state) — SubscriptionPublisherBase[]
const { publishers: catalogue } = await subscriptions.getPublishers(tenantId);

// Channel availability — InApp is always implicit and not listed
const { channels } = await subscriptions.getSupportedChannels(tenantId);
const slack = channels.find(c => c.name === NotificationMode.Slack);

Sample SDK Responses

subscriptions.getSupportedChannels(tenantId)
{
  "channels": [
    { "name": "Email", "isEnabled": true },
    { "name": "Slack", "isEnabled": false },
    { "name": "Teams", "isEnabled": false }
  ]
}
subscriptions.getPublishers(tenantId, { name: 'Apps' })SubscriptionGetPublishersResponse
{
  "publishers": [
    {
      "id": "3b5c8128-8af0-46ec-eb1d-08da31909e0f",
      "name": "Apps",
      "displayName": "Apps",
      "topics": [
        { "id": "b3f9b2fd-122f-4d40-3d88-08da31909e1a", "name": "Apps.Shared", "displayName": "Apps Shared", "description": "Apps is Shared", "group": "Apps Activities" },
        { "id": "7a647eb5-381e-4712-55de-08db6be7681e", "name": "Apps.Cloned", "displayName": "App Duplicated", "description": "App is Duplicated", "group": "Apps Activities" }
      ]
    }
  ]
}

The discovery endpoint returns only id/name/displayName/description/group on each topic (the SubscriptionTopicBase shape).

subscriptions.getAll(tenantId, { publishers: ['Apps'] })SubscriptionGetResponse
{
  "publishers": [
    {
      "id": "3b5c8128-8af0-46ec-eb1d-08da31909e0f",
      "name": "Apps",
      "displayName": "Apps",
      "isUserOptin": true,
      "modes": [
        { "name": "InApp", "isActive": true },
        { "name": "Email", "isActive": true }
      ],
      "topics": [
        {
          "id": "b3f9b2fd-122f-4d40-3d88-08da31909e1a",
          "name": "Apps.Shared",
          "category": "Success",
          "isSubscribed": true,
          "isMandatory": false,
          "modes": [
            { "name": "InApp", "isSubscribed": true, "isSubscribedByDefault": true },
            { "name": "Email", "isSubscribed": true, "isSubscribedByDefault": true }
          ]
        }
      ]
    }
  ]
}

Verification

Check Status
npm run typecheck ✅ clean
npm run lint ✅ 0 warnings, 0 errors
npm run test:unit ✅ 24 tests in notification suite (16 notifications + 8 subscriptions)
npm run build ✅ ESM/CJS/UMD/.d.ts

Files

Area Files
Endpoint constants src/utils/constants/endpoints/notification.ts (SUBSCRIPTION_ENDPOINTS, SUBSCRIPTION_API_BASE)
Enums src/models/notification/notifications.types.ts (NotificationMode)
Types src/models/notification/subscriptions.types.ts (SubscriptionPublisherBase/SubscriptionPublisher, SubscriptionTopicBase/SubscriptionTopic, SubscriptionMode, AllowedMode, SupportedChannel, SubscriptionEntity, response + options types)
Models src/models/notification/subscriptions.models.ts (SubscriptionServiceModel), src/models/notification/index.ts
Service src/services/notification/subscriptions.ts, src/services/notification/index.ts
Unit tests tests/unit/services/notification/subscriptions.test.ts (8 tests)
Integration tests tests/integration/shared/notification/subscriptions.integration.test.ts
Integration wiring tests/integration/config/unified-setup.ts (Subscriptions registration)
Test utils tests/utils/constants/notification.ts (subscription constants, ERROR_PUBLISHER_NOT_FOUND), tests/utils/mocks/notification.ts (createBasicSubscriptionPublisher/Base, createBasicSubscriptionTopic/Base, createBasicSupportedChannels)

PR Stack

# Branch Status
1 feat/notifications-sdk #512 (merged)
2 feat/notifications-mark-read #513 (merged)
3 feat/notifications-delete #514 (merged)
4 feat/subscriptions-sdk (this PR)
5 feat/subscriptions-topic-updates upcoming
6 feat/subscriptions-publisher-updates upcoming
7 feat/subscriptions-mode-reset upcoming

🤖 Generated with Claude Code

Comment thread src/models/notification/subscriptions.models.ts Outdated
Comment thread src/services/notification/subscriptions.ts Outdated
Comment thread tests/unit/services/notification/subscriptions.test.ts Outdated
@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

3 raw-string enum violations (CLAUDE.md: use enums for fixed value sets — NEVER leave raw strings/numbers):

  • subscriptions.models.ts:112 — JSDoc example uses 'Slack' instead of NotificationMode.Slack; also needs a NotificationMode import to be self-contained
  • subscriptions.ts:127 — same raw string in the service-file JSDoc copy
  • subscriptions.test.ts:134 — unit test asserts .not.toContain('InApp') instead of .not.toContain(NotificationMode.InApp); needs a NotificationMode import

Everything else looks good — architecture, pagination pattern, transform pipeline, @track decorators, JSDoc structure, test coverage, and integration test setup all follow conventions.

sarthak688 added a commit that referenced this pull request Jun 12, 2026
Three CLAUDE.md violations (use enums for fixed value sets — NEVER
raw strings):

- subscriptions.models.ts JSDoc example: 'Slack' → NotificationMode.Slack
  (with import for self-contained example)
- subscriptions.ts JSDoc example: same fix
- subscriptions.test.ts assertion: 'InApp' → NotificationMode.InApp
  (with import)

Addresses review comments on PR #516.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@sarthak688

Copy link
Copy Markdown
Contributor Author

All findings addressed in the latest commit on this branch. Inline reply threads are resolved with commit references.

Comment thread src/services/notification/subscriptions.ts Outdated
@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

1 new finding this run:

subscriptions.ts:33-34 — PR-description sentence in the class-level JSDoc ("This PR ships the three read methods…") will surface in IDE tooltips for SDK consumers and become stale once mutation methods land in the follow-up PR. Suggestion posted inline to drop the last paragraph.

@claude

claude Bot commented Jun 12, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

@sarthak688 sarthak688 force-pushed the feat/notifications-delete branch 2 times, most recently from fe38d8a to 28f2e06 Compare July 2, 2026 12:22
Base automatically changed from feat/notifications-delete to main July 2, 2026 13:05
@sarthak688 sarthak688 requested a review from a team July 2, 2026 13:05
@sarthak688 sarthak688 force-pushed the feat/subscriptions-sdk branch 2 times, most recently from 2f0d37c to 1d382c9 Compare July 6, 2026 08:26
@claude

claude Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

1 new finding this run:

tests/integration/shared/notification/subscriptions.integration.test.ts:8describe.skip is missing the required explanation comment. The sibling notifications.integration.test.ts has the pattern (lines 9–11). CLAUDE.md rules: describe.skip is permitted only when the service does not support PAT auth — add a comment explaining the limitation. Suggestion posted inline.

@sonarqubecloud

sonarqubecloud Bot commented Jul 6, 2026

Copy link
Copy Markdown

Comment on lines +68 to +69
headers: createHeaders({ [TENANT_ID]: tenantId }),
...(options?.publishers ? { params: { Publishers: options.publishers } } : {}),

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please test getAll() with multiple publishers as this is the first method in the SDK that passes an array as a query param. The sample output in the PR description only covers publishers: ['Apps'] a single-element array

@sarthak688 sarthak688 Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I didn't get your point. Passing single element or multiple element API logic doesn't change.
The API is already well tested by NS integration/e2e test.

Comment thread src/services/notification/subscriptions.ts Outdated
Comment on lines +181 to +184
export interface SubscriptionGetResponse {
/** Publishers with their topics and subscription state. */
publishers: SubscriptionPublisher[];
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why are these responses wrapped? Lets not wrap the responses

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We have the same pattern in our API contract. It's done to keep both DTOs aligned.
Think of a scenario where we want to add a field to this DTO in addition to the publishers. We can simply add it in both places without breaking compatibility.
It's better to keep them aligned.
Also, what benefit do you see in unwrapping them?

Comment on lines +192 to +195
export interface SubscriptionGetPublishersResponse {
/** Publishers with their topic catalogue. */
publishers: SubscriptionPublisherBase[];
}

@Sarath1018 Sarath1018 Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same here and for below method as well

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

same reply

Comment thread src/models/notification/subscriptions.types.ts Outdated
…ods [internal]

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@sarthak688 sarthak688 force-pushed the feat/subscriptions-sdk branch from 1d382c9 to 18c7fcb Compare July 9, 2026 07:15
@claude

claude Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

✅ No issues found. Checked for bugs and CLAUDE.md compliance.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants