Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions packages/client/src/clients/guide/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ export const predicateUrlRules = (
url: URL,
urlRules: GuideActivationUrlRuleData[],
) => {
const hasBlockRulesOnly = urlRules.every((r) => r.directive === "block");
const predicateDefault = hasBlockRulesOnly ? true : undefined;

return urlRules.reduce<boolean | undefined>((acc, urlRule) => {
// Any matched block rule prevails so no need to evaluate further
// as soon as there is one.
Expand All @@ -171,13 +174,18 @@ export const predicateUrlRules = (
return matched ? false : acc;
}
}
}, undefined);
}, predicateDefault);
};

export const predicateUrlPatterns = (
url: URL,
urlPatterns: KnockGuideActivationUrlPattern[],
) => {
const hasBlockPatternsOnly = urlPatterns.every(
(r) => r.directive === "block",
);
const predicateDefault = hasBlockPatternsOnly ? true : undefined;

return urlPatterns.reduce<boolean | undefined>((acc, urlPattern) => {
// Any matched block rule prevails so no need to evaluate further
// as soon as there is one.
Expand All @@ -203,5 +211,5 @@ export const predicateUrlPatterns = (
return matched ? false : acc;
}
}
}, undefined);
}, predicateDefault);
};
46 changes: 26 additions & 20 deletions packages/client/test/clients/guide/guide.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1098,8 +1098,9 @@ describe("KnockGuideClient", () => {
const client = new KnockGuideClient(mockKnock, channelId);
const result = client["_selectGuide"](stateWithGuides);

// Should include the guide with allow directive for /dashboard
expect(result!.key).toBe("onboarding");
// feature_tour has a block pattern for /settings, so it's allowed on /dashboard
// Since feature_tour comes first in display_sequence, it should be selected
expect(result!.key).toBe("feature_tour");
});

test("filters guides by url patterns - block directive", () => {
Expand Down Expand Up @@ -1152,8 +1153,9 @@ describe("KnockGuideClient", () => {
const client = new KnockGuideClient(mockKnock, channelId);
const result = client["_selectGuide"](stateWithGuides);

// Should include the guide with allow rule for /dashboard
expect(result!.key).toBe("onboarding");
// feature_tour still has a block pattern for /settings, so it's allowed on /dashboard
// Since feature_tour comes first in display_sequence, it should be selected
expect(result!.key).toBe("feature_tour");
});

test("filters guides by activation_url_rules - allow directive with contains", () => {
Expand Down Expand Up @@ -1187,8 +1189,9 @@ describe("KnockGuideClient", () => {
const client = new KnockGuideClient(mockKnock, channelId);
const result = client["_selectGuide"](stateWithGuides);

// Should include the guide with contains rule matching "dash"
expect(result!.key).toBe("onboarding");
// feature_tour still has a block pattern for /settings, so it's allowed on /dashboard
// Since feature_tour comes first in display_sequence, it should be selected
expect(result!.key).toBe("feature_tour");
});

test("filters guides by activation_url_rules - block directive with equal_to", () => {
Expand Down Expand Up @@ -1298,11 +1301,10 @@ describe("KnockGuideClient", () => {
const client = new KnockGuideClient(mockKnock, channelId);
const result = client["_selectGuide"](stateWithGuides);

// Block rule should prevail even if allow rule also matches
// feature_tour is excluded because it has url_patterns that don't match this location
// onboarding is excluded because block rule prevails
// system_status has no rules and is included
expect(result!.key).toBe("system_status");
// feature_tour has a block pattern for "/settings" which doesn't match "/admin/settings"
// Since it only has block patterns and doesn't match, it defaults to allowed
// feature_tour comes first in display_sequence, so it gets selected
expect(result!.key).toBe("feature_tour");
});

test("filters guides by activation_url_rules - multiple allow rules (any match allows)", () => {
Expand Down Expand Up @@ -1342,8 +1344,10 @@ describe("KnockGuideClient", () => {
const client = new KnockGuideClient(mockKnock, channelId);
const result = client["_selectGuide"](stateWithGuides);

// Should allow the guide when any allow rule matches
expect(result!.key).toBe("onboarding");
// feature_tour has a block pattern for "/settings" which doesn't match "/settings/profile"
// Since it only has block patterns and doesn't match, it defaults to allowed
// feature_tour comes first in display_sequence, so it gets selected
expect(result!.key).toBe("feature_tour");
});

test("filters guides by activation_url_rules - handles leading slash in arguments", () => {
Expand Down Expand Up @@ -1377,8 +1381,10 @@ describe("KnockGuideClient", () => {
const client = new KnockGuideClient(mockKnock, channelId);
const result = client["_selectGuide"](stateWithGuides);

// Should handle argument without leading slash correctly
expect(result!.key).toBe("onboarding");
// feature_tour has a block pattern for "/settings" which doesn't match "/dashboard"
// Since it only has block patterns and doesn't match, it defaults to allowed
// feature_tour comes first in display_sequence, so it gets selected
expect(result!.key).toBe("feature_tour");
});

test("filters guides by activation_url_rules - no match when url rules don't match", () => {
Expand Down Expand Up @@ -1412,11 +1418,11 @@ describe("KnockGuideClient", () => {
const client = new KnockGuideClient(mockKnock, channelId);
const result = client["_selectGuide"](stateWithGuides);

// Should not match the guide when url rules don't match
// feature_tour is excluded because it has url_patterns that don't match this location
// onboarding is excluded because its url_rules don't match
// system_status has no rules and is included
expect(result!.key).toBe("system_status");
// feature_tour has a block pattern for "/settings" which doesn't match "/dashboard"
// Since it only has block patterns and doesn't match, it defaults to allowed
// onboarding is excluded because its url_rules require "/admin" but location is "/dashboard"
// feature_tour comes first in display_sequence, so it gets selected
expect(result!.key).toBe("feature_tour");
});

test("handles guides without location when location is undefined", () => {
Expand Down
Loading
Loading