Disable ?locale on static embeds/public links
#50391
Conversation
| @@ -1009,7 +1009,7 @@ describeEE("scenarios > embedding > dashboard appearance", () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it("should allow to set locale from the `locale` query parameter", () => { | |||
| it("should allow to set locale from the `#locale` hash parameter (metabase#50182)", () => { | |||
There was a problem hiding this comment.
All tests that were tested with search param ?locale need to be migrated to test with hash param and only in EE, since this feature only works with Pro plan.
| @@ -198,24 +198,42 @@ describe("scenarios > embedding > questions", () => { | |||
| // eslint-disable-next-line no-unscoped-text-selectors -- deprecated usage | |||
| cy.contains("October 7, 2023, 1:34 AM"); | |||
| }); | |||
| }); | |||
|
|
|||
| describeEE("scenarios [EE] > embedding > questions", () => { | |||
There was a problem hiding this comment.
The previous test was in describe, so I need to move it inside describeEE. This uses the same setup as describe but add setTokenFeatures("all")
| cy.request("PUT", `/api/card/${ORDERS_QUESTION_ID}`, { | ||
| enable_embedding: true, | ||
| }); | ||
|
|
||
| visitQuestion(ORDERS_QUESTION_ID); | ||
|
|
||
| openStaticEmbeddingModal({ activeTab: "parameters" }); | ||
| openStaticEmbeddingModal({ activeTab: "parameters", acceptTerms: false }); |
There was a problem hiding this comment.
we don't need to accept terms and condition on Pro plan.
| @@ -1138,7 +1138,7 @@ describeEE("issue 8490", () => { | |||
| }); | |||
| }); | |||
|
|
|||
| it("static embeddings should respect `#locale` hash in the URL (metabase#8490)", () => { | |||
| it("static embeddings should respect `#locale` hash parameter (metabase#8490, metabase#50182)", () => { | |||
There was a problem hiding this comment.
just a name update for tracking purposes.
| @@ -357,4 +345,16 @@ describeEE("scenarios [EE] > public > dashboard", () => { | |||
| cy.title().should("eq", "Test Dashboard · Custom Application Name"); | |||
| }); | |||
| }); | |||
|
|
|||
| it("should allow to set locale from the `#locale` hash parameter (metabase#50182)", () => { | |||
There was a problem hiding this comment.
moved from describe to describeEE
| updateSetting("enable-public-sharing", true); | ||
| }); | ||
|
|
||
| it("should allow to set locale from the `#locale` hash parameter (metabase#50182)", () => { |
| {:bootstrapJS (load-inline-js "index_bootstrap") | ||
| :bootstrapJSON (escape-script (json/generate-string public-settings)) | ||
| :assetOnErrorJS (load-inline-js "asset_loading_error") | ||
| :userLocalizationJSON (escape-script (load-localization (:locale params))) | ||
| :userLocalizationJSON (escape-script (load-localization (when should-load-locale-params? (:locale params)))) |
There was a problem hiding this comment.
This is where we inject _metabaseUserLocalization into the HTML. We only want to do this when we're not using static embeds or public links.
We will still inject the locale on interactive embedding.
| * @param {Record<string, string>} options.params | ||
| * @param {Record<string, string>} options.hash | ||
| */ | ||
| export function visitPublicQuestion(id, { params = {}, hash = {} } = {}) { |
There was a problem hiding this comment.
This function never receives params, or hash before, so I make it the same interface as visitPublicDashboard for consistency, and use it to replace some direct cy.visit calls.
WiNloSt
deleted the
50313-locale-remove-locale-parameter-from-static-embeds
branch
Note
Do not backport this PR because it targets 52
Closes #50313
Description
Since we have the new hash option
#localeon static embeds/public links. We want to avoid confusion on the undocumented?localeparameter by removing it and opt for#localeinstead.How to verify
?locale=xxwith the locale of your choice e.g.?locale=koin either a static embed or a public link on either a dashboard or a question. And none of it should work?locale=xxwith the locale of your choice on normal Metabase and it should work. This means the interactive embed would work as well.#locale, and it should only work when you have a Pro license (Put#localeunderwhitelabelfeature flag #50199).Demo
with
?locale=en, it doesn't work anymorewith
#localo=en, only when we use the hash locale, then it would workChecklist