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

Handle empty strings in slugify macro #774

Closed
wants to merge 4 commits into from
Closed

Handle empty strings in slugify macro #774

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
85ac778
Handle empty strings in slugify macro
atvaccaro Mar 28, 2023
c9c35e8
update changelog with PR number
atvaccaro Mar 28, 2023
7e2acd6
fix slugify test
atvaccaro Mar 28, 2023
413227d
return empty string rather than none
atvaccaro Apr 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,10 +11,11 @@
# Unreleased
## Fixes
- get_relations_by_pattern for Databricks connection with Unity catalog #768 ([#768](https://github.com/dbt-labs/dbt-utils/issues/768), [#769](https://github.com/dbt-labs/dbt-utils/pull/769))
- Handle empty strings in slugify macro #773 ([#773](https://github.com/dbt-labs/dbt-utils/issues/773), [#774](https://github.com/dbt-labs/dbt-utils/pull/774))
Copy link
Contributor

Choose a reason for hiding this comment

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

We have a merge conflict that we need to resolve in CHANGELOG.md.

I wasn't able to push to the remote for some reason, so I'm going to try to remove these changes in order to resolve the merge conflict.

Suggested change
- Handle empty strings in slugify macro #773 ([#773](https://github.com/dbt-labs/dbt-utils/issues/773), [#774](https://github.com/dbt-labs/dbt-utils/pull/774))


## Contributors:
@Harmuth94, [#768](https://github.com/dbt-labs/dbt-utils/issues/768)

@atvaccaro, [#773](https://github.com/dbt-labs/dbt-utils/issues/773)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
@atvaccaro, [#773](https://github.com/dbt-labs/dbt-utils/issues/773)


# dbt utils v1.0

Expand Down
4 changes: 3 additions & 1 deletion integration_tests/tests/jinja_helpers/test_slugify.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
with comparisons as (
select '{{ dbt_utils.slugify("") }}' as output, '' as expected
union all
select '{{ dbt_utils.slugify("!Hell0 world-hi") }}' as output, 'hell0_world_hi' as expected
union all
select '{{ dbt_utils.slugify("0Hell0 world-hi") }}' as output, '_0hell0_world_hi' as expected
)

select *
from comparisons
where output != expected
where output != expected
6 changes: 5 additions & 1 deletion macros/jinja_helpers/slugify.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
{% macro slugify(string) %}

{% if not string %}
{{ return(string) }}
atvaccaro marked this conversation as resolved.
Show resolved Hide resolved
{% endif %}

{#- Lower case the string -#}
{% set string = string | lower %}
{#- Replace spaces and dashes with underscores -#}
Expand All @@ -11,4 +15,4 @@

{{ return(string) }}

{% endmacro %}
{% endmacro %}