Skip to content

Commit 93cb57d

Browse files
humitoshumitos
authored
Addons: always inject META tag if the value exists (#279)
* Addons: always inject META tag if the value exists We were checking for `projectSlug` and `versionSlug` to exists before adding all the META tags. When one of them doesn't exist the `resolverFilename` and `httpStatus` were added. Now, I'm checking for each of the values and adding them if they exist. I found this issue while working on the analytics addons, since the `httpStatus` META tag wasn't added because I was accessing an invalid URL without version. * Improve tests to check META headers * Remove old test for `build.commands` that's not relevant anymore * Add specific test for this use case
1 parent 6f242c0 commit 93cb57d

File tree

2 files changed

+41
-44
lines changed

2 files changed

+41
-44
lines changed

‎packages/addons-inject/index.js

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -259,17 +259,23 @@ class addMetaTags {
259259

260260
element(element) {
261261
console.log(
262-
`addProjectVersionSlug. projectSlug=${this.projectSlug} versionSlug=${this.versionSlug} resolverFilename=${this.resolverFilename}`,
262+
`addProjectVersionSlug. projectSlug=${this.projectSlug} versionSlug=${this.versionSlug} resolverFilename=${this.resolverFilename} httpStatus=${this.httpStatus}`,
263263
);
264-
if (this.projectSlug && this.versionSlug) {
264+
if (this.projectSlug) {
265265
const metaProject = `<meta name="readthedocs-project-slug" content="${this.projectSlug}" />`;
266-
const metaVersion = `<meta name="readthedocs-version-slug" content="${this.versionSlug}" />`;
267-
const metaResolverFilename = `<meta name="readthedocs-resolver-filename" content="${this.resolverFilename}" />`;
268-
const metaHttpStatus = `<meta name="readthedocs-http-status" content="${this.httpStatus}" />`;
269-
270266
element.append(metaProject, { html: true });
267+
}
268+
if (this.versionSlug) {
269+
const metaVersion = `<meta name="readthedocs-version-slug" content="${this.versionSlug}" />`;
271270
element.append(metaVersion, { html: true });
271+
}
272+
if (this.resolverFilename) {
273+
const metaResolverFilename = `<meta name="readthedocs-resolver-filename" content="${this.resolverFilename}" />`;
272274
element.append(metaResolverFilename, { html: true });
275+
}
276+
277+
if (this.httpStatus) {
278+
const metaHttpStatus = `<meta name="readthedocs-http-status" content="${this.httpStatus}" />`;
273279
element.append(metaHttpStatus, { html: true });
274280
}
275281
}

‎packages/addons-inject/tests/wrangler.test.js

Lines changed: 29 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,11 @@ describe("Addons when enabled", async () => {
4949
expect(response.status).toBe(200);
5050
expect(response.headers.get("X-RTD-Force-Addons")).toBe("true");
5151
expect(await response.text()).toBe(
52-
`<html><head>${AddonsConstants.scriptAddons}</head><body></body></html>`,
52+
`<html><head>${AddonsConstants.scriptAddons}<meta name="readthedocs-http-status" content="200" /></head><body></body></html>`
5353
);
5454
});
5555

56-
it("injects project/version metadata", async () => {
56+
it("injects project/version/filename/httpstatus metadata", async () => {
5757
fetchMock
5858
.get("http://test-builds.devthedocs.org")
5959
.intercept({ path: "/en/latest/" })
@@ -63,6 +63,7 @@ describe("Addons when enabled", async () => {
6363
"X-RTD-Force-Addons": true,
6464
"X-RTD-Project": "test-builds",
6565
"X-RTD-Version": "latest",
66+
"X-RTD-Resolver-Filename": "index.html",
6667
},
6768
});
6869
let response = await SELF.fetch(
@@ -74,7 +75,32 @@ describe("Addons when enabled", async () => {
7475
.to.contain(
7576
`<meta name="readthedocs-project-slug" content="test-builds" />`,
7677
)
77-
.to.contain(`<meta name="readthedocs-version-slug" content="latest" />`);
78+
.to.contain(`<meta name="readthedocs-version-slug" content="latest" />`)
79+
.to.contain(`<meta name="readthedocs-resolver-filename" content="index.html" />`)
80+
.to.contain(`<meta name="readthedocs-http-status" content="200" />`);
81+
});
82+
83+
it("injects httpstatus metadata when no version", async () => {
84+
fetchMock
85+
.get("http://test-builds.devthedocs.org")
86+
.intercept({ path: "/en/latest/" })
87+
.reply(200, "<html><head></head><body></body></html>", {
88+
headers: {
89+
"Content-type": "text/html",
90+
"X-RTD-Force-Addons": true,
91+
"X-RTD-Project": "test-builds",
92+
},
93+
});
94+
let response = await SELF.fetch(
95+
"http://test-builds.devthedocs.org/en/latest/",
96+
);
97+
expect(response.status).toBe(200);
98+
expect(await response.text())
99+
.to.contain(AddonsConstants.scriptAddons)
100+
.to.contain(
101+
`<meta name="readthedocs-project-slug" content="test-builds" />`,
102+
)
103+
.to.contain(`<meta name="readthedocs-http-status" content="200" />`);
78104
});
79105

80106
it("removes old flyout doc embed assets", async () => {
@@ -162,41 +188,6 @@ describe("Addons when enabled", async () => {
162188
matcher.to.not.contain(`x-ref="flyout"`);
163189
});
164190

165-
it("only injects content for build.commands builds", async () => {
166-
fetchMock
167-
.get("http://test-builds.devthedocs.org")
168-
.intercept({ path: "/en/latest/" })
169-
.reply(
170-
200,
171-
`
172-
<html>
173-
<head>
174-
<script src="https://assets.readthedocs.org/static/core/js/readthedocs-doc-embed.js"></script>
175-
<script src="https://example.com"></script>
176-
</head>
177-
<body>
178-
</body>
179-
</html>`,
180-
{
181-
headers: {
182-
"Content-type": "text/html",
183-
"X-RTD-Force-Addons": false,
184-
},
185-
},
186-
);
187-
let response = await SELF.fetch(
188-
"http://test-builds.devthedocs.org/en/latest/",
189-
);
190-
expect(response.status).toBe(200);
191-
const matcher = expect(await response.text());
192-
matcher.to.contain(AddonsConstants.scriptAddons);
193-
// Ensure matches aren't too aggressive or removing content
194-
matcher.to.contain(
195-
`<script src="https://assets.readthedocs.org/static/core/js/readthedocs-doc-embed.js"></script>`,
196-
);
197-
matcher.to.contain(`<script src="https://example.com"></script>`);
198-
});
199-
200191
it("skips content types other than HTML", async () => {
201192
fetchMock
202193
.get("http://test-builds.devthedocs.org")

0 commit comments

Comments
 (0)