asset group tags to not contain whitespaces #437
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
irshadaj
force-pushed
the
assetgroup_tag_validations
branch
2 times, most recently
from
06ead89 to
be6a9fb
Compare
irshadaj
force-pushed
the
assetgroup_tag_validations
branch
from
36bac5a to
d017a4e
Compare
mistahj67
reviewed
Feb 27, 2024
superlinkx
reviewed
Feb 27, 2024
|
|
||
| if err := api.ReadJSONRequestPayloadLimited(&createRequest, request); err != nil { | ||
| api.WriteErrorResponse(request.Context(), api.BuildErrorResponse(http.StatusBadRequest, err.Error(), request), response) | ||
| } else if strings.Contains(createRequest.Tag, " ") { |
Contributor
There was a problem hiding this comment.
Quick question: should we disallow all forms of whitespace? It seems like it may be a good idea just to be safe. We'd probably use a regex to test for any whitespace character, rather than enumerate whitespace checks manually.
Contributor
Author
There was a problem hiding this comment.
Good call, I'll add this in a follow up MR.
Contributor
There was a problem hiding this comment.
We may want to make this v5.8.0 due to the next version definitely being a feature update
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Whitespaces in asset group tags conflict with delimiters in the "system_group" properties. This MR adds a check to disallow spaces in the tags field when creating an asset group. It also adds a database migration to update any existing asset group tags and trim the whitespace out.
How Has This Been Tested?
Added unit test for validation
Manual test for database migration
Screenshots (if appropriate):
Set up the database with one asset group containing a space in the tag
Ran the update to remove spaces
Verified that the spaces are removed from id=3
Types of changes
Checklist: