Implement campaigns module with support for the Campaigns API by Sher-Bakhodirov · Pull Request #257 · Constructor-io/constructorio-node

Sher-Bakhodirov · 2026-06-05T11:55:22Z

No description provided.

Mudaafi

This is looking pretty good! I just have some comments w.r.t. the types, but thanks for working on adding this in!

Mudaafi · 2026-06-11T20:33:46Z

+export interface CampaignRuleInput extends Record<string, any> {
+  rule: Record<string, any>;
+  rule_type?:
+    | 'boost'
+    | 'blacklist'
+    | 'slot'
+    | 'content'
+    | 'filters_slot'
+    | 'whitelist'
+    | 'variation_slicing';
+  request_tag_name?: RequestTag;
+  request_tag_value?: string;
+  active?: boolean;
+  start_time?: string;
+  end_time?: string;
+  campaign_id?: number;
+  automatically_generated?: boolean;
+}


Seems a little verbose to repeat type declarations here. Can we use something like this instead? wdyt?

Suggested change

export interface CampaignRuleInput extends Record<string, any> {

rule: Record<string, any>;

rule_type?:

| 'boost'

| 'blacklist'

| 'slot'

| 'content'

| 'filters_slot'

| 'whitelist'

| 'variation_slicing';

request_tag_name?: RequestTag;

request_tag_value?: string;

active?: boolean;

start_time?: string;

end_time?: string;

campaign_id?: number;

automatically_generated?: boolean;

}

export interface CampaignRuleInput extends Partial<Omit<CampaignRule, 'created_at' | 'updated_at'>> {

id: number;

}

