Fix Embedded dashboard parameters parse runtime error #41545
Conversation
deniskaber
added
.Team/Embedding
backport
deniskaber
changed the title
|
| }); | ||
|
|
||
| // should not throw runtime error and render dashboard content | ||
| expect(screen.getByText(DASHBOARD_TITLE)).toBeInTheDocument(); |
There was a problem hiding this comment.
This test only verifies that the dashboard is rendered not crashed. But it doesn't assert that the filter value is retained.
I've tried a few iterations to make this unit test work by passing dashcards and parameters but couldn't do that. Maybe you could try or create an E2E if that's easier. I think it's important to capture that 01 would still be present and used as a filter.
At first I thought this code wasn't fixing the issue because there's also no filter value assertion in the test.
| }); | ||
|
|
||
| // should not throw runtime error and render dashboard content | ||
| expect(screen.getByText(DASHBOARD_TITLE)).toBeInTheDocument(); |
There was a problem hiding this comment.
I think the behaviour customers expect is that the filter will have value 1, should we make an expect on that too?
I think we'll need to so some more mocking to make the filter show up in the UI
There was a problem hiding this comment.
To get 01 in the URL, customer needs to enter 01 in the filter. If the initial result shows as 01 then I'd expect that he might expect it to still show 01 (like what it initially was before refreshing the page)
| it("should render when a filter passed with value starting from '0' (metabase#41483)", () => { | ||
| cy.get("@dashboardId").then(id => { | ||
| visitPublicDashboard(id, { | ||
| params: { number: "02" }, |
There was a problem hiding this comment.
could you use text as it's already existing?
| params: { number: "02" }, | |
| params: { text: "02" }, |
Then we should be able to query the widget value like so
cy.url().should("include", "text=02");
filterWidget().findByText("02").should("be.visible");
Filter value with a leading zero is not invalid. It's common in ID columns to have leading zeros. Therefore the leading zero should not be removed when used as filter value. |
* Fix parameters parse * Add e2e test * Add e2e test
Closes #41483
Description
This fixes runtime issue with parsing embedded / public dashboard parameters that represent numbers with leading "0".
Mainly caused by https://github.com/metabase/metabase/pull/38271/files#diff-c472624b3ca71f585a7e771826cca7ae8434b170963e19f384cbde56d066c804R110-R113
How to verify
Follow steps from #41483
Demo
Upload a demo video or before/after screenshots if sensible or remove the section
Checklist