Chore: filter out more redundant scopes from ConfigurationVariablesBy… #753
Conversation
client/configuration_variable.go
Outdated
| var filteredVariables []ConfigurationVariable | ||
| for _, variable := range result { | ||
| if scope == variable.Scope { | ||
| filteredVariables = append(filteredVariables, variable) | ||
| } |
There was a problem hiding this comment.
that's not good enough 😅
as per the explanation in the issue #751
There'd be several variables from the same scope "type" from a different specific scope.
you'd get more variables from the API in the PROJECT scope, but which are NOT defined on the requested projectId.
Does that make sense?
There was a problem hiding this comment.
Each variable has a scope and a scopeId.
The scope can be ENVIRONMENT, PROJECT, etc.
The scopeIds are uuids fir the specific environment, project, etc. respectfully.
There was a problem hiding this comment.
so.. should I filter instead based on scope ID? (instead of scope).
github-actions
bot
added
ready to merge
TomerHeber
added
pending final review
reviewed - awaiting author
and removed
ready to merge
client/configuration_variable.go
Outdated
| // The API returns variables of upper scopes. Filter them out. | ||
| var filteredVariables []ConfigurationVariable | ||
| for _, variable := range result { | ||
| if scopeId == variable.ScopeId { |
There was a problem hiding this comment.
I would also add scope == variable.Scope here for safety
There was a problem hiding this comment.
@Wassap124 - do you concur?
There was a problem hiding this comment.
it's conceptually redundant but it won't harm + add explicitness to the code, so im not against it.
Also that way it'd be the exact same validation / predicate that's being made on env0 code post change so it'd be aligned with it
|
btw @TomerHeber we hadn't forgotten about this PR, we've started QAing this |
github-actions
bot
added
ready to merge
…Scope
Issue & Steps to Reproduce / Feature Request
closes #751
Solution