Done, with two tweaks: omitted id instead of requiring it (it's server-assigned on create), and used a distributive OmitServerFields<T> helper so the discriminated union doesn't collapse (a plain Omit would decouple rule_type from rule). Fixed in b3d1005.

Mudaafi · 2026-06-11T20:35:36Z

+  requestTagName?: RequestTag;
+  requestTagValue?: string;
+  startTime?: string;
+  endTime?: string;
+  refinedQueries?: RefinedQuery[];
+  refinedFilters?: RefinedFilter[];
+  refinedRecommendationContexts?: RecommendationContext[];
+  boostRules?: CampaignRuleInput[];
+  blacklistRules?: CampaignRuleInput[];
+  slotRules?: CampaignRuleInput[];
+  contentRules?: CampaignRuleInput[];
+  filtersSlotRules?: CampaignRuleInput[];
+  whitelistRule?: CampaignRuleInput | null;
+  variationSlicingRule?: CampaignRuleInput | null;
+  metadataJson?: CampaignMetadata;


Let's make this a CampaignRule object since it's being re-used across multiple interfaces.

or actually we already have a Campaign object below. Can't we reuse that here rather than redefining all these fields?

Extracted a shared CampaignParametersBase that both Create/Update extend. Didn't reuse Campaign since it's the snake_case response shape (rules carry server-assigned ids), whereas these are camelCase input params. Fixed in edc257e.

Mudaafi · 2026-06-11T20:41:52Z

+
+export interface RecommendationContext extends Record<string, any> {
+  pod_id: string;
+  condition: Record<string, any>;


This can be defined into three types of conditions right? https://docs.constructor.com/reference/v1-searchandising-create-campaign

Fixed in 24d09a3.

Mudaafi · 2026-06-11T20:44:32Z

+  condition: Record<string, any>;
+}
+
+export interface CampaignRule extends Record<string, any> {


Let's generalize this to RuleObj since it's a shared object for searchandizing rules as well

Fixed in 7f5bc29.

Mudaafi · 2026-06-11T20:46:44Z

+
+export interface CampaignRule extends Record<string, any> {
+  id: number;
+  rule: Record<string, any>;


Similarly, we can define a generic rule object and branch it out to the different types by following the schema here: https://github.com/Constructor-io/autocomplete/blob/master/ingestion_service/src/ingestion_service/blueprints/serializers/rules.py#L162

Fixed in 7f5bc29.

Mudaafi · 2026-06-11T20:53:54Z

+   * @param {object} [parameters.refinedFilters] - An object of refined filters to filter by
+   * @param {number} [parameters.numResultsPerPage=20] - The number of campaigns to return - maximum value 100
+   * @param {number} [parameters.page] - The page of results to return
+   * @param {number} [parameters.offset] - The number of results to skip from the beginning - cannot be used together with `page`
+   * @param {object} [parameters.refinedRecommendationContexts] - A filter for refined recommendation contexts
+   * @param {string[]} [parameters.refinedQueries] - A list of refined queries to filter by


I know this is the order the documentation lists the parameters, but let's move the refinedQueries and refinedRecommendationContexts` up just for better QOL

Suggested change

* @param {object} [parameters.refinedFilters] - An object of refined filters to filter by

* @param {number} [parameters.numResultsPerPage=20] - The number of campaigns to return - maximum value 100

* @param {number} [parameters.page] - The page of results to return

* @param {number} [parameters.offset] - The number of results to skip from the beginning - cannot be used together with `page`

* @param {object} [parameters.refinedRecommendationContexts] - A filter for refined recommendation contexts

* @param {string[]} [parameters.refinedQueries] - A list of refined queries to filter by

* @param {string[]} [parameters.refinedQueries] - A list of refined queries to filter by

* @param {object} [parameters.refinedFilters] - An object of refined filters to filter by

* @param {object} [parameters.refinedRecommendationContexts] - A filter for refined recommendation contexts

* @param {number} [parameters.numResultsPerPage=20] - The number of campaigns to return - maximum value 100

* @param {number} [parameters.page] - The page of results to return

* @param {number} [parameters.offset] - The number of results to skip from the beginning - cannot be used together with `page`

I'll see if I can throw up a PR to the official docs to better improve the docs there too

Fixed in c39f99c.

Mudaafi · 2026-06-11T21:03:43Z

+    it('Should retrieve a list of campaigns given section', (done) => {
+      const section = 'Products';


If we want to include this test, we should use a different section. Since the default is always Products, we won't know if this actually succeeds or if it's just using the default.

Note: This means upon starting the test-suite, we'll probably want to make sure a campaign exists in another section as well.

Fixed in cf831d8.

TarekAlQaddy

left 2 small comments.

What do you think about having a separate sub-module for campaigns instead of having it inside searchandising.js? I don't like that catalog.js has become a huge file now (~5k lines) that includes different entities.
I'm thinking about having

searchandising/index.js
searchandising/campaigns.js
searchandising/search.js
...

each sub-module would be a separate class having it's own methods and it will be initiated in the parent constructor. From the consumer's POV it should be used as such searchandising.campaigns.retrieveCampiagns()
Please let me know your thoughts 👀

TarekAlQaddy · 2026-06-23T14:56:17Z

+        refinedQueries = refined_queries,
+      } = parameters;
+
+      // Pull section from parameters


I was wondering if these type of comments (Pull X from parameters) add value as they are used for every query param in this method which might be excessive

Agreed — removed the repetitive // Pull X from parameters comments in 20379dd.

TarekAlQaddy · 2026-06-23T15:40:13Z

+    setTimeout(done, sendTimeout);
+  });
+
+  describe('retrieveCampaigns', () => {


I would suggest adding tests to validate passing the query params in "retrieveCampaigns" method as they are explicitly extracted from the "parameters" param

Added query-param tests for retrieveCampaigns in b481ff9, covering each param extracted from parameters — id (single + array), refinedFilters, refinedRecommendationContexts, refinedQueries, plus their snake_case aliases. Also added the missing searchandising.test-d.ts type test.

Split the flat searchandising module into a parent class that exposes a campaigns sub-module, so each searchandising entity lives in its own class and file. Consumers now call searchandising.campaigns.retrieveCampaigns(). Also drops the repetitive "Pull X from parameters" comments.

Cover query params extracted from parameters in retrieveCampaigns (id, refinedFilters, refinedRecommendationContexts, refinedQueries, plus their snake_case aliases) and add the missing searchandising type tests.

Use an interface for the campaigns sub-module so the type file keeps a single declared class, satisfying the max-classes-per-file lint rule that also runs over .d.ts files in the pre-push hook.

esezen · 2026-06-24T14:47:22Z

@TarekAlQaddy good call on the sub-module split — done in 20379dd. Restructured into:

searchandising/
  index.js      // parent class, constructs this.campaigns
  campaigns.js  // Campaigns class with the 5 methods

Usage is now searchandising.campaigns.retrieveCampaigns() as you suggested, which gives us a clean place to add future searchandising entities without growing one file. Types are split the same way (Campaigns interface + Searchandising exposing campaigns).

constructor-claude-bedrock

Code Review

This PR implements a new searchandising module with a campaigns sub-module exposing CRUD operations for the Campaigns API, complete with TypeScript type definitions and integration tests — the implementation is well-structured and consistent with existing modules, but has a few bugs and quality issues that should be addressed before merging.

Inline comments: 9 discussions added

Overall Assessment: ⚠️ Needs Work

constructor-claude-bedrock · 2026-06-24T14:48:01Z

@@ -0,0 +1,428 @@
+/* eslint-disable max-len */
+/* eslint-disable camelcase, no-underscore-dangle, no-unneeded-ternary, brace-style */


Important Issue: no-underscore-dangle and no-unneeded-ternary appear in the blanket disable comment but neither pattern is actually present in this file. brace-style violations are also not visible. Remove the rules that aren't needed so future readers don't have to wonder why they were disabled.

Additionally, the max-len disable on line 1 is split across two separate eslint-disable comments, which is unconventional — they can be merged into one.

constructor-claude-bedrock · 2026-06-24T14:48:01Z

+    serviceUrl,
+    version,
+  } = options;
+  let queryParams = {


Important Issue: The c (client version) parameter is placed in queryParams before additionalQueryParams is spread onto it:

let queryParams = { c: version, ...additionalQueryParams, };

If additionalQueryParams ever contains a key c, it will silently override the version. Compare with createCatalogUrl in catalog.js, which spreads additionalQueryParams first and then sets c (and later key) unconditionally after the spread so that they cannot be overridden. Apply the same ordering here for safety.

constructor-claude-bedrock · 2026-06-24T14:48:01Z

+        queryParams.section = section;
+      }
+
+      if (id) {


Important Issue: Using a truthy check (if (id)) to decide whether to forward the id parameter will silently drop id: 0. While campaign IDs are unlikely to be 0 in practice, numeric 0 is falsy in JavaScript and would be excluded. The same pattern is repeated for page, offset, and numResultsPerPage. Use an explicit null/undefined check instead:

if (id != null) { queryParams.id = id; }

The same applies to page, offset, and numResultsPerPage.

constructor-claude-bedrock · 2026-06-24T14:48:01Z

+   *     section: 'Products',
+   * });
+   */
+  retrieveCampaign(parameters = {}, networkParameters = {}) {


Important Issue: retrieveCampaign does not validate that id is provided before using it in the URL path (campaigns/${id}). When id is undefined, the constructed URL becomes campaigns/undefined, which will result in a confusing 404 from the server rather than a clear client-side validation error. Add an explicit check and return a rejected promise (matching the pattern used for createCampaignsUrl path validation):

if (id == null) { return Promise.reject(new Error('id is a required parameter')); }

The same issue applies to updateCampaign and deleteCampaign.

constructor-claude-bedrock · 2026-06-24T14:48:01Z

+      }
+
+      return helpers.throwHttpErrorFromResponse(new Error(), response);
+    }).then((json) => json);


