Skip to content

Commit 3a6086f

Browse files
committed
Fix placeholder limits not working with fallbacks
1 parent 5426901 commit 3a6086f

File tree

3 files changed

+240
-1
lines changed

3 files changed

+240
-1
lines changed

services/user-feeds-next/src/articles/formatter/article-formatter.test.ts

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,84 @@ describe("article-formatter", () => {
8888
);
8989
assert.strictEqual(result, "Third Value");
9090
});
91+
92+
it("applies placeholder limits with fallback syntax - first placeholder used", () => {
93+
const result = replaceTemplateString(
94+
{ summary: "This is a long summary that should be truncated", description: "Description text" },
95+
"{{summary||description}}",
96+
{
97+
supportFallbacks: true,
98+
split: {
99+
func: (str, opts) => str.substring(0, opts.limit) + (str.length > opts.limit ? (opts.appendString || "") : ""),
100+
limits: [
101+
{ placeholder: "summary", characterCount: 15, appendString: "..." },
102+
{ placeholder: "description", characterCount: 20, appendString: "..." },
103+
],
104+
},
105+
}
106+
);
107+
// Should use summary limit (15) since summary was used
108+
assert.ok(result!.startsWith("This is a long "));
109+
assert.ok(result!.endsWith("..."));
110+
assert.ok(result!.length <= 18); // 15 + 3 for "..."
111+
});
112+
113+
it("applies placeholder limits with fallback syntax - falls back to second placeholder", () => {
114+
const result = replaceTemplateString(
115+
{ description: "This is a long description that should be truncated" },
116+
"{{summary||description}}",
117+
{
118+
supportFallbacks: true,
119+
split: {
120+
func: (str, opts) => str.substring(0, opts.limit) + (str.length > opts.limit ? (opts.appendString || "") : ""),
121+
limits: [
122+
{ placeholder: "summary", characterCount: 10, appendString: " [sum]" },
123+
{ placeholder: "description", characterCount: 15, appendString: " [desc]" },
124+
],
125+
},
126+
}
127+
);
128+
// Should use description limit (15) since description was used as fallback
129+
assert.ok(result!.startsWith("This is a long "));
130+
assert.ok(result!.endsWith("[desc]"));
131+
assert.ok(result!.length <= 22); // 15 + 7 for " [desc]"
132+
});
133+
134+
it("does not apply placeholder limits when fallback resolves to literal text", () => {
135+
const result = replaceTemplateString(
136+
{},
137+
"{{summary||text::No summary available}}",
138+
{
139+
supportFallbacks: true,
140+
split: {
141+
func: (str, opts) => str.substring(0, opts.limit) + (str.length > opts.limit ? (opts.appendString || "") : ""),
142+
limits: [
143+
{ placeholder: "summary", characterCount: 5, appendString: "..." },
144+
],
145+
},
146+
}
147+
);
148+
// Literal text should not be truncated
149+
assert.strictEqual(result, "No summary available");
150+
});
151+
152+
it("supports limit on full accessor string as fallback", () => {
153+
const result = replaceTemplateString(
154+
{ summary: "This is a long summary text" },
155+
"{{summary||description}}",
156+
{
157+
supportFallbacks: true,
158+
split: {
159+
func: (str, opts) => str.substring(0, opts.limit) + (str.length > opts.limit ? (opts.appendString || "") : ""),
160+
limits: [
161+
{ placeholder: "summary||description", characterCount: 10, appendString: "..." },
162+
],
163+
},
164+
}
165+
);
166+
// Should use the full accessor limit
167+
assert.ok(result!.length <= 13); // 10 + 3 for "..."
168+
});
91169
});
92170

