Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed async problem when checking for unknown fields #3937

Merged
merged 5 commits into from Apr 4, 2022

Conversation

asteriscos
Copy link
Member

Hi team,

this pull request solves an asynchronism issue when multiple fields are missing in the Events view rows details. Once this PR is applied only 1 toast should pop informing the index pattern was updated.

Closes #3923

To test it try to reproduce the linked issue.

@asteriscos asteriscos added the 4.3 label Mar 31, 2022
@asteriscos asteriscos self-assigned this Mar 31, 2022
@Desvelao
Copy link
Member

Desvelao commented Apr 1, 2022

thought: I think we should change the flag to control that function is only executed one time, to a component instance variable instead of a state variable.
https://github.com/wazuh/wazuh-kibana-app/blob/fix/multiple-index-pattern-refresh-toast-3923/public/components/common/modules/events.tsx#L209-L211

Updating the state is an asynchronous method.

suggestion:
For example:

refreshKnownFields = async () => {
  if (!this.hasRefreshedKnownFields) {
    try {
      this.hasRefreshedKnownFields = true;
      this.isRefreshing = true;
      if (satisfyPluginPlatformVersion('<7.11')) {
        await PatternHandler.refreshIndexPattern();
      }
      this.isRefreshing = false;
      this.reloadToast();
    } catch (error) {
      this.isRefreshing = false;
      throw error;
    }
  } else if (this.isRefreshing) { // maybe this could be removed and the `isRefreshing` variable or does we expect the method could be called multiple times?
    await new Promise((r) => setTimeout(r, 150));
    await this.refreshKnownFields();
  }
};

I am not sure if we need the isRefreshing variable in this scenario that the refresh fields of the index pattern should be done after computing if this action is required.

If we use component instance variables here, it would be interesting to apply the same change to the similar action for the Dashboard tab when there is a unknown field in some visualization.

Copy link
Member

@Desvelao Desvelao left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

review:
Code review ✔️
Test: ✔️

@github-actions
Copy link
Contributor

github-actions bot commented Apr 4, 2022

Jest Test Coverage % values
Statements 4.04% ( 1476 / 36556 )
Branches 1.61% ( 459 / 28483 )
Functions 2.98% ( 266 / 8934 )
Lines 4.09% ( 1428 / 34948 )

@Desvelao Desvelao merged commit caeaa2c into 4.3-7.10 Apr 4, 2022
@Desvelao Desvelao deleted the fix/multiple-index-pattern-refresh-toast-3923 branch April 4, 2022 10:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Several "The index pattern was refreshed successfully" panel appears after restarting
2 participants