Automatically Enable/disable depenencies tab #311
Conversation
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
|
This change is |
pavolloffay
changed the title
Codecov Report
@@ Coverage Diff @@
## master #311 +/- ##
==========================================
- Coverage 88.68% 88.63% -0.05%
==========================================
Files 70 70
Lines 3156 3169 +13
==========================================
+ Hits 2799 2809 +10
- Misses 244 246 +2
- Partials 113 114 +1
Continue to review full report at Codecov.
|
| }, | ||
| { | ||
| j: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"es-archive.enabled": "true"})}}, | ||
| expected: &v1.JaegerSpec{Storage: v1.JaegerStorageSpec{Options: v1.NewOptions(map[string]interface{}{"es-archive.enabled": "true"})}, | ||
| UI: v1.JaegerUISpec{Options: v1.NewFreeForm(map[string]interface{}{"archiveEnabled": true})}}, | ||
| UI: v1.JaegerUISpec{Options: v1.NewFreeForm(map[string]interface{}{"archiveEnabled": true, "dependencies": map[string]interface{}{"menuEnabled": false}})}}, |
There was a problem hiding this comment.
Isn't this wrong - if the storage type is not defined, it should default to memory, which should show the dependencies tab. The expected results is showing it disabled.
There was a problem hiding this comment.
In general yes, but this tests only normalizeUI which works on any storage
Signed-off-by: Pavol Loffay <[email protected]>
Signed-off-by: Pavol Loffay <[email protected]>
pkg/strategy/controller.go
Outdated
|
|
||
| func disableDependenciesTab(uiOpts map[string]interface{}, storage string, depsEnabled *bool) { | ||
| // dependency tab is by default enabled and memory storage support it | ||
| if strings.EqualFold(storage, "memory") || (depsEnabled != nil && *depsEnabled == false) { |
There was a problem hiding this comment.
Just realised, with the additional test, if dependencies are explicitly disabled, then menuEnabled should be false?
There was a problem hiding this comment.
so should the condition be if strings.EqualFold(storage, "memory") || depsEnabled == nil || *depsEnabled == true) {?
There was a problem hiding this comment.
You are right, changing
Signed-off-by: Pavol Loffay <[email protected]>
| @@ -161,7 +161,7 @@ func enableArchiveButton(uiOpts map[string]interface{}, sOpts map[string]string) | |||
|
|
|||
| func disableDependenciesTab(uiOpts map[string]interface{}, storage string, depsEnabled *bool) { | |||
| // dependency tab is by default enabled and memory storage support it | |||
| if strings.EqualFold(storage, "memory") || (depsEnabled != nil && *depsEnabled == false) { | |||
| if strings.EqualFold(storage, "memory") || (depsEnabled != nil && *depsEnabled == true) { | |||
There was a problem hiding this comment.
I think it needs to be ... || depsEnabled == nil || *depsEnabled == true - as if the depsEnabled is nil, then it means dependencies are enabled by default, doesn't it? So UI config shouldn't be changed.
There was a problem hiding this comment.
The normalization of deps spec runs before - so we assuming correct settings here
Signed-off-by: Pavol Loffay <[email protected]>
|
@pavolloffay LGTM - although there is a test failure. |
Signed-off-by: Pavol Loffay <[email protected]>
Resolves #163