93171
describe("applySplit", () => {

services/user-feeds-next/src/articles/formatter/article-formatter.ts

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -713,29 +713,36 @@ export function replaceTemplateString(
713713
while ((result = regex.exec(str)) !== null) {
714714
const accessor = result[1]!;
715715
let value = "";
716+
let usedPlaceholder: string | null = null;
716717

717718
if (options?.supportFallbacks) {
718719
const values = accessor.split("||");
719720
for (const subvalue of values) {
720721
// Special "text::" prefix for literal fallback text
721722
if (subvalue.startsWith("text::")) {
722723
value = subvalue.replace("text::", "");
724+
usedPlaceholder = null; // literal text, no placeholder limit applies
723725
break;
724726
}
725727
const valueInObject = object[subvalue];
726728
if (valueInObject) {
727729
value = valueInObject;
730+
usedPlaceholder = subvalue;
728731
break;
729732
}
730733
}
731734
} else {
732735
value = object[accessor] || "";
736+
usedPlaceholder = accessor;
733737
}
734738

735739
// Apply split limits for specific placeholders
740+
// Check for limit on: 1) the specific placeholder that provided the value, 2) the full accessor
736741
if (options?.split?.func && options.split.limits) {
737742
const limit = options.split.limits.find(
738-
(i) => i.placeholder === accessor
743+
(i) =>
744+
(usedPlaceholder && i.placeholder === usedPlaceholder) ||
745+
i.placeholder === accessor
739746
);
740747
if (limit) {
741748
const appendString = replaceTemplateString(object, limit.appendString, {

services/user-feeds-next/test/e2e/discord-payload-placeholder-limits.e2e-spec.ts

Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -591,4 +591,158 @@ describe("Discord Payload Placeholder Limits (e2e)", { concurrency: true }, () =
591591
}
592592
});
593593
});
594+
595+
describe("Fallback Syntax with Placeholder Limits", () => {
596+
it("applies limit when using fallback syntax and first placeholder has value", async () => {
597+
const ctx = createTestContext(stores);
598+
599+
const eventWithLimits = createEventWithPlaceholderLimits(
600+
ctx.testFeedV2Event,
601+
[
602+
{
603+
placeholder: "summary",
604+
characterCount: 20,
605+
appendString: "...",
606+
},
607+
{
608+
placeholder: "description",
609+
characterCount: 30,
610+
appendString: "...",
611+
},
612+
],
613+
{
614+
content: "{{summary||description}}",
615+
}
616+
);
617+
618+
try {
619+
await ctx.seedArticles(eventWithLimits);
620+
621+
ctx.setFeedResponse(() => ({
622+
body: getTestRssFeed(
623+
[
624+
{
625+
guid: "fallback-limit-summary-test",
626+
summary:
627+
"This is a long summary that should be truncated to 20 chars",
628+
description: "This description should not be used",
629+
},
630+
],
631+
true
632+
),
633+
hash: randomUUID(),
634+
}));
635+
636+
const results = await ctx.handleEvent(eventWithLimits);
637+
638+
assert.notStrictEqual(results, null);
639+
640+
const payload = getDiscordPayload(ctx);
641+
// Summary limit (20) + append string (3) = 23 max
642+
assert.ok(payload.content.length <= 23);
643+
assert.ok(payload.content.includes("..."));
644+
} finally {
645+
ctx.cleanup();
646+
}
647+
});
648+
649+
it("applies limit when using fallback syntax and falls back to second placeholder", async () => {
650+
const ctx = createTestContext(stores);
651+
652+
const eventWithLimits = createEventWithPlaceholderLimits(
653+
ctx.testFeedV2Event,
654+
[
655+
{
656+
placeholder: "summary",
657+
characterCount: 20,
658+
appendString: " [sum]",
659+
},
660+
{
661+
placeholder: "description",
662+
characterCount: 25,
663+
appendString: " [desc]",
664+
},
665+
],
666+
{
667+
content: "{{summary||description}}",
668+
}
669+
);
670+
671+
try {
672+
await ctx.seedArticles(eventWithLimits);
673+
674+
ctx.setFeedResponse(() => ({
675+
body: getTestRssFeed(
676+
[
677+
{
678+
guid: "fallback-limit-description-test",
679+
// No summary - will fall back to description
680+
description:
681+
"This is a long description that should be truncated to 25 chars",
682+
},
683+
],
684+
true
685+
),
686+
hash: randomUUID(),
687+
}));
688+
689+
const results = await ctx.handleEvent(eventWithLimits);
690+
691+
assert.notStrictEqual(results, null);
692+
693+
const payload = getDiscordPayload(ctx);
694+
// Description limit (25) + append string (7) = 32 max
695+
assert.ok(payload.content.length <= 32);
696+
assert.ok(payload.content.includes("[desc]"));
697+
assert.ok(!payload.content.includes("[sum]"));
698+
} finally {
699+
ctx.cleanup();
700+
}
701+
});
702+
703+
it("does not apply limit when fallback resolves to literal text", async () => {
704+
const ctx = createTestContext(stores);
705+
706+
const eventWithLimits = createEventWithPlaceholderLimits(
707+
ctx.testFeedV2Event,
708+
[
709+
{
710+
placeholder: "summary",
711+
characterCount: 10,
712+
appendString: "...",
713+
},
714+
],
715+
{
716+
content: "{{summary||text::No summary available}}",
717+
}
718+
);
719+
720+
try {
721+
await ctx.seedArticles(eventWithLimits);
722+
723+
ctx.setFeedResponse(() => ({
724+
body: getTestRssFeed(
725+
[
726+
{
727+
guid: "fallback-literal-test",
728+
// No summary - will fall back to literal text
729+
},
730+
],
731+
true
732+
),
733+
hash: randomUUID(),
734+
}));
735+
736+
const results = await ctx.handleEvent(eventWithLimits);
737+
738+
assert.notStrictEqual(results, null);
739+
740+
const payload = getDiscordPayload(ctx);
741+
// Literal text should not be truncated
742+
assert.strictEqual(payload.content, "No summary available");
743+
} finally {
744+
ctx.cleanup();
745+
}
746+
});
747+
});
594748
});

0 commit comments

Comments
 (0)