Bugfix/5405 parent child tenant update #5474
Merged
Conversation
This file contains 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
added 3 commits
January 27, 2021 16:06
As it stands currently, there is a bug where you can update a Tenant and select a child of said tenant to be the new parent. This causes a circular reference in the database with the adjanceny list and then prevents the child and parent from being returned in the recursive CTE, which makes it appear as though those values have been deleted. > Note that should this happen it is still possible to salvage the data by manually editing the `tenants` table and fixing the incorrect parent/child relationship. To prevent the problem, a check is done to ensure the chosen ParentID does not reside in the current list of children when attempting to update the Tenant.
mattjackson220
requested changes
Jan 28, 2021
Comment on lines
214
to
216
| usererr, syserr, statusCode := ten.isUpdatable() | ||
| if usererr != nil || syserr != nil { | ||
| return usererr, syserr, statusCode |
There was a problem hiding this comment.
just for consistency in the project, would you mind changing these to userErr and sysErr? and in a couple places down below as well? sorry for such a nitpicky comment
There was a problem hiding this comment.
Hey! Great catch. I had it in my head to dbl chk before submitting, but apparently missed it. Thanks for getting that.
mattjackson220
approved these changes
Jan 29, 2021
16 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
What does this PR (Pull Request) do?
Which Traffic Control components are affected by this PR?
No documentation is required as this is a bug fix to align with expected behavior.
What is the best way to verify this PR?
A Go Unit Test has been added to ensure this behavior is ensured in the future.
Manual testing can be performed for verification as well.
If this is a bug fix, what versions of Traffic Control are affected?
The following criteria are ALL met by this PR
Additional Information
Some additional cleanup was performed as well, such as using the same name for the receiver throughout
tenant.goand comments added.