Skip to content

Conversation

@michellethomas
Copy link
Contributor

It looks like in this PR we updated the slug function to remove anything that is not a character, number, or hyphen. I think we should also allow underscores in the slug.

The wiki article mentions that underscores or hyphens would be appropriate in a slug.

A number of users here currently use underscores in slugs, especially for the templated links in the time table viz (like /superset/dashboard/dashboard_{{metric }}) where the metric would have an underscore). So when they edit dashboards the underscores get removed and their links no longer work.

@mistercrunch @john-bodley

Copy link
Member

@john-bodley john-bodley left a comment

Choose a reason for hiding this comment

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

@michellethomas _ isn't a special character and thus I don't think it should be escaped.

@michellethomas
Copy link
Contributor Author

Fixed, thanks

@mistercrunch
Copy link
Member

LGTM

@graceguo-supercat graceguo-supercat merged commit 34d6618 into apache:master Nov 29, 2017
@michellethomas michellethomas deleted the fix_slug_underscore branch May 22, 2018 18:15
michellethomas added a commit to michellethomas/panoramix that referenced this pull request May 24, 2018
* Allow underscores in slugs

* Switching regex to use shorter \w
wenchma pushed a commit to wenchma/incubator-superset that referenced this pull request Nov 16, 2018
* Allow underscores in slugs

* Switching regex to use shorter \w
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.21.0 First shipped in 0.21.0 labels Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 0.21.0 First shipped in 0.21.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants