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

Add tag validation #855

Merged
merged 7 commits into from
Oct 4, 2022
Merged

Add tag validation #855

merged 7 commits into from
Oct 4, 2022

Conversation

EMaksy
Copy link
Member

@EMaksy EMaksy commented Sep 29, 2022

Description

This pr adds:

  • frontend tag validation
  • backend tag validation
  • new test for wrong tags

When a user wants to create a tag in the frontend, every character of the input is validated.
In the case a forbidden character was used, the frontend will reject the input and don't add the character.
Additionally, a backend validation was added to ensure that the backend is protected when a user tries directly to interact with the backend.

Allowed characters are letters, numbers representable in UTF-8, and the following characters: + - = . , _ : @
This is leaned to the aws tag guidelines, for instance tags
See here: https://docs.aws.amazon.com/AWSEC2/latest/UserGuide/Using_Tags.html

Fixes #464

How was this tested?

Existing tests were adjusted to the new requirements.
A new test was added to check if the wrong tags are handled.

@EMaksy EMaksy added bug Something isn't working enhancement New feature or request labels Sep 29, 2022
@EMaksy EMaksy self-assigned this Sep 29, 2022
Remove overseen IO.inspect
@CDimonaco
Copy link
Member

The frontend has some sort of error message on the failed validation?

@EMaksy
Copy link
Member Author

EMaksy commented Sep 29, 2022

Currently, the frontend does not give user feedback on wrong input.
An UI pop feedback was already discussed and needs to be refined for the future implementation.

@EMaksy EMaksy force-pushed the main branch 2 times, most recently from a8b0352 to a886dce Compare September 29, 2022 13:47
@rtorrero
Copy link
Contributor

The frontend has some sort of error message on the failed validation?

I wouldn't add any message, the fact that the invalid character simply doesn't appear on the input box seems to me that it should give enough feedback. A message could be a bit annoying in this context, wdyt?

@EMaksy
Copy link
Member Author

EMaksy commented Sep 30, 2022

So for me as a user, when I try to create a tag and my input was not added, I get the feeling that something is not working properly. A popup could inform me and show me the allowed character. So I directly can do it right on the second try.

For now this works fine, but I would suggest adding some user feedback in the future.

What do you think, guys?

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.

My thoughts:

  • The eslint ignore is fine for now, tinkering with this doesn't stop us merging
  • The visual feedback is a lot of work that we can track in a further user story to refine

LGTM

@EMaksy EMaksy merged commit c11cf42 into trento-project:main Oct 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Disallow certain chars in tags
4 participants