Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Lib.expressionClauseForLegacyExpression throws when using a multi-value string/contains filter #43990

Closed
kamilmielnik opened this issue Jun 11, 2024 · 3 comments · Fixed by #45599
Closed

Lib.expressionClauseForLegacyExpression throws when using a multi-value string/contains filter #43990

kamilmielnik opened this issue Jun 11, 2024 · 3 comments · Fixed by #45599
Assignees
@ranquild
Labels
.Frontend Priority:P2 Average run of the mill bug Querying/Drill Thrus Refining existing queries with Drill Thrus Querying/MBQL Reporting/Dashboards .Team/QueryingComponents Type:Bug Product defects
Milestone
0.50.14

Comments

@kamilmielnik
Copy link
Contributor

kamilmielnik commented Jun 11, 2024 •
edited by ranquild
Loading

Thread with info https://metaboat.slack.com/archives/C0645JP1W81/p1718109273559439

Describe the bug

2024-06-11.15-38-53.mp4

To Reproduce

  1. Create a question Orders > Limit 5
  2. Add it to a new dashboard
  3. Add Text > Contains filter to the dashboard and connect it to Source field in the card
  4. Save dashboard
  5. Set 2 filter values: "aa" and "bb"
  6. Click dashcard title

Nothing happens.
There's an error in JS console:

Uncaught (in promise) Error: No protocol method IAssociative.-assoc defined for type number: 14

Expected behavior

No error in JS console.
You get navigated to question page.

Information about your Metabase installation

master, 4594299, v50+ (we didn't support multiple values in these filters before)

Severity

P1/P2
@dosubot dosubot bot added .Frontend Querying/Parameters & Variables Filter widgets, field filters, variables etc. labels Jun 11, 2024
kamilmielnik added a commit that referenced this issue Jun 11, 2024
@kamilmielnik
Add a workaround for #43990
08a5606
@kamilmielnik
Copy link
Contributor Author

This issue happens when computing URL for the dashcard title.
It has not been caught by tests because you need to click such link to get the error.

However #43816 adds link computation even before the click event happens, so this bug prevails there.
I had to add a temporary try/catch to work around it (see 08a5606)

⚠️ Please revert that commit when issue is fixed ⚠️

kamilmielnik added a commit that referenced this issue Jun 11, 2024
@kamilmielnik
Copy question link from dashboard (#43816)
c0a804b 
* Improve typing in Question

* Remove dead CSS

* Improve typing in getUrlWithParameters

* Add LegendLabel

* Drop navigateToNewCardFromDashboard in PublicOrEmbeddedDashboard

* Extract getNewCardUrl and move getParametersMappedToDashcard closer to it

* Add link to dashboard chart title

* Update title-drill tests

* Format code

* Update test

* Add HrefMemo

* Use state to memoize href

* Recompute state on getHref prop change

* Format code

* Refactor href computation

* Update test

* Use selector instead of useMemo

* Fix unit tests

* Ensure type safety

* Revert getDashCardById type change to not make this PR too large

* Use complete dashboard

* Add a workaround for #43990
@kamilmielnik kamilmielnik added Querying/Drill Thrus Refining existing queries with Drill Thrus Reporting/Dashboards labels Jun 12, 2024
@kamilmielnik kamilmielnik mentioned this issue Jun 12, 2024
Closed
@kamilmielnik
Copy link
Contributor Author

There's a repro for it already, see: #43000
⚠️ When this issue is fixed please unskip it and update the issue id in the test name ⚠️

@ranquild
Copy link
Contributor

As discussed, for legacy mbql only we need to pass args differently on the FE.

@ranquild ranquild added the .Already Fixed will apear in "Already Fixed" in the release notes (not "Bug Fixes" that we actively fixed) label Jul 15, 2024
@ranquild ranquild self-assigned this Jul 15, 2024
@ranquild ranquild removed the .Already Fixed will apear in "Already Fixed" in the release notes (not "Bug Fixes" that we actively fixed) label Jul 15, 2024
@ranquild ranquild mentioned this issue Jul 15, 2024
Merged
@github-actions github-actions bot added this to the 0.50.14 milestone Jul 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ranquild ranquild

Labels
.Frontend Priority:P2 Average run of the mill bug Querying/Drill Thrus Refining existing queries with Drill Thrus Querying/MBQL Reporting/Dashboards .Team/QueryingComponents Type:Bug Product defects
Projects
None yet
Milestone
0.50.14
Development

Successfully merging a pull request may close this issue.

Fix string filters with multiple values in dashboards metabase/metabase
2 participants
@kamilmielnik @ranquild