feat(cc-addon-*.smart-elastic): init #1609
Conversation
|
🔎 A preview has been automatically published : https://clever-components-preview.cellar-c2.services.clever-cloud.com/dashboard-addons/elastic/index.html. This preview will be deleted once this PR is closed. |
HeleneAmouzou
requested review from
Galimede,
florian-sanders-cc,
pdesoyres-cc and
roberttran-cc
florian-sanders-cc
left a comment
florian-sanders-cc
left a comment
There was a problem hiding this comment.
A few small fixes but overall great job for a tricky subject 🤯
src/components/cc-addon-credentials-beta/cc-addon-credentials-beta.smart-elastic.js
Outdated
Show resolved
Hide resolved
| updateComponent( | ||
| 'state', | ||
| /** @param {AddonCredentialsBetaStateLoaded|AddonCredentialsBetaStateLoading} state */ | ||
| (state) => { | ||
| state.type = 'loaded'; | ||
| // Build tabs object with only enabled services | ||
| /** @type {Record<string, {content: AddonCredential[], docLink: {text: string, href: string}}>} */ | ||
| const updatedTabs = { | ||
| elastic: { | ||
| ...state.tabs.elastic, | ||
| content: elastic, | ||
| }, | ||
| }; | ||
|
|
||
| if (enabledServices.kibana) { | ||
| updatedTabs.kibana = { | ||
| ...state.tabs.kibana, | ||
| content: kibana, | ||
| }; | ||
| } | ||
|
|
||
| if (enabledServices.apm) { | ||
| updatedTabs.apm = { | ||
| ...state.tabs.apm, | ||
| content: apm, | ||
| }; | ||
| } | ||
|
|
||
| state.tabs = updatedTabs; | ||
| }, | ||
| ); |
There was a problem hiding this comment.
praise + question: I really like the way you handled this tricky subjet, especially compared to the solution I initially implemented in the branch (with the Object.entries(Object.fromEntriers()) 💀).
I wonder if we really need the enabledServices stuff, if apm / Kibana is disabled, then kibana & apm will be null / undefined no? We could just check for that and remove the whole enabledServices object 🤔
There was a problem hiding this comment.
I have remove enabledServices and it seems cleaner, thanks !
| if (tabType === 'elastic') { | ||
| return [ | ||
| { | ||
| code: 'host', | ||
| value: addonProvider.config.host, | ||
| }, | ||
| { | ||
| code: 'user', | ||
| value: addonProvider.config.user, | ||
| }, | ||
| { | ||
| code: 'password', | ||
| value: addonProvider.config.password, | ||
| }, | ||
| ]; | ||
| } else if (tabType === 'kibana') { | ||
| return [ | ||
| { | ||
| code: 'user', | ||
| value: addonProvider.config.kibana_user, | ||
| }, | ||
| { | ||
| code: 'password', | ||
| value: addonProvider.config.kibana_password, | ||
| }, | ||
| ]; | ||
| } | ||
| return [ | ||
| { | ||
| code: 'user', | ||
| value: addonProvider.config.apm_user, | ||
| }, | ||
| { | ||
| code: 'password', | ||
| value: addonProvider.config.apm_password, | ||
| }, | ||
| { | ||
| code: 'token', | ||
| value: addonProvider.config.apm_auth_token, | ||
| }, | ||
| ]; |
There was a problem hiding this comment.
suggestion: try using a switch instead but I don't know if it's actually better or not, I feel like it could improve readability but I'm not sure.
I'm also unsure if this layer should know about the shape expected by the UI component (code + value) but I'm fine with it and I don't have a better idea so I would stick with that unless we get suggestions 👀
There was a problem hiding this comment.
I have changed it to use a switch, it does improve readability, thanks !
src/components/cc-addon-header/cc-addon-header.smart-elastic.js
Outdated
Show resolved
Hide resolved
| // Remove encryption since it's not part of the addon features | ||
| // TODO: reinstate encryption |
There was a problem hiding this comment.
nitpick: I remember we discussed this subject but is it still relevant?
There was a problem hiding this comment.
I forgot to deleted the comments after adding encryption
HeleneAmouzou
force-pushed
the
dashboard-addons/elastic
branch
from
7101f3c to
46371ab
Compare
roberttran-cc
left a comment
roberttran-cc
left a comment
There was a problem hiding this comment.
Great work, the code is clean, making the review really manageable.
Only a couple minor feedback for me.
What does this PR do?
index.html,cc-addon-header,cc-addon-infoandcc-addon-credentials-betafor Elasticsearch product.How to review?
demo-smart.The
.mddocumentation files has been coded by IA, even though I have checked and adjusted afterwards, please pay attention to it.TO DO