Skip to content

Commit 4ca8d5d

Browse files
floooatclaude
andcommitted
fix: evaluate exclusion page rules with AND, not OR
Multiple "does not contain" / "is not" page rules were OR-joined, making any multi-page exclusion a tautology (a URL can't contain all of them) so the pop-up/tooltip always showed. checkPageRules now splits rules by polarity: positive rules (contains/is/startswith/endswith) OR as an allow-list, negative rules (notcontains/isnot) AND as a deny-list. Adds GleapPageFilter.test.js covering single/positive/negative/mixed configs. Ref ticket #137150. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 2f1e722 commit 4ca8d5d

2 files changed

Lines changed: 144 additions & 1 deletion

File tree

src/GleapPageFilter.js

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,10 +82,31 @@ export const checkPageFilter = (currentUrl, pageFilter, pageFilterType) => {
8282
return matched;
8383
};
8484

85+
// Exclusion (negative) rule types. These express "hide here" and must all
86+
// hold simultaneously, so they are combined with AND. Everything else is a
87+
// positive "show here" rule and is combined with OR.
88+
const NEGATIVE_FILTER_TYPES = ['notcontains', 'isnot'];
89+
8590
export const checkPageRules = (currentUrl, action) => {
8691
const rules = action.pageRules && action.pageRules.length > 0
8792
? action.pageRules
8893
: (action.pageFilter ? [{ pageFilter: action.pageFilter, pageFilterType: action.pageFilterType }] : []);
8994
if (rules.length === 0) return true;
90-
return rules.some(r => checkPageFilter(currentUrl, r.pageFilter, r.pageFilterType));
95+
96+
const positiveRules = rules.filter(r => !NEGATIVE_FILTER_TYPES.includes(r.pageFilterType));
97+
const negativeRules = rules.filter(r => NEGATIVE_FILTER_TYPES.includes(r.pageFilterType));
98+
99+
// Positive rules are an allow-list: show if the page matches ANY of them.
100+
const positivesPass = positiveRules.length === 0
101+
? true
102+
: positiveRules.some(r => checkPageFilter(currentUrl, r.pageFilter, r.pageFilterType));
103+
104+
// Negative rules are a deny-list: only show if the page passes EVERY
105+
// exclusion. Combining exclusions with OR (the previous behaviour) made any
106+
// multi-page exclusion a tautology that always resolved to "show".
107+
const negativesPass = negativeRules.length === 0
108+
? true
109+
: negativeRules.every(r => checkPageFilter(currentUrl, r.pageFilter, r.pageFilterType));
110+
111+
return positivesPass && negativesPass;
91112
};

src/GleapPageFilter.test.js

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,122 @@
1+
import { checkPageRules, checkPageFilter } from './GleapPageFilter';
2+
3+
const url = (path) => `https://app.example.com${path}`;
4+
5+
describe('checkPageFilter (single condition primitives)', () => {
6+
test('missing params pass (lenient)', () => {
7+
expect(checkPageFilter('', '/x', 'contains')).toBe(true);
8+
expect(checkPageFilter(url('/x'), '', 'contains')).toBe(true);
9+
expect(checkPageFilter(url('/x'), '/x', '')).toBe(true);
10+
});
11+
12+
test('contains / notcontains', () => {
13+
expect(checkPageFilter(url('/dashboard'), '/dashboard', 'contains')).toBe(true);
14+
expect(checkPageFilter(url('/dashboard'), '/onboarding', 'contains')).toBe(false);
15+
expect(checkPageFilter(url('/dashboard'), '/onboarding', 'notcontains')).toBe(true);
16+
expect(checkPageFilter(url('/onboarding'), '/onboarding', 'notcontains')).toBe(false);
17+
});
18+
19+
test('is / isnot use exact (trailing-slash-insensitive) match', () => {
20+
expect(checkPageFilter(url('/a/'), url('/a'), 'is')).toBe(true);
21+
expect(checkPageFilter(url('/a'), url('/b'), 'isnot')).toBe(true);
22+
expect(checkPageFilter(url('/a'), url('/a'), 'isnot')).toBe(false);
23+
});
24+
25+
test('unknown / unsupported types resolve to false', () => {
26+
expect(checkPageFilter(url('/a'), '/a', 'empty')).toBe(false);
27+
expect(checkPageFilter(url('/a'), '/a', 'notempty')).toBe(false);
28+
});
29+
});
30+
31+
describe('checkPageRules — no-op / single-rule (must be unchanged by the fix)', () => {
32+
test('no rules => show', () => {
33+
expect(checkPageRules(url('/anything'), {})).toBe(true);
34+
expect(checkPageRules(url('/anything'), { pageRules: [] })).toBe(true);
35+
});
36+
37+
test('legacy single pageFilter fallback', () => {
38+
expect(checkPageRules(url('/onboarding'), { pageFilter: '/onboarding', pageFilterType: 'notcontains' })).toBe(false);
39+
expect(checkPageRules(url('/dashboard'), { pageFilter: '/onboarding', pageFilterType: 'notcontains' })).toBe(true);
40+
});
41+
42+
test('single positive rule', () => {
43+
expect(checkPageRules(url('/members/home'), { pageRules: [{ pageFilter: 'members', pageFilterType: 'contains' }] })).toBe(true);
44+
expect(checkPageRules(url('/public'), { pageRules: [{ pageFilter: 'members', pageFilterType: 'contains' }] })).toBe(false);
45+
});
46+
47+
test('single negative rule', () => {
48+
expect(checkPageRules(url('/onboarding'), { pageRules: [{ pageFilter: '/onboarding', pageFilterType: 'notcontains' }] })).toBe(false);
49+
expect(checkPageRules(url('/dashboard'), { pageRules: [{ pageFilter: '/onboarding', pageFilterType: 'notcontains' }] })).toBe(true);
50+
});
51+
});
52+
53+
describe('checkPageRules — all-positive multi-rule (OR preserved, no regression)', () => {
54+
const rules = [
55+
{ pageFilter: '/dashboard', pageFilterType: 'contains' },
56+
{ pageFilter: '/settings', pageFilterType: 'contains' },
57+
];
58+
test('shows if ANY positive matches', () => {
59+
expect(checkPageRules(url('/dashboard'), { pageRules: rules })).toBe(true);
60+
expect(checkPageRules(url('/settings'), { pageRules: rules })).toBe(true);
61+
});
62+
test('hidden if NONE match', () => {
63+
expect(checkPageRules(url('/billing'), { pageRules: rules })).toBe(false);
64+
});
65+
});
66+
67+
describe('checkPageRules — all-negative multi-rule (THE BUG: was tautology, now AND)', () => {
68+
// Ticket #137150 ROASForm config.
69+
const rules = [
70+
{ pageFilter: '/signup', pageFilterType: 'notcontains' },
71+
{ pageFilter: '/register', pageFilterType: 'notcontains' },
72+
{ pageFilter: '/onboarding', pageFilterType: 'notcontains' },
73+
{ pageFilter: '/login', pageFilterType: 'notcontains' },
74+
];
75+
test('SUPPRESSED on every excluded page (previously always showed)', () => {
76+
expect(checkPageRules(url('/onboarding?step=1'), { pageRules: rules })).toBe(false);
77+
expect(checkPageRules(url('/signup'), { pageRules: rules })).toBe(false);
78+
expect(checkPageRules(url('/register'), { pageRules: rules })).toBe(false);
79+
expect(checkPageRules(url('/login'), { pageRules: rules })).toBe(false);
80+
});
81+
test('shown on non-excluded pages', () => {
82+
expect(checkPageRules(url('/dashboard'), { pageRules: rules })).toBe(true);
83+
expect(checkPageRules(url('/forms/123'), { pageRules: rules })).toBe(true);
84+
});
85+
});
86+
87+
describe('checkPageRules — mixed multi-rule (positive scope minus exclusions)', () => {
88+
// Mirrors real live configs: CSAT "members" survey, Tess onboarding modal, Zeevou banner.
89+
test('CSAT: contains members AND excludes auth/dev pages', () => {
90+
const rules = [
91+
{ pageFilter: '/login', pageFilterType: 'notcontains' },
92+
{ pageFilter: '/signup', pageFilterType: 'notcontains' },
93+
{ pageFilter: '/profile/retention', pageFilterType: 'notcontains' },
94+
{ pageFilter: 'ewnova.dev', pageFilterType: 'notcontains' },
95+
{ pageFilter: 'members', pageFilterType: 'contains' },
96+
];
97+
expect(checkPageRules('https://app.ewnova.com/members/home', { pageRules: rules })).toBe(true);
98+
expect(checkPageRules('https://app.ewnova.com/members/login', { pageRules: rules })).toBe(false); // excluded
99+
expect(checkPageRules('https://app.ewnova.com/public', { pageRules: rules })).toBe(false); // not a member page
100+
expect(checkPageRules('https://ewnova.dev/members/home', { pageRules: rules })).toBe(false); // dev host excluded
101+
});
102+
103+
test('Tess: contains chat AND not payment', () => {
104+
const rules = [
105+
{ pageFilter: 'payment', pageFilterType: 'notcontains' },
106+
{ pageFilter: 'chat', pageFilterType: 'contains' },
107+
];
108+
expect(checkPageRules(url('/chat/room'), { pageRules: rules })).toBe(true);
109+
expect(checkPageRules(url('/chat/payment'), { pageRules: rules })).toBe(false);
110+
expect(checkPageRules(url('/home'), { pageRules: rules })).toBe(false);
111+
});
112+
113+
test('Zeevou: contains app host AND isnot signup wizard (exact)', () => {
114+
const rules = [
115+
{ pageFilter: 'https://app.zeevou.com/', pageFilterType: 'contains' },
116+
{ pageFilter: 'http://app.zeevou.com/wizard/signup', pageFilterType: 'isnot' },
117+
];
118+
expect(checkPageRules('https://app.zeevou.com/rates', { pageRules: rules })).toBe(true);
119+
expect(checkPageRules('http://app.zeevou.com/wizard/signup', { pageRules: rules })).toBe(false);
120+
expect(checkPageRules('https://other.com/rates', { pageRules: rules })).toBe(false);
121+
});
122+
});

0 commit comments

Comments
 (0)