feat: adding post messages to link unlinked ct to variant group by karancs06 · Pull Request #514 · contentstack/live-preview-sdk

karancs06 · 2025-10-10T09:46:07Z

Screen.Recording.2025-10-14.at.3.36.48.PM.mov

github-actions · 2025-10-10T10:30:20Z

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	72.6%	9531 / 13127
🔵	Statements	72.6%	9531 / 13127
🔵	Functions	71.51%	339 / 474
🔵	Branches	84.05%	1207 / 1436

File Coverage

File	Stmts	Branches	Functions	Lines	Uncovered Lines
Changed Files
src/visualBuilder/index.ts	56.87%	66.66%	42.85%	56.87%	98-105, 108-112, 115-127, 130-203, 209-241, 249-271, 322-325, 328-329, 379-385
src/visualBuilder/visualBuilder.style.ts	100%	100%	100%	100%
src/visualBuilder/components/fieldLabelWrapper.tsx	70.15%	57.44%	72.72%	70.15%	48-50, 58-60, 142-153, 156-160, 179-208, 221-252, 277-278, 282-287, 291, 356-358, 364-382, 387-399
src/visualBuilder/eventManager/useRevalidateFieldDataPostMessageEvent.ts	100%	91.3%	80%	100%
src/visualBuilder/utils/constants.ts	100%	100%	100%	100%
src/visualBuilder/utils/fieldSchemaMap.ts	94.73%	100%	87.5%	94.73%	98-100
src/visualBuilder/utils/isFieldDisabled.ts	98.44%	96.07%	100%	98.44%	129-130
src/visualBuilder/utils/types/index.types.ts	100%	100%	100%	100%
src/visualBuilder/utils/types/postMessage.types.ts	100%	100%	100%	100%

Generated in workflow #614 for commit 388370e by the Vitest Coverage Report Action

csAyushDubey

Left a few comments

csAyushDubey

LGTM

hiteshshetty-dev · 2025-10-30T13:10:59Z