Suggestion: The final .then((json) => json) is a no-op — it returns the same value it receives and serves no purpose. This pattern is repeated across retrieveCampaigns, retrieveCampaign, createCampaign, and updateCampaign. Remove the redundant .then calls to reduce noise.

constructor-claude-bedrock · 2026-06-24T14:48:01Z

+  numResultsPerPage?: number;
+  page?: number;
+  offset?: number;
+  refinedRecommendationContexts?: RefinedRecommendationContext;


Important Issue: In RetrieveCampaignsParameters, refinedRecommendationContexts is typed as a single RefinedRecommendationContext object (with optional pod_id and condition_type arrays). However, in CampaignParametersBase (used for create/update), the same field is RecommendationContext[] — an array of objects each with pod_id and a condition discriminated union.

These two shapes are inconsistent with each other and with the JSDoc in campaigns.js, which describes the retrieve-side parameter as {object} (implying a filter map). Confirm the correct API contract and align the type with the actual parameter the backend accepts.

constructor-claude-bedrock · 2026-06-24T14:48:01Z

+  goal_description?: string;
+}
+
+export interface Campaign extends Record<string, any> {


Suggestion: Extending Record<string, any> on Campaign effectively widens the type to any for all property accesses not explicitly declared, defeating the purpose of the declared properties. If the intent is to allow additional undocumented server fields, consider using an index signature scoped to unknown values ([key: string]: unknown) or simply drop the extension if consumers should not rely on undeclared fields.

constructor-claude-bedrock · 2026-06-24T14:48:01Z

+
+    return fetch(requestUrl, {
+      method: 'POST',
+      body: JSON.stringify(helpers.toSnakeCaseKeys(body, false)),


Important Issue: toSnakeCaseKeys is called with toRecurse = false, meaning only the top-level keys of the request body are converted to snake_case. Nested camelCase keys inside objects like boostRules, slotRules, etc. will be sent as-is to the API. The JSDoc examples show camelCase input (e.g. boostRules). Verify whether the API expects fully snake_cased nested bodies; if so, pass true for toRecurse, or document clearly that callers must pass nested keys in snake_case already.

constructor-claude-bedrock · 2026-06-24T14:48:01Z

+    }
+  });
+
+  describe('createCampaign', () => {


Suggestion: createCampaignName is declared with const createCampaignName = \test-campaign-${Date.now()}`at thedescribeblock level.Date.now()is evaluated once when the module is first loaded, not per test run. Tests that use suffixes like-4, -5, -6to create uniquely-named campaigns will reuse the same base name across test retries, which can cause conflicts. Consider generating the timestamp inside abefore/beforeEach` hook or lazily within each test.

TarekAlQaddy

Thanks @esezen for making the changes 🙏

Outdated

Implement campaigns module with support for the Campaigns API

88a480d

Sher-Bakhodirov requested a review from Alexey-Pavlov June 5, 2026 11:55

Sher-Bakhodirov requested a review from a team as a code owner June 5, 2026 11:55

Copilot AI review requested due to automatic review settings June 5, 2026 11:55

Copilot started reviewing on behalf of Sher-Bakhodirov June 5, 2026 11:55 View session

This comment was marked as outdated.

Sign in to view

Sher-Bakhodirov requested a review from a team June 5, 2026 12:05

update delete campaign tests

7a929de

This comment was marked as outdated.

Sign in to view

Mudaafi previously requested changes Jun 11, 2026

View reviewed changes

esezen added 8 commits June 16, 2026 15:51

Move jsdocs

c39f99c

Update RecommendationContext

24d09a3

Update CampaignRule types

7f5bc29

Update CampaignRuleInput

b3d1005

Drop record

72f7c45

Update CampaignParametersBase

edc257e

Update test

cf831d8

Update type tests

30f4518

This comment was marked as outdated.

Sign in to view

Rename module

75c75db

This comment was marked as outdated.

Sign in to view

esezen requested a review from Mudaafi June 16, 2026 21:14

TarekAlQaddy reviewed Jun 23, 2026

View reviewed changes

esezen added 3 commits June 24, 2026 10:27

Add retrieveCampaigns query param tests and type tests

b481ff9

Cover query params extracted from parameters in retrieveCampaigns (id, refinedFilters, refinedRecommendationContexts, refinedQueries, plus their snake_case aliases) and add the missing searchandising type tests.

Type campaigns sub-module as an interface

25e7453

Use an interface for the campaigns sub-module so the type file keeps a single declared class, satisfying the max-classes-per-file lint rule that also runs over .d.ts files in the pre-push hook.

esezen requested a review from TarekAlQaddy June 24, 2026 14:47

constructor-claude-bedrock Bot reviewed Jun 24, 2026

View reviewed changes

TarekAlQaddy approved these changes Jun 24, 2026

View reviewed changes

esezen merged commit 9a64f2c into master Jun 25, 2026
5 of 8 checks passed

esezen deleted the cdx-225-node-sdk-support-for-campaigns-api branch June 25, 2026 18:39

		it('Should retrieve a list of campaigns given section', (done) => {
		const section = 'Products';

		@@ -0,0 +1,428 @@
		/* eslint-disable max-len */
		/* eslint-disable camelcase, no-underscore-dangle, no-unneeded-ternary, brace-style */

Uh oh!

Conversation

Sher-Bakhodirov commented Jun 5, 2026

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

Mudaafi left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

esezen Jun 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

TarekAlQaddy left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

esezen commented Jun 24, 2026

Uh oh!

constructor-claude-bedrock Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

constructor-claude-bedrock Bot Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jun 24, 2026

Choose a reason for hiding this comment

Uh oh!

constructor-claude-bedrock Bot Jun 24, 2026

esezen Jun 16, 2026 •

edited

Loading