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

Disallow empty tags #364

Merged
merged 6 commits into from
Apr 19, 2022
Merged

Disallow empty tags #364

merged 6 commits into from
Apr 19, 2022

Conversation

nelsonkopliku
Copy link
Member

@nelsonkopliku nelsonkopliku commented Apr 15, 2022

Makes the backend a bit more rebust when creating tags.

Frontend couterpart has not been addressed here.

@@ -9,7 +9,9 @@ defmodule Trento.Tags do

alias Trento.Repo

@spec create_tag(String.t(), Ecto.UUID.t(), String.t()) ::
@type taggable_resource :: :host | :cluster | :sap_system | :database
Copy link
Member

Choose a reason for hiding this comment

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

defp empty?(<<" "::binary, rest::binary>>), do: empty?(rest)
defp empty?(""), do: true
defp empty?(_string), do: false

Copy link
Member

Choose a reason for hiding this comment

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

not sure what this does? couldn't you check the length of the string?

Copy link
Member Author

Choose a reason for hiding this comment

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

this guy checks for empty and blank strings 😄

Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

LGTM

@nelsonkopliku nelsonkopliku merged commit c2d1ba1 into main Apr 19, 2022
@nelsonkopliku nelsonkopliku deleted the disallow_empty_tags branch April 19, 2022 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants