wazuh-dashboard-plugins icon indicating copy to clipboard operation
wazuh-dashboard-plugins copied to clipboard

Add switch to toggle the use of custom branding

Open asteriscos opened this issue 3 years ago • 3 comments

Description

This PR adds a new configuration setting to enable or disable the whole branding customization. This makes it easier to switch on or off all the white-labeling configurations without changing or deleting all the custom branding configurations. Changing customization.enabled configuration to true or false is enough to deactivate all the branding customizations.

Closes https://github.com/wazuh/wazuh-kibana-app/issues/4506

Screenshot

wazuh-2022-09-14-12_59_25

Test

Manual tests

Scenario: Modify the value of customization.enabled setting through Settings/Configuration When the user modifies the custmization.enabled setting and save the configuration. Then the configuration file should be updated and the frontend

Scenario: Modify the value of customization.enabled setting through an API request with a valid value. When the user does an API request to update the setting customization.enabled with a valid value: true or false Then the response should be successful And the configuration file should be updated

Scenario: Modify the value of customization.enabled setting through an API request with an invalid value. When the user does an API request to update the setting customization.enabled with an invalid value. Then the response should be a bad request

Scenario: Customization is applied when the customization.enabled setting is enabled. Given the customization.enabled setting value is true and the customization.* settings have custom values When the user goes to the plugin, Then the menu displays the custom image And the plugin category displays the custom image And the plugin health check displays the custom image And the PDF report image displays the custom image And the PDF header displays the custom text And the PDF footer displays the custom text

Scenario: Customization is not applied when the customization.enabled setting is disabled. Given the customization.enabled setting value is false and the customization.* settings have custom values When the user goes to the plugin, Then the menu displays the default image And the plugin category displays the default image And the plugin health check displays the default image And the PDF report image displays the default image And the PDF header displays the default text And the PDF footer displays the default text

asteriscos avatar Sep 13 '22 16:09 asteriscos

I added some manual tests to the initial comment.

Desvelao avatar Sep 15 '22 09:09 Desvelao

Changes

  • Updated the development branch with the last changes of the base branch.

Desvelao avatar Sep 19 '22 15:09 Desvelao

Update

  • Update the development branch with the last changes of the base branch.

Desvelao avatar Sep 23 '22 09:09 Desvelao

Update

  • Add tests
  • Sync with the integration branch

Desvelao avatar Sep 27 '22 06:09 Desvelao

Update

  • Update the development branch with the base branch
  • Add a test for the customization.enabled setting in the form input

Desvelao avatar Oct 05 '22 11:10 Desvelao

Update

  • Update the development branch with the base branch

Desvelao avatar Oct 06 '22 09:10 Desvelao

Update

I updated the development branch with the last changes of the base branch and I fonund a problem.

The settings related to the customization of reports don't work, the PDF is printed using the default values. This is related to the getConfiguration backend service, that only gets the configuration defined in the file. When the customization.enabled setting is not defined, the service used to get the configuration depending on if the customization is enabled or not.

I was in a meeting with @asteriscos reviewing the code and we consider refactoring it. The service should return the mixed configuration: default and read in the configuration file. With this change, we can remove some code management done in some sections of the backend.

Desvelao avatar Oct 31 '22 13:10 Desvelao

Update

I am analyzing the problem mentioned above. https://github.com/wazuh/wazuh-kibana-app/pull/4507#issuecomment-1297115591

Due to the plugin having some settings with hidden default values, it seems that is not possible to refactor and simplify the getConfiguration service to return the plugin configuration. We have 2 different requirements:

  • In Settings/Configuration, the settings don't display the hidden default values.
  • Using the settings, the hidden default values are required.

For example, when the user has not defined the customization.reports.header setting, in Settings/Configuration it is displayed as an empty string while when it is used in the backend to build the PDF reports, for this case, this will have the [email protected]\nhttps://wazuh.com value.

The hidden default values exist because the approach of the recent plugins is that if the user configures these settings, then it will use these values. If these settings are not defined and have a hidden default value, then it will use them instead.

Desvelao avatar Oct 31 '22 14:10 Desvelao

I found other problems:

  • The customization.enabled setting is not used for the api/logos endpoint. It means, that if this is disabled, and the user has a customized sidebar logo, this will be displayed.
  • I found another complex problem, related to when customizing the customization.enabled setting, which could not display the required user action if the modified settings require it. The setting should take into account all the required actions when this setting is changed.

Desvelao avatar Oct 31 '22 15:10 Desvelao

There's an issue in public/plugin.ts when the app loads and makes a request to obtain any possible custom logos, it does not evaluate if the customization switch is enabled. The endpoint api/logos has to include the customization.enable conditional to centralize the source of truth.

asteriscos avatar Oct 31 '22 15:10 asteriscos

Update

The customization.logo.sidebar setting is changed without requiring reloading the browser tab. Related to https://github.com/wazuh/wazuh-kibana-app/pull/4507#issuecomment-1297271223

This solves an indirect problem when the customization.enabled setting is changed from the UI, which should take into account the rest of the customization.* settings to display some action.

This change enhances the UX because the change is instant and doesn't require reloading the browser tab.

Desvelao avatar Oct 31 '22 16:10 Desvelao

There's an issue in public/plugin.ts when the app loads and makes a request to obtain any possible custom logos, it does not evaluate if the customization switch is enabled. The endpoint api/logos has to include the customization.enable conditional to centralize the source of truth.

Fixed in https://github.com/wazuh/wazuh-kibana-app/pull/4507/commits/6ffbdb6aeef9bfd331d3059fd8b64316ee1ff624

asteriscos avatar Oct 31 '22 17:10 asteriscos

Update

  • This change was reverted https://github.com/wazuh/wazuh-kibana-app/pull/4507#issuecomment-1297354125. The customization.logo.sidebar requires reloading the browser tab when this is changed.
  • Add the reloading browser tab requirement when changing the customization.enabled setting.

I was researching how to avoid the user having to reload the browser tab when changing some of these settings, but the changes in the customization.enabled requires to check the value of the customization.logo.sidebar and changing or resetting to default the logo through the plugin state updater. The applied solution reduces the complexity of the logic.

Desvelao avatar Nov 02 '22 08:11 Desvelao

Code coverage (Jest) % values
Statements 8.31% ( 3044 / 36650 )
Branches 3.87% ( 1092 / 28214 )
Functions 7.29% ( 662 / 9084 )
Lines 8.36% ( 2933 / 35080 )

github-actions[bot] avatar Nov 02 '22 10:11 github-actions[bot]

The backport to 4.4-2.3-wzd failed:

The process '/usr/bin/git' failed with exit code 1

To backport manually, run these commands in your terminal:

# Fetch latest updates from GitHub
git fetch
# Create a new working tree
git worktree add .worktrees/backport-4.4-2.3-wzd 4.4-2.3-wzd
# Navigate to the new working tree
cd .worktrees/backport-4.4-2.3-wzd
# Create a new branch
git switch --create backport-4507-to-4.4-2.3-wzd
# Cherry-pick the merged commit of this pull request and resolve the conflicts
git cherry-pick -x --mainline 1 c1c39b359148f788985da17c2ff2b8b0530c19e9
# Push it to GitHub
git push --set-upstream origin backport-4507-to-4.4-2.3-wzd
# Go back to the original working tree
cd ../..
# Delete the working tree
git worktree remove .worktrees/backport-4.4-2.3-wzd

Then, create a pull request where the base branch is 4.4-2.3-wzd and the compare/head branch is backport-4507-to-4.4-2.3-wzd.

github-actions[bot] avatar Nov 02 '22 15:11 github-actions[bot]