+                                {(() => {
+                                    const [before, after] = reason.split(
+                                        /here/i
+                                    );


We might not need regex here.

@karancs06 - I missed this in my earlier review but why do we need regex here?

we have changed that while changing to constants

…variant-group

hiteshshetty-dev

LGTM!!

prathamesh-sawant-cs · 2025-11-06T06:46:44Z


+            const handleLinkVariant = async () => {
+                if (fieldSchema.field_metadata?.canLinkVariant) {
+                    try {


The check can be done inside the try block.

prathamesh-sawant-cs · 2025-11-06T06:47:33Z

+                if (fieldSchema.field_metadata?.canLinkVariant) {
+                    try {
+                        const result = await visualBuilderPostMessage?.send<{
+                            type: "success" | "error";


Create constants for both success & error.

prathamesh-sawant-cs · 2025-11-06T06:48:12Z

+                        }
+
+                        // If linking was successful and requires revalidation, revalidate
+                        if (result.type === "success") {


reuse the success constant here instead of creating a new string.

prathamesh-sawant-cs · 2025-11-06T06:48:22Z

+                        );
+
+                        // If the modal was closed or linking failed, do nothing
+                        if (!result || result.type === "error") {


reuse the error constant here instead of creating a new string.

prathamesh-sawant-cs · 2025-11-06T06:48:35Z

+                        if (result.type === "success") {
+                            await handleRevalidateFieldData();
+                        }
+                    } catch (message) {


Suggested change

} catch (message) {

} catch (error) {

prathamesh-sawant-cs · 2025-11-06T07:23:55Z

+    let overlayWrapper: HTMLDivElement;
+    let focusedToolbar: HTMLDivElement;
+
+    beforeEach(() => {


Do we need this before every test case?
Can the test cases be refactored to avoid doing this before every test and instead do it once for the class?

prathamesh-sawant-cs · 2025-11-06T07:28:47Z

+        focusedElement?.getAttribute("data-cslp-unique-id") || null;
+    const shouldRefocus = !!focusedElement;
+
+    try {


Having multiple nested try catch blocks is a bad practice and creates code readability issues.
Please refactor this code.

prathamesh-sawant-cs · 2025-11-06T07:34:06Z

        expect(result.isDisabled).toBe(true);
        expect(result.reason).toBe(
-            "This field is not editable as it is not linked to the selected variant"
+            "This field is not editable as it is not linked to the selected variant. Contact your stack admin or owner to link it."


I would expect all these messages to come from constants. I have already mentioned the benefits of doing so in the above comments.

prathamesh-sawant-cs · 2025-11-06T07:35:49Z

+    if (flags.updateRestrictDueToUnlinkVariant) {
+        let reason = DisableReason.UnlinkedVariant;
+        if (flags.canLinkVariant) {
+            reason += ` ${DisableReason.CanLinkVaraint}`;


NIT: Typo

Suggested change

reason += ` ${DisableReason.CanLinkVaraint}`;

reason += ` ${DisableReason.CanLinkVariant}`;

prathamesh-sawant-cs · 2025-11-06T07:36:33Z

-        return DisableReason.UnlinkedVariant;
+    if (flags.updateRestrictDueToUnlinkVariant) {
+        let reason = DisableReason.UnlinkedVariant;
+        if (flags.canLinkVariant) {


String concatenation logic can be improved & made cleaner:

return flags.canLinkVariant ? `${DisableReason.UnlinkedVariant} ${DisableReason.CanLinkVariant}` : `${DisableReason.UnlinkedVariant} ${DisableReason.CannotLinkVariant}`;

prathamesh-sawant-cs · 2025-11-11T12:59:32Z

+                                {(() => {
+                                    const [before, after] = reason.split(
+                                        /here/i
+                                    );


@karancs06 - I missed this in my earlier review but why do we need regex here?

prathamesh-sawant-cs · 2025-11-11T13:01:38Z

            expect(result.reason).toBe(
-                "You do not have Edit access to this entry on the 'Review Stage' workflow stage"
+                DisableReason.WorkflowStagePermission({
+                    stageName: "Review Stage",


stageNames can be come from an ENUM or constant file.

prathamesh-sawant-cs · 2025-11-11T13:01:50Z

            expect(result.reason).toBe(
-                "Editing is restricted for your role or by the rules for the 'Final Review' stage. Contact your admin for edit access."
+                DisableReason.EntryUpdateRestrictedRoleAndWorkflowStage({
+                    stageName: "Final Review",


Same as above.

prathamesh-sawant-cs · 2025-11-11T13:02:18Z

            );
            expect(result.isDisabled).toBe(false);
-            expect(result.reason).toBe("");
+            expect(result.reason).toBe(DisableReason.None);


Do we really need a constant for a blank string.

prathamesh-sawant-cs · 2025-11-11T13:02:36Z

            expect(result.isDisabled).toBe(true);
            expect(result.reason).toBe(
-                "You do not have Edit access to this entry on the 'Unknown' workflow stage"
+                DisableReason.WorkflowStagePermission({ stageName: "Unknown" })


const REVIEW_STAGES = Object.freeze({ UNKNOWN: 'Unknown', FINAL_REVIEW: 'Final Review', IN_REVIEW: 'In Review', // add other values as needed. });

prathamesh-sawant-cs · 2025-11-11T13:06:52Z


 export const DATA_CSLP_ATTR_SELECTOR = "data-cslp";

+export const RESULT_TYPES = {


export const RESULT_TYPES = Object.freeze({ SUCCESS: 'success', ERROR: 'error', });

prathamesh-sawant-cs · 2025-11-11T13:08:51Z

 import { WorkflowStageDetails } from "./getWorkflowStageDetails";

-const DisableReason = {
+export const DisableReason = {


Constants should be named in UPPER CASE.

Suggested change

export const DisableReason = {

export const REASON_FOR_DISABLING = {

prathamesh-sawant-cs · 2025-11-11T13:09:03Z


-const DisableReason = {
+export const DisableReason = {
    ReadOnly: "You have only read access to this field",


cannot change this constant as it would change a lot in the code base and also edit files are not intended to be editted in this pr

prathamesh-sawant-cs · 2025-11-11T13:09:35Z

-const DisableReason = {
+export const DisableReason = {
    ReadOnly: "You have only read access to this field",
    LocalizedEntry: "Editing this field is restricted in localized entries",


All constants to be written in UPPERCASE.

Suggested change

LocalizedEntry: "Editing this field is restricted in localized entries",

LOCALIZED_ENTRY: "Editing this field is restricted in localized entries",

cannot change this constant as it would change a lot in the code base and also edit files are not intended to be editted in this pr

prathamesh-sawant-cs · 2025-11-11T13:09:57Z

        "This field is not editable as it is not linked to the selected variant.",
-    CanLinkVaraint: "Click here to link a variant",
+    CanLinkVariant: "Click here to link a variant",
+    SplitOn: "here",


Rename this to something more meaningful.

…variant-group

feat: adding post messages to link unlinked ct to variant group

30975d3

karancs06 requested a review from a team as a code owner October 10, 2025 09:46

fix: updated console

0817211

karancs06 requested a review from hiteshshetty-dev October 10, 2025 09:50

feat: test cases added

6936fef

csAyushDubey suggested changes Oct 14, 2025

View reviewed changes

Base automatically changed from develop_v4 to stage_v4 October 14, 2025 10:07

karancs06 added 2 commits October 14, 2025 16:00

fix: string and focus unfocus on variant link

17e8f5d

fix: addressed changes

8f2da6e

karancs06 requested a review from csAyushDubey October 14, 2025 11:59

karancs06 changed the base branch from stage_v4 to develop_v4 October 15, 2025 04:40

csAyushDubey approved these changes Oct 16, 2025

View reviewed changes

fix: removing revalidate post message

9e568d8

hiteshshetty-dev reviewed Oct 30, 2025

View reviewed changes

karancs06 added 3 commits October 30, 2025 20:47

Merge branch 'develop_v4' into VB-530-support-to-link-unlinked-ct-to-…

b84c08a

…variant-group

fix: requested changes done

cb98dad

fix: response of postmessage changed

1ba39e6

karancs06 requested a review from prathamesh-sawant-cs November 3, 2025 09:44

karancs06 added 2 commits November 3, 2025 15:51

fix: remove console

13cb10f

fix: formatting removed

300b479

hiteshshetty-dev approved these changes Nov 4, 2025

View reviewed changes

prathamesh-sawant-cs suggested changes Nov 6, 2025

View reviewed changes

fix: changes requested

ad51ff2

karancs06 requested a review from prathamesh-sawant-cs November 7, 2025 12:49

fix: lowercase

2628269

prathamesh-sawant-cs suggested changes Nov 11, 2025

View reviewed changes

fix: object freeze

c0d9611

karancs06 requested a review from prathamesh-sawant-cs November 12, 2025 03:31

prathamesh-sawant-cs approved these changes Nov 13, 2025

View reviewed changes

Merge branch 'develop_v4' into VB-530-support-to-link-unlinked-ct-to-…

388370e

…variant-group

karancs06 merged commit 4c9d670 into develop_v4 Nov 13, 2025
10 checks passed

karancs06 deleted the VB-530-support-to-link-unlinked-ct-to-variant-group branch November 13, 2025 07:46

hiteshshetty-dev restored the VB-530-support-to-link-unlinked-ct-to-variant-group branch December 5, 2025 08:14

	reason += ` ${DisableReason.CanLinkVaraint}`;
	reason += ` ${DisableReason.CanLinkVariant}`;


		export const DATA_CSLP_ATTR_SELECTOR = "data-cslp";

		export const RESULT_TYPES = {

	export const DisableReason = {
	export const REASON_FOR_DISABLING = {

	LocalizedEntry: "Editing this field is restricted in localized entries",
	LOCALIZED_ENTRY: "Editing this field is restricted in localized entries",

Conversation

karancs06 commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented Oct 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Coverage Report

Uh oh!

csAyushDubey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

csAyushDubey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

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!

Uh oh!

hiteshshetty-dev 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!

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!

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!

Choose a reason for hiding this comment

karancs06 commented Oct 10, 2025 •

edited

Loading

github-actions Bot commented Oct 10, 2025 •

edited

Loading