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

Added Sample Data Component #3475

Merged
merged 11 commits into from Jul 27, 2021

Conversation

mpRegalado
Copy link
Member

Closes #3423
Created a component to check for sample data and display a message if it does.

This component also replaces the implementation in WzVisualize

To test:

  • Any component that uses WzVisualize, such as security events, should display a warning about sample data if it is active, and should not do it if it isn't.

@mpRegalado mpRegalado self-assigned this Jul 14, 2021
@mpRegalado mpRegalado linked an issue Jul 14, 2021 that may be closed by this pull request
2 tasks
Copy link
Contributor

@gabiwassan gabiwassan left a comment

Choose a reason for hiding this comment

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

Just some comments, great job!! 💯

public/components/visualize/components/sample-data.js Outdated Show resolved Hide resolved
public/components/visualize/components/sample-data.js Outdated Show resolved Hide resolved
public/components/visualize/components/sample-data.js Outdated Show resolved Hide resolved
public/components/visualize/components/sample-data.test.js Outdated Show resolved Hide resolved
public/components/visualize/components/sample-data.test.js Outdated Show resolved Hide resolved
title: error.name || error,
},
};
getErrorOrchestrator.mockImplementation(() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

🚀

public/components/visualize/components/sample-data.test.js Outdated Show resolved Hide resolved
public/components/visualize/wz-visualize.js Outdated Show resolved Hide resolved
Copy link
Contributor

@gabiwassan gabiwassan left a comment

Choose a reason for hiding this comment

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

LGTM ! 🚀

Copy link
Contributor

@pablomarga pablomarga left a comment

Choose a reason for hiding this comment

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

There should be an space between Go and here.
Screenshot_20210715_124522

Comment on lines 22 to 42
const usesSampleData = async () => {
try {
const result = (await WzRequest.genericReq('GET', '/elastic/samplealerts', {})).data
.sampleAlertsInstalled;
setIsSampleData(result);
} catch (error) {
const options = {
context: `${SampleData.name}.usesSampleData`,
level: UI_LOGGER_LEVELS.ERROR,
severity: UI_ERROR_SEVERITIES.UI,
error: {
error: error,
message: error.message || error,
title: error.name || error,
},
};
getErrorOrchestrator().handleError(options);
}
};
useEffect(() => {
usesSampleData();
Copy link
Member

Choose a reason for hiding this comment

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

nitpick: I would avoid the prefix use for a function, this prefix is usually used for hooks.

You could move the usesSampleData to the useEffect. You could use the async/await with a Self Invoking Function as:

useEffect(() => {
  (async(){
    // logic
  })()
},[dependencies])

Copy link
Member Author

Choose a reason for hiding this comment

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

Applied in the latest commit

Copy link
Contributor

@pablomarga pablomarga left a comment

Choose a reason for hiding this comment

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

Testing: ✔️
CR: ✔️

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 ✅

@Desvelao Desvelao added the type/enhancement Enhancement issue label Jul 26, 2021
@github-actions
Copy link
Contributor

Jest Test Coverage % values
Statements 3.84% ( 1370 / 35666 )
Branches 1.48% ( 417 / 28107 )
Functions 2.66% ( 229 / 8618 )
Lines 3.89% ( 1325 / 34097 )

@mpRegalado mpRegalado changed the base branch from 4.3-7.10 to feature/Office365 July 27, 2021 07:59
@mpRegalado mpRegalado changed the title Added Sample Data Componet Added Sample Data Component Jul 27, 2021
@mpRegalado mpRegalado changed the base branch from feature/Office365 to 4.3-7.10 July 27, 2021 09:13
@mpRegalado mpRegalado changed the base branch from 4.3-7.10 to feature/Office365 July 27, 2021 09:13
@mpRegalado mpRegalado merged commit 06a2358 into feature/Office365 Jul 27, 2021
@mpRegalado mpRegalado deleted the enhancement/3423_sample-data-component branch July 27, 2021 09:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement Enhancement issue
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Create a component that checks for sample data
4 participants