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.

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

Don't use the slugify util for filenames #42475

Merged
merged 4 commits into from May 13, 2024
Merged

Conversation

adam-james-v
Copy link
Contributor

Fixes: #41669

Since PR #39120 , the slugify util was used to make card names URL safe. This was done for turning things like spaces into underscores. But, it unfortunately encoded many characters into URL safe codes, which doesn't make for a nice filename.

This PR just removes the use of slugify when creating filenames, so that the characters can remain readable.

@adam-james-v adam-james-v requested a review from camsaul as a code owner May 9, 2024 19:06
@metabase-bot metabase-bot bot added the .Team/DashViz Dashboard and Viz team label May 9, 2024
Copy link

replay-io bot commented May 9, 2024

Status In Progress ↗︎ 51 / 52
Commit dbceb78
Results
⚠️ 3 Flaky
2484 Passed

@adam-james-v adam-james-v added the backport Automatically create PR on current release branch on merge label May 10, 2024
Copy link
Contributor

@escherize escherize left a comment

Choose a reason for hiding this comment

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

LGTM

@adam-james-v adam-james-v merged commit 127a62f into master May 13, 2024
109 checks passed
@adam-james-v adam-james-v deleted the bugfix-41669-filename-fix branch May 13, 2024 20:21
Copy link

@adam-james-v Did you forget to add a milestone to the issue for this PR? When and where should I add a milestone?

github-actions bot pushed a commit that referenced this pull request May 13, 2024
* Don't use the slugify util for filenames

* Fix tests that looked for incorrect filenames

* add a test

* fix filename in pulse test util
metabase-bot bot added a commit that referenced this pull request May 13, 2024
* Don't use the slugify util for filenames

* Fix tests that looked for incorrect filenames

* add a test

* fix filename in pulse test util

Co-authored-by: adam-james <21064735+adam-james-v@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Automatically create PR on current release branch on merge .Team/DashViz Dashboard and Viz team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

After #39120 , Dashboard subscription's Japanese filename of attachment is broken
2 participants