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

Custom Expression name containing brackets ([ or ]) can break the UI #15316

Closed
flamber opened this issue Mar 24, 2021 · 5 comments · Fixed by #20927
Closed

Custom Expression name containing brackets ([ or ]) can break the UI #15316

flamber opened this issue Mar 24, 2021 · 5 comments · Fixed by #20927
Assignees
@ariya
Labels
.Frontend Priority:P2 Average run of the mill bug Querying/Notebook/Custom Expression .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Milestone
0.43

Comments

@flamber
Copy link
Contributor

flamber commented Mar 24, 2021

Describe the bug
When a Custom Expression uses a name containing brackets ([ or ]), then autocomplete dropdown stops working and forcefully trying to use that expression again in another expression will break the UI until browser refresh (or show "Something went wrong").

To Reproduce

  1. Custom question > Sample Dataset > Orders
  2. Custom Column formula 1 + 1 with the name MyCC [2021]
  3. Summarize > Custom Expression > type Sum([ notice there's no field dropdown
    image
  4. Summarize > Sum of > pick the MyCC [2021]
  5. Click the metric again, and click the title to go back to list of metrics and click Custom Expression - the UI is now broken and requires a browser refresh to function again (on upcoming 0.39 master, it will reset everything done in the Notebook and show "Something went wrong" in the lower-right corner)
    image

There are a lot of browser console errors

Firefox: TypeError: e._wrapperState is null
Chrome: Uncaught Error: String currently can't contain brackets: MyCC [2021]      <-- hey, why didn't it tell me that in the UI?

Information about your Metabase Installation:
Tested 0.38.2 and master

Severity
Giving P2, since it breaks UI (or clears everything on master), even though it's an easy workaround - don't use brackets.
Are there other characters that could cause similar problem?
@flamber flamber added Type:Bug Product defects Priority:P2 Average run of the mill bug Querying/Notebook Items specific to the Custom/Notebook query builder labels Mar 24, 2021
nemanjaglumac added a commit that referenced this issue Mar 30, 2021
@nemanjaglumac
Add repro for #15316
a6c2735
@nemanjaglumac nemanjaglumac mentioned this issue Mar 30, 2021
Merged
nemanjaglumac added a commit that referenced this issue Mar 30, 2021
@nemanjaglumac
#15316 Repro: Custom Expression name containing brackets can break th…
aabf30e 
…e UI (#15396)
@nemanjaglumac nemanjaglumac added the .Reproduced Issues reproduced in test (usually Cypress) label Mar 30, 2021
tsmacdonald pushed a commit that referenced this issue Apr 2, 2021
@nemanjaglumac @tsmacdonald
#15316 Repro: Custom Expression name containing brackets can break th…
3a9fbb1 
…e UI (#15396)
@howonlee howonlee self-assigned this Apr 5, 2021
@howonlee
Copy link
Contributor

howonlee commented Apr 5, 2021

confirming different-but-same brokenness for parens

@howonlee howonlee mentioned this issue Apr 12, 2021
Closed
@ariya
Copy link
Contributor

ariya commented May 5, 2021

This is currently the limitation of the lexer used in the Chevrotain-based parser, it does not understand a field reference that contains brackets or parentheses or other special characters (which must be escaped), e.g. [MyCC \[2021\]].

@ariya
Copy link
Contributor

ariya commented May 27, 2021

Now that I think about this problem, maybe the solution is to prevent the use of these special characters as the name of any custom column. It's going to cumbersome anyway to use a name that has brackets, even if character escaping is supported.

Thus, the plan is: if the custom column name contains one ore more special characters, show an error message and don't let the user continue.

Does that sound like a good compromise?

@howonlee
Copy link
Contributor

howonlee commented May 27, 2021 •
edited
Loading

Doesn't this one affect non-custom columns too? Quote delimited columns in postgres can have anything - brackets, emoji...

@flamber
Copy link
Contributor Author

flamber commented May 27, 2021

@ariya Sounds great, but as @howonlee mentions - this can also happen via the display name changed in Admin > Data Model.
Just tried Subtotal [2010] - then no suggestions are shown, when trying to summarize and console borks with:
Error: String currently can't contain brackets: Subtotal [2010]

@ariya ariya added the .Frontend label Jun 2, 2021
@flamber flamber added Querying/Notebook/Custom Expression and removed Querying/Notebook Items specific to the Custom/Notebook query builder labels Nov 15, 2021
@flamber flamber mentioned this issue Feb 11, 2022
Closed
@ariya ariya linked a pull request Mar 9, 2022 that will close this issue
Custom expression: escape/unescape field name #20927
Merged
@flamber flamber added this to the 0.43 milestone Mar 11, 2022
@ariya ariya mentioned this issue Mar 18, 2022
Closed
@flamber flamber assigned ariya and unassigned howonlee Mar 18, 2022
@deploysentinel deploysentinel bot mentioned this issue Nov 21, 2022
Merged
@deploysentinel deploysentinel bot mentioned this issue Dec 1, 2022
Closed
1 task
This was referenced Dec 12, 2022
Merged
Closed
This was referenced May 2, 2024
Merged
Merged
Closed
Merged
Merged
Merged
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Merged
Closed
Merged
Merged
Merged
Merged
Merged
Merged
Merged
@replay-io replay-io bot mentioned this issue May 9, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@ariya ariya

Labels
.Frontend Priority:P2 Average run of the mill bug Querying/Notebook/Custom Expression .Reproduced Issues reproduced in test (usually Cypress) Type:Bug Product defects
Projects
None yet
Milestone
0.43
Development

Successfully merging a pull request may close this issue.

Custom expression: escape/unescape field name metabase/metabase
4 participants
@ariya @howonlee @flamber @nemanjaglumac