Skip to content

Commit 15f5eb8

Browse files
Fix killer sudoku's wrong AmuseLabs URL, remove slug-derived template
BUG: every AmuseLabs-hosted PuzzleConfig entry's iframe URL was built from one shared template, substituting DCR's own slug into AmuseLabs' set=guardian-{slug} query param. That assumption was unsafe and already silently wrong for sudoku-killer: the real, confirmed AmuseLabs set (from the native Android/iOS apps' own working integration) is killer-sudoku-medium, not sudoku-killer, a different word order plus an unexplained "-medium" suffix that is genuinely part of the real identifier. Readers using killer sudoku were very likely being served the wrong puzzle or an AmuseLabs error. FIX: removes the shared URL template and {slug} substitution mechanism entirely, not just this one instance. PuzzleIframeConfig's urlTemplate field is replaced with url, holding each entry's own complete, explicit, hardcoded iframe URL. resolveIframeUrl now just returns config.iframe.url verbatim, there is nothing left to substitute, and nothing left to be silently wrong about for any future entry. validatePuzzleConfigs now checks iframe.url is present and a well-formed absolute URL. Tests hardcode and check each of the 6 entries' own exact expected URL individually (this is what would have caught the original bug), rather than testing a substitution mechanism in the abstract. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent 2b3aa30 commit 15f5eb8

2 files changed

Lines changed: 137 additions & 23 deletions

File tree

dotcom-rendering/src/model/puzzles/puzzleConfigs.test.ts

Lines changed: 66 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -46,15 +46,45 @@ describe('puzzleConfigs registry', () => {
4646
).toThrow(TypeError);
4747
});
4848

