Skip to content

Commit

Permalink
FIX: Don't unnecessarily scrub query params from homepage (#25665)
Browse files Browse the repository at this point in the history
Seems like an accident that the homepage route will always strip all query params from the URL.. This fixes that :)
  • Loading branch information
markvanlan committed Feb 13, 2024
1 parent d1ebca9 commit 4f75cee
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,31 @@ export default function applyRouterHomepageOverrides(router) {
}
}

const homepageRewriteParam = "_discourse_homepage_rewrite";

/**
* Returns a magic URL which `discovery-index` will redirect to.
* We watch for this, and then perform the rewrite in the router.
*/
export function homepageDestination() {
return `/${defaultHomepage()}?_discourse_homepage_rewrite=1`;
return `/${defaultHomepage()}?${homepageRewriteParam}=1`;
}

function rewriteIfNeeded(url, transition) {
const intentUrl = transition?.intent?.url;
if (
intentUrl?.startsWith(homepageDestination()) ||
(transition?.intent.name === `discovery.${defaultHomepage()}` &&
transition?.intent.queryParams["_discourse_homepage_rewrite"])
transition?.intent.queryParams[homepageRewriteParam])
) {
const params = url.split("?", 2)[1];
const params = (intentUrl || url).split("?", 2)[1];
url = "/";
if (params) {
url += `?${params}`;
const searchParams = new URLSearchParams(params);
searchParams.delete(homepageRewriteParam);
if (searchParams.size) {
url += `?${searchParams.toString()}`;
}
}
}
return url;
Expand Down
14 changes: 14 additions & 0 deletions app/assets/javascripts/discourse/tests/acceptance/homepage-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,4 +74,18 @@ acceptance("Dynamic homepage handling", function () {
await click(".nav-item_categories a");
assertOnCategories("/categories");
});

test("it passes query params through", async function (assert) {
await visit("/?test-one=true");

const router = getOwner(this).lookup("service:router");
assert.strictEqual(router.currentURL, "/?test-one=true");
});

test("it removes the _discourse_homepage_rewrite param from the URL", async function (assert) {
await visit("/?_discourse_homepage_rewrite=1&test-one=true");

const router = getOwner(this).lookup("service:router");
assert.strictEqual(router.currentURL, "/?test-one=true");
});
});

1 comment on commit 4f75cee

@discoursebot
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This commit has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/gtm-parameters-are-not-preserved-or-passed-to-gtm/307240/3

Please sign in to comment.