Skip to content

Conversation

@singhkunal2050
Copy link
Collaborator

@singhkunal2050 singhkunal2050 commented Sep 12, 2025

Changes

Fixed localstorage issue faced due to https://knowledge.clevertap.net/t/wzrk-camp-limit-exceed/5655

Changes to Public Facing API if any

  1. removed getAllQualifiedCampaignDetails

How Has This Been Tested?

  1. Ensured WZRK_QC key is not updated now and in case of any issues

Checklist

  • Code compiles without errors
  • Version Bump added to package.json & CHANGELOG.md
  • All tests pass
  • Build process is successful
  • Documentation has been updated (if needed)
  • Automation tests are passing

Link to Deployed SDK

Use these url for testing :

  1. https://static.wizrocket.com/staging/<CURRENT_BRANCH_NAME>/js/clevertap.min.js
  2. https://static.wizrocket.com/staging/<CURRENT_BRANCH_NAME>/js/sw_webpush.min.js

How to trigger Automations

Just add a empty commit after all your changes are done in the PR with the command

 git commit --allow-empty -m "[run-test] Testing Automation"

This will trigger the automation suite

Summary by CodeRabbit

  • Refactor
    • Removed local storage of qualified campaign data and the ability to retrieve it via the public API.
  • Documentation
    • Updated changelog with version 2.2.3 (dated 12 Sept 2025) noting removal of Campaigns details from LC.
  • Chores
    • Bumped SDK version to 2.2.3.

@coderabbitai
Copy link

coderabbitai bot commented Sep 12, 2025

Walkthrough

Removes all code paths and public APIs related to storing and retrieving “qualified campaigns” from local storage, deletes the associated constant, and strips related calls. Bumps SDK version to 2.2.3 and updates CHANGELOG and package.json. No new public entities are added.

Changes

Cohort / File(s) Summary of Changes
Version bump & metadata
CHANGELOG.md, package.json, clevertap.js
Added 2.2.3 entry to changelog; bumped package version to 2.2.3; updated getSDKVersion() and internal reference from web-sdk-v2.2.2 to web-sdk-v2.2.3.
Remove qualified-campaigns storage API & usage
src/util/constants.js, src/util/campaignRender/utilities.js, src/util/campaignHouseKeeping/commonCampaignUtils.js, src/clevertap.js, clevertap.js
Deleted QUALIFIED_CAMPAIGNS constant; removed addCampaignToLocalStorage export and all invocations; removed CleverTap.getAllQualifiedCampaignDetails() public method; pruned related imports and loops that persisted campaigns.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor App
  participant SDK as Web SDK
  participant Storage as Local Storage

  rect rgba(230,245,255,0.6)
  note over SDK: Previous flow (<= 2.2.2)
  App->>SDK: Evaluate campaign(s)
  SDK->>Storage: Persist qualified campaign details (WZRK_QC)
  App->>SDK: getAllQualifiedCampaignDetails()
  SDK->>Storage: Read WZRK_QC
  Storage-->>SDK: Campaign list
  SDK-->>App: Campaign list
  end

  rect rgba(240,240,240,0.6)
  note over SDK: New flow (2.2.3)
  App->>SDK: Evaluate campaign(s)
  SDK--x Storage: (No persistence)
  App--xSDK: getAllQualifiedCampaignDetails() (removed)
  note over App,SDK: Qualified campaigns are not stored or retrievable via API
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Suggested reviewers

  • kkyusuftk

Pre-merge checks (1 passed, 1 warning, 1 inconclusive)

❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
Title Check ❓ Inconclusive The title "Removed Campaign details from LC" reflects the PR's intent to stop persisting qualified campaign details (removal of QUALIFIED_CAMPAIGNS/WZRK_QC, addCampaignToLocalStorage, and getAllQualifiedCampaignDetails) but uses the ambiguous abbreviation "LC" and does not explicitly name the affected storage key or removed public API, which reduces clarity for reviewers scanning history. Please update the title to be explicit and concise, for example "Remove qualified campaign details from localStorage (WZRK_QC) and drop getAllQualifiedCampaignDetails", and include the Jira/issue reference if available so the change is immediately clear in history.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed The PR description largely follows the repository template and documents the change, the public API impact (removed getAllQualifiedCampaignDetails), testing note, checklist, deploy URLs, and automation instructions, but it is missing a Jira issue reference and the "How Has This Been Tested?" section is minimal and should include concrete steps, environments, and observed outcomes; overall the description is mostly complete.

Poem

I nibbled the bytes, light and spry—
Out hop the campaigns, we wave goodbye.
No crumbs in storage, clean as dew,
Version hops to two-two-three anew.
Thump-thump! I sign the changelog page,
A tidy warren for the codey stage. 🐇✨

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.38.6)
clevertap.js

Tip

👮 Agentic pre-merge checks are now available in preview!

Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.

  • Built-in checks – Quickly apply ready-made checks to enforce title conventions, require pull request descriptions that follow templates, validate linked issues for compliance, and more.
  • Custom agentic checks – Define your own rules using CodeRabbit’s advanced agentic capabilities to enforce organization-specific policies and workflows. For example, you can instruct CodeRabbit’s agent to verify that API documentation is updated whenever API schema files are modified in a PR. Note: Upto 5 custom checks are currently allowed during the preview period. Pricing for this feature will be announced in a few weeks.

Please see the documentation for more information.

Example:

reviews:
  pre_merge_checks:
    custom_checks:
      - name: "Undocumented Breaking Changes"
        mode: "warning"
        instructions: |
          Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).

Please share your feedback with us on this Discord post.

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch local-storage-fix

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@francispereira
Copy link

francispereira commented Sep 12, 2025

🎉 Snyk checks have passed. No issues have been found so far.

security/snyk check is complete. No issues have been found. (View Details)

license/snyk check is complete. No issues have been found. (View Details)

code/snyk check is complete. No issues have been found. (View Details)

@singhkunal2050 singhkunal2050 marked this pull request as ready for review September 12, 2025 08:30
@singhkunal2050 singhkunal2050 requested a review from a team as a code owner September 12, 2025 08:30
@singhkunal2050 singhkunal2050 changed the title Fixed Local storage Removed Campaign details from LC Sep 12, 2025
@singhkunal2050 singhkunal2050 changed the base branch from master to develop September 12, 2025 08:36
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
clevertap.js (1)

17974-17989: Clean up legacy WZRK_QC key on init to prevent future bloat

The PR removes QUALIFIED_CAMPAIGNS/WZRK_QC usage, but any existing WZRK_QC entry in localStorage will persist (and can still grow if older pages write to it). Proactively delete it during init to stabilize storage size.

       StorageManager.removeCookie('WZRK_P', window.location.hostname);
+      // Remove legacy qualified-campaigns cache key to prevent storage bloat
+      try {
+        if (StorageManager._isLocalStorageSupported()) {
+          localStorage.removeItem('WZRK_QC');
+        }
+      } catch (_) { /* ignore */ }
🧹 Nitpick comments (3)
CHANGELOG.md (1)

4-6: Clarify the entry and flag the public API removal as breaking

Spell out “localStorage” and name the key/API so upgraders know what changed. Example:

-## [2.2.3] 12th Sept 2025
-- Removed Campaigns details from LC
+## [2.2.3] 12 Sept 2025
+- Removed storing “Qualified Campaigns” details (WZRK_QC) in localStorage.
+- Removed public API `getAllQualifiedCampaignDetails` (breaking).
+- Behavior: SDK no longer reads/writes WZRK_QC; existing data is ignored.

I can add a migration note or a one-line deprecation shim suggestion if you want.

src/util/campaignRender/utilities.js (1)

13-14: Optional: proactively clean up stale WZRK_QC once at init

Since we’ve stopped persisting qualified campaigns, consider removing any leftover WZRK_QC to avoid confusion during debugging.

Outside this file (e.g., SDK init), add:

try { localStorage.removeItem('WZRK_QC'); } catch {}
clevertap.js (1)

13820-13833: Unify SDK version format across code paths

The postMessage sends '2.2.3' while telemetry uses 'web-sdk-v2.2.3'. If external tools compare these, prefer a single format, e.g., always send raw '2.2.3' and derive 'web-sdk-v...' where required.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d773ca1 and 63e89ab.

⛔ Files ignored due to path filters (2)
  • clevertap.js.map is excluded by !**/*.map
  • clevertap.min.js is excluded by !**/*.min.js
📒 Files selected for processing (7)
  • CHANGELOG.md (1 hunks)
  • clevertap.js (3 hunks)
  • package.json (1 hunks)
  • src/clevertap.js (0 hunks)
  • src/util/campaignHouseKeeping/commonCampaignUtils.js (1 hunks)
  • src/util/campaignRender/utilities.js (1 hunks)
  • src/util/constants.js (0 hunks)
💤 Files with no reviewable changes (2)
  • src/util/constants.js
  • src/clevertap.js
🧰 Additional context used
🧬 Code graph analysis (1)
clevertap.js (1)
src/modules/visualBuilder/pageBuilder.js (1)
  • sdkVersion (33-33)
🔇 Additional comments (4)
package.json (1)

3-3: Breaking change — removed public API getAllQualifiedCampaignDetails: add a shim or mark release as BREAKING and bump version

CHANGELOG.md still mentions the API (CHANGELOG.md:67). Verification run skipped files, so absence of callers/version drift could not be confirmed.

  • Action: either add a backward-compatible deprecation shim for getAllQualifiedCampaignDetails OR mark this release as BREAKING in CHANGELOG and bump the major version (e.g., 2.2.3 → 3.0.0).
  • Verify: run these checks and fix any remaining references/version strings:
    • rg -nP '\bgetAllQualifiedCampaignDetails\b|QUALIFIED_CAMPAIGNS|WZRK_QC' -uu
    • rg -n '2.2.2' -uu -g '!/dist/' -g '!/build/'
    • cat package.json | jq -r .version
    • rg -nP 'getSDKVersion|web-sdk-v2.2.3' -uu src || true
src/util/campaignRender/utilities.js (1)

13-14: Export verified — no action required

WEB_NATIVE_DISPLAY_VISUAL_EDITOR_TYPES is exported in src/util/constants (line 87).

src/util/campaignHouseKeeping/commonCampaignUtils.js (1)

53-54: ```shell
#!/bin/bash
set -euo pipefail
echo "== search: addCampaignToLocalStorage (word) =="
rg -n --hidden -S '\baddCampaignToLocalStorage\b' || true
echo "== search: addCampaignToLocalStorage (call) =="
rg -n --hidden -S '\baddCampaignToLocalStorage\s*(' || true
echo "== search: case-insensitive =="
rg -n --hidden -S -i 'addcampaigntolocalstorage' || true
echo "== search: partial token =="
rg -n --hidden -S 'addCampaignToLocalStor' || true
echo "== search: WZRK_QC =="
rg -n --hidden -S '\bWZRK_QC\b' || true
echo "== search: WZRK_Q =="
rg -n --hidden -S 'WZRK_Q' || true
echo "== search: webNativeDisplayCampaignUtils =="
rg -n --hidden -S 'webNativeDisplayCampaignUtils' || true
echo "== file: src/util/campaignHouseKeeping/commonCampaignUtils.js (head 160 lines) =="
if [ -f src/util/campaignHouseKeeping/commonCampaignUtils.js ]; then
sed -n '1,160p' src/util/campaignHouseKeeping/commonCampaignUtils.js || true
else
echo "file not found"
fi


</blockquote></details>
<details>
<summary>clevertap.js (1)</summary><blockquote>

`1-1`: **Breaking change — public API removed but package bumped as patch (version 2.2.3)**

getAllQualifiedCampaignDetails has been removed from the codebase; package.json is "version": "2.2.3" and CHANGELOG.md lists [2.2.3] (12 Sep 2025). Removing a public API is a breaking change — a patch bump is misleading.

- Fix (pick one):
  - Restore a backward-compatible deprecated shim that logs a warning and returns an empty result for at least one minor release, or
  - Bump the minor/major version and document the removal in CHANGELOG + migration notes.

Suggested shim:
```diff
/**
 * @deprecated Removed. No replacement; qualified campaigns are no longer stored.
 */