49-
it('rejects an entry with an empty iframe urlTemplate', () => {
49+
it('rejects an entry with an empty iframe url', () => {
5050
expect(() =>
5151
validatePuzzleConfigs({
5252
...puzzleConfigs,
5353
wordiply: {
5454
...puzzleConfigs.wordiply!,
5555
iframe: {
5656
...puzzleConfigs.wordiply!.iframe,
57-
urlTemplate: '',
57+
url: '',
58+
},
59+
},
60+
}),
61+
).toThrow(TypeError);
62+
});
63+
64+
it('rejects an entry with a non-absolute iframe url', () => {
65+
expect(() =>
66+
validatePuzzleConfigs({
67+
...puzzleConfigs,
68+
wordiply: {
69+
...puzzleConfigs.wordiply!,
70+
iframe: {
71+
...puzzleConfigs.wordiply!.iframe,
72+
url: '/not-absolute',
73+
},
74+
},
75+
}),
76+
).toThrow(TypeError);
77+
});
78+
79+
it('rejects an entry with a missing iframe provider', () => {
80+
expect(() =>
81+
validatePuzzleConfigs({
82+
...puzzleConfigs,
83+
wordiply: {
84+
...puzzleConfigs.wordiply!,
85+
iframe: {
86+
...puzzleConfigs.wordiply!.iframe,
87+
provider: '',
5888
},
5989
},
6090
}),
@@ -178,13 +208,45 @@ describe('puzzleConfigs registry', () => {
178208
});
179209

180210
describe('resolveIframeUrl', () => {
181-
it('substitutes the slug into the AmuseLabs URL template', () => {
211+
// Each of these hardcodes and checks a single entry's own, complete,
212+
// expected real URL independently, this is deliberately what would
213+
// have caught the original killer-sudoku bug (a shared, slug-derived
214+
// URL template silently produced the wrong AmuseLabs "set" for it),
215+
// rather than testing a substitution mechanism in the abstract.
216+
it('resolves sudoku-easy to its exact, confirmed AmuseLabs URL', () => {
182217
expect(resolveIframeUrl(puzzleConfigs['sudoku-easy']!)).toBe(
183218
'https://tg.amuselabs.com/guardian/date-picker?set=guardian-sudoku-easy&embed=1&idx=1',
184219
);
185220
});
186221

187-
it('returns the bespoke provider URL unchanged when it has no placeholder', () => {
222+
it('resolves sudoku-medium to its exact, confirmed AmuseLabs URL', () => {
223+
expect(resolveIframeUrl(puzzleConfigs['sudoku-medium']!)).toBe(
224+
'https://tg.amuselabs.com/guardian/date-picker?set=guardian-sudoku-medium&embed=1&idx=1',
225+
);
226+
});
227+
228+
it('resolves sudoku-hard to its exact, confirmed AmuseLabs URL', () => {
229+
expect(resolveIframeUrl(puzzleConfigs['sudoku-hard']!)).toBe(
230+
'https://tg.amuselabs.com/guardian/date-picker?set=guardian-sudoku-hard&embed=1&idx=1',
231+
);
232+
});
233+
234+
it('resolves sudoku-killer to its exact, confirmed AmuseLabs URL (killer-sudoku-medium, not sudoku-killer)', () => {
235+
expect(resolveIframeUrl(puzzleConfigs['sudoku-killer']!)).toBe(
236+
'https://tg.amuselabs.com/guardian/date-picker?set=guardian-killer-sudoku-medium&embed=1&idx=1',
237+
);
238+
expect(
239+
resolveIframeUrl(puzzleConfigs['sudoku-killer']!),
240+
).not.toContain('guardian-sudoku-killer');
241+
});
242+
243+
it('resolves word-wheel to its exact, confirmed AmuseLabs URL', () => {
244+
expect(resolveIframeUrl(puzzleConfigs['word-wheel']!)).toBe(
245+
'https://tg.amuselabs.com/guardian/date-picker?set=guardian-word-wheel&embed=1&idx=1',
246+
);
247+
});
248+
249+
it('resolves wordiply to its exact, own explicit (non-AmuseLabs) URL', () => {
188250
expect(resolveIframeUrl(puzzleConfigs.wordiply!)).toBe(
189251
'https://www.wordiply.com/',
190252
);

dotcom-rendering/src/model/puzzles/puzzleConfigs.ts

Lines changed: 71 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,12 @@ import { formatPuzzleDateShort } from '../../lib/puzzleDate';
1616
* (on-the-ball, film-reveal) were removed for V0 and may return later once
1717
* the team is ready to support them.
1818
*
19-
* This registry is deliberately data-driven: all AmuseLabs-hosted puzzles
20-
* (the sudoku variants, and word-wheel) share the exact same iframe URL
21-
* template and differ only by the `{slug}` substitution, so they are
22-
* modelled as data rather than near-duplicate code paths.
19+
* Every entry's `iframe.url` is a complete, explicit, hardcoded URL string,
20+
* there is deliberately no shared URL template or `{slug}`-style
21+
* substitution mechanism here (there used to be one; see the "why no
22+
* template" note on `PuzzleIframeConfig.url` below for why it was
23+
* removed). Even though most AmuseLabs-hosted entries happen to share the
24+
* same URL *shape*, each is still written out independently in full.
2325
*/
2426

2527
export const puzzleGroups = ['logic-puzzles', 'word-games'] as const;
@@ -29,10 +31,27 @@ export type PuzzleGroup = (typeof puzzleGroups)[number];
2931
export interface PuzzleIframeConfig {
3032
provider: string;
3133
/**
32-
* The iframe src URL. May contain a `{slug}` placeholder token, which is
33-
* substituted with the puzzle's `slug` at render time.
34+
* The complete, final iframe src URL for this specific puzzle, written
35+
* out explicitly and independently, e.g.
36+
* `https://tg.amuselabs.com/guardian/date-picker?set=guardian-sudoku-easy&embed=1&idx=1`.
37+
*
38+
* Why no shared URL template: this registry used to build every
39+
* AmuseLabs-hosted entry's URL from one shared template, substituting
40+
* DCR's own `slug` in for AmuseLabs' `set=guardian-{slug}` query param.
41+
* That was an unsafe assumption, nothing guarantees a provider's own
42+
* naming convention matches our internal slug, and it already silently
43+
* didn't for killer sudoku (AmuseLabs' real, confirmed set for it is
44+
* `killer-sudoku-medium`, not `sudoku-killer`, a different word order,
45+
* plus an unexplained "-medium" suffix that is genuinely part of the
46+
* real, working identifier, not a mistake to "fix"). Rather than patch
47+
* that one instance, every entry now specifies its own complete,
48+
* independent URL, confirmed against the actual provider (or, here,
49+
* the native Android/iOS apps' own real, working AmuseLabs
50+
* integration), so a future change to one entry can never silently or
51+
* accidentally affect another, and there is no shared assumption left
52+
* to be wrong about. See docs/puzzle-page.md.
3453
*/
35-
urlTemplate: string;
54+
url: string;
3655
}
3756

3857
export interface PuzzleConfig {
@@ -86,18 +105,16 @@ export interface PuzzleConfig {
86105
image?: string;
87106
}
88107

89-
const amuseLabsUrlTemplate =
90-
'https://tg.amuselabs.com/guardian/date-picker?set=guardian-{slug}&embed=1&idx=1';
91-
92108
const amuseLabsPuzzle = (
93109
slug: string,
94110
puzzleGroup: PuzzleGroup,
95111
title: string,
96112
description: string,
113+
url: string,
97114
): PuzzleConfig => ({
98115
slug,
99116
puzzleGroup,
100-
iframe: { provider: 'amuselabs', urlTemplate: amuseLabsUrlTemplate },
117+
iframe: { provider: 'amuselabs', url },
101118
shareEnabled: true,
102119
printEnabled: true,
103120
hasArchive: true,
@@ -118,6 +135,12 @@ const amuseLabsPuzzle = (
118135
* a `<meta name="keywords">` tag (major search engines ignore that tag
119136
* entirely, so it provides no real SEO benefit today, see
120137
* docs/puzzle-page.md).
138+
*
139+
* Every entry's `iframe.url` is its own complete, independently-written
140+
* URL, confirmed against the native (Android/iOS) apps' own real, working
141+
* AmuseLabs integration (`remote_config_defaults.xml`/
142+
* `PuzzleGameViewModel.kt`). Do not derive any of these from `slug` or
143+
* from each other, see `PuzzleIframeConfig.url`'s doc comment for why.
121144
*/
122145
export const puzzleConfigs: Record<string, PuzzleConfig> = {
123146
// Target search terms (reference only, not implemented as a meta tag):
@@ -127,6 +150,7 @@ export const puzzleConfigs: Record<string, PuzzleConfig> = {
127150
'logic-puzzles',
128151
'Easy sudoku {date} - logic puzzle | The Guardian',
129152
'Easy sudoku {date}. Ease yourself in with this easy sudoku. Fill the grid with the numbers 1 to 9, appearing only once in every column, row and 3x3 box.',
153+
'https://tg.amuselabs.com/guardian/date-picker?set=guardian-sudoku-easy&embed=1&idx=1',
130154
),
131155
// Target search terms (reference only, not implemented as a meta tag):
132156
// medium sudoku
@@ -135,6 +159,7 @@ export const puzzleConfigs: Record<string, PuzzleConfig> = {
135159
'logic-puzzles',
136160
'Medium sudoku {date} - logic puzzle | The Guardian',
137161
'Medium sudoku {date}. Ready to master the medium sudoku? Fill the grid with the numbers 1 to 9, appearing only once in every column, row and 3x3 box.',
162+
'https://tg.amuselabs.com/guardian/date-picker?set=guardian-sudoku-medium&embed=1&idx=1',
138163
),
139164
// Target search terms (reference only, not implemented as a meta tag):
140165
// hard sudoku
@@ -143,14 +168,24 @@ export const puzzleConfigs: Record<string, PuzzleConfig> = {
143168
'logic-puzzles',
144169
'Hard sudoku {date} - logic puzzle | The Guardian',
145170
'Hard sudoku {date}. Ready to take on the hard sudoku? Fill the grid with the numbers 1 to 9, appearing only once in every column, row and 3x3 box.',
171+
'https://tg.amuselabs.com/guardian/date-picker?set=guardian-sudoku-hard&embed=1&idx=1',
146172
),
147173
// Target search terms (reference only, not implemented as a meta tag):
148174
// killer sudoku
175+
//
176+
// This URL's `set=guardian-killer-sudoku-medium` is NOT
177+
// `set=guardian-sudoku-killer` (this DCR slug, template-derived). It is
178+
// confirmed from the native (Android/iOS) apps' real, working AmuseLabs
179+
// integration: a different word order, plus an unexplained "-medium"
180+
// suffix that is genuinely part of the real identifier, not a mistake.
181+
// This was previously a live bug (built from a shared, slug-derived URL
182+
// template), see docs/puzzle-page.md.
149183
'sudoku-killer': amuseLabsPuzzle(
150184
'sudoku-killer',
151185
'logic-puzzles',
152186
'Killer sudoku {date} - logic puzzle | The Guardian',
153187
'Killer sudoku {date}. Killer sudoku adds a twist. Fill the grid with the numbers 1 to 9, appearing only once in every column, row and 3x3 box.',
188+
'https://tg.amuselabs.com/guardian/date-picker?set=guardian-killer-sudoku-medium&embed=1&idx=1',
154189
),
155190
// Target search terms (reference only, not implemented as a meta tag):
156191
// daily word wheel, word wheel puzzle, word wheel online, word wheel
@@ -161,6 +196,7 @@ export const puzzleConfigs: Record<string, PuzzleConfig> = {
161196
'word-games',
162197
'Word wheel {date} - word game | The Guardian',
163198
'Word wheel {date}. See how many words you can make out of the nine-letter daily word wheel, including the panagram.',
199+
'https://tg.amuselabs.com/guardian/date-picker?set=guardian-word-wheel&embed=1&idx=1',
164200
),
165201
// Target search terms (reference only, not implemented as a meta tag):
166202
// guardian wordiply, wordiply today
@@ -169,7 +205,7 @@ export const puzzleConfigs: Record<string, PuzzleConfig> = {
169205
puzzleGroup: 'word-games',
170206
iframe: {
171207
provider: 'wordiply',
172-
urlTemplate: 'https://www.wordiply.com/',
208+
url: 'https://www.wordiply.com/',
173209
},
174210
shareEnabled: true,
175211
printEnabled: true,
@@ -189,11 +225,14 @@ export const getPuzzleConfig = (slug: string): PuzzleConfig | undefined =>
189225
puzzleConfigs[slug];
190226

191227
/**
192-
* Resolve the final iframe src URL for a puzzle, expanding the `{slug}`
193-
* placeholder token in `PuzzleIframeConfig.urlTemplate`.
228+
* Resolve the iframe src URL for a puzzle. There is nothing to substitute
229+
* any more, every entry's `iframe.url` is already its own complete, final
230+
* URL, this exists purely so callers have one stable access point rather
231+
* than reaching into `config.iframe.url` directly, matching the shape of
232+
* the other `resolvePuzzle*` helpers below.
194233
*/
195234
export const resolveIframeUrl = (config: PuzzleConfig): string =>
196-
config.iframe.urlTemplate.replaceAll('{slug}', config.slug);
235+
config.iframe.url;
197236

198237
/**
199238
* Substitutes `{date}` in a `title`/`description` template with the given
@@ -234,10 +273,22 @@ export const resolvePuzzleDescription = (
234273
puzzleDate: string | undefined,
235274
): string => resolveDateTemplate(config.description, puzzleDate);
236275

276+
const isAbsoluteUrl = (value: string): boolean => {
277+
try {
278+
new URL(value);
279+
return true;
280+
} catch {
281+
return false;
282+
}
283+
};
284+
237285
const isValidPuzzleConfig = (key: string, config: PuzzleConfig): boolean => {
238286
if (config.slug !== key) return false;
239287
if (!puzzleGroups.includes(config.puzzleGroup)) return false;
240-
if (!config.iframe.provider || !config.iframe.urlTemplate) return false;
288+
if (!config.iframe.provider) return false;
289+
if (!config.iframe.url.trim() || !isAbsoluteUrl(config.iframe.url)) {
290+
return false;
291+
}
241292
if (!config.title.trim()) return false;
242293
if (!config.description.trim()) return false;
243294
if (config.image !== undefined && !config.image.trim()) return false;
@@ -246,9 +297,10 @@ const isValidPuzzleConfig = (key: string, config: PuzzleConfig): boolean => {
246297

247298
/**
248299
* Fail fast if the registry itself is malformed (e.g. a mismatched slug key,
249-
* a missing/empty `iframe` config, a missing/empty `title`/`description`,
250-
* or a present-but-empty `image`). Run once at module load so a bad
251-
* registry entry surfaces immediately rather than at request time.
300+
* a missing `iframe.provider`, a missing/empty/non-absolute `iframe.url`, a
301+
* missing/empty `title`/`description`, or a present-but-empty `image`). Run
302+
* once at module load so a bad registry entry surfaces immediately rather
303+
* than at request time.
252304
*/
253305
export const validatePuzzleConfigs = (
254306
configs: Record<string, PuzzleConfig>,

0 commit comments

Comments
 (0)