getAllQualifiedCampaignDetails() {
  this._logger?.debug('getAllQualifiedCampaignDetails is deprecated and returns an empty result.');
  return {};
}

if (parentWindow) {
logger.debug('SDK version check');
const sdkVersion = '2.2.2';
const sdkVersion = '2.2.3';
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Avoid hard-coded SDK version; derive from a single source of truth

The string literal '2.2.3' here will drift from other places. Introduce a single SDK_VERSION constant and reuse it for the dashboard postMessage, data.af.lib, and getSDKVersion().

Apply something like:

+// At top-level (near other constants)
+const SDK_VERSION = '2.2.3';

 // inside SDK check
-const sdkVersion = '2.2.3';
+const sdkVersion = SDK_VERSION;

And below (see other comments) derive "web-sdk-v..." from SDK_VERSION instead of embedding more literals.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const sdkVersion = '2.2.3';
const SDK_VERSION = '2.2.3';
const sdkVersion = SDK_VERSION;
🤖 Prompt for AI Agents
In clevertap.js around line 13826, replace the hard-coded string '2.2.3' with a
single SDK_VERSION constant defined near the top of the module and reuse that
constant wherever the SDK version is needed (dashboard postMessage payload,
data.af.lib assignment, and the getSDKVersion() return); also compute the
derived identifier "web-sdk-v..." by prefixing the SDK_VERSION constant instead
of embedding literals so all version uses come from the single source of truth.

Comment on lines +16298 to 16301
lib: 'web-sdk-v2.2.3',
protocol: proto,
...$ct.flutterVersion
}; // app fields
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

DRY the SDK version in telemetry payload

data.af.lib repeats a versioned string. Build it from a single SDK_VERSION constant to prevent mismatches with other locations (handleActionMode and getSDKVersion).

-        lib: 'web-sdk-v2.2.3',
+        lib: `web-sdk-v${SDK_VERSION}`,
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
lib: 'web-sdk-v2.2.3',
protocol: proto,
...$ct.flutterVersion
}; // app fields
lib: `web-sdk-v${SDK_VERSION}`,
protocol: proto,
...$ct.flutterVersion
}; // app fields
🤖 Prompt for AI Agents
In clevertap.js around lines 16298 to 16301, the telemetry payload sets
data.af.lib to a hardcoded 'web-sdk-v2.2.3', which duplicates the SDK version
string used elsewhere (handleActionMode and getSDKVersion); define a single
SDK_VERSION constant (e.g., const SDK_VERSION = 'web-sdk-v2.2.3') at module
scope and replace all hardcoded occurrences (including data.af.lib,
handleActionMode, and getSDKVersion) to reference that constant so the version
is maintained in one place.

Comment on lines +18220 to 18221
return 'web-sdk-v2.2.3';
}
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Expose getSDKVersion from the same constant

Return the same version used elsewhere; avoid another hard-coded literal.

-      return 'web-sdk-v2.2.3';
+      return `web-sdk-v${SDK_VERSION}`;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return 'web-sdk-v2.2.3';
}
return `web-sdk-v${SDK_VERSION}`;
}
🤖 Prompt for AI Agents
In clevertap.js around lines 18220-18221, the function currently returns a
hard-coded 'web-sdk-v2.2.3'; instead return the existing shared SDK version
constant used elsewhere (e.g., SDK_VERSION or exported getSDKVersion constant)
to avoid duplicating literals — import or reference that constant and return it
here so the same version value is used across the codebase.

@singhkunal2050 singhkunal2050 marked this pull request as draft September 12, 2025 10:18
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.

3 participants