-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
add button link to ingest #70142
add button link to ingest #70142
Conversation
320a875
to
604316a
Compare
Pinging @elastic/ingest-management (Team:Ingest Management) |
Pinging @elastic/endpoint-app-team (Feature:Endpoint) |
Pinging @elastic/endpoint-management (Team:Endpoint Management) |
@caitlinbetz @bfishel @kevinlog are we going to change the text to this? It seems a little less informative than the current info, but just wanted to make sure. |
@@ -8,6 +8,7 @@ import { FrameworkAdapter, FrameworkRequest } from '../framework'; | |||
import { SourceStatusAdapter } from './index'; | |||
import { buildQuery } from './query.dsl'; | |||
import { ApmServiceNameAgg } from './types'; | |||
import { ENDPOINT_METADATA_INDEX } from '../../../common/constants'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should work, but we'll also wanna make sure that the endpoint events and alerts index are added to the defaults. Can be in another PR
const read = services.application.capabilities.ingestManager?.read ?? false; | ||
|
||
// Check if all Ingest Manager permissions are enabled | ||
if (show === false || write === false || read === false) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit - could just return show === true && write === true && read === true
Alternatively, to make this more versatile in the future, you could return each of show
, write
, and read
in an object together and the dev could decide what to do with them.
export const EMPTY_ACTION_ENDPOINT = i18n.translate( | ||
'xpack.securitySolution.pages.common.emptyActionEndpoint', | ||
{ | ||
defaultMessage: 'Add data with Elastic Agent', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should probably note that this is Beta
Add data with Elastic Agent (Beta)
?
EDIT: also, maybe it shouldn't be the first button listed?
@parkiino I had a couple comments, the only one I think we should resolve before merging is the Button text and placement I pulled it down and it looks and works great! |
Hi @parkiino thanks! I just had one nit about the buttons I saw in the GIF. Per EUI guidelines for buttons, there should only be "one primary button per layout". I'd suggest changing the buttons to something like: If we want to "recommend" a specific action, like going through Ingest Manager, the first option could work. If we want to treat all actions equally, i'd recommend the 2nd option. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 for the Ingest changes. I left comments/suggestions but the changes in Ingest are small enough it should be easy enough to back out or update if we need to.
const initialCategory = | ||
queryParams.get('category') !== null ? (queryParams.get('category') as string) : ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is null
significant? If possible, I'd prefer
const initialCategory = | |
queryParams.get('category') !== null ? (queryParams.get('category') as string) : ''; | |
const initialCategory = queryParams.get('category') || ''; |
I find it more straightforward and we don't lose any safety, afaict.
// clear category query param in the url | ||
if (queryParams.get('category') !== null) { | ||
history.push({}); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@parkiino is null
significant here? Can we rely on "truthy"?
// clear category query param in the url | |
if (queryParams.get('category') !== null) { | |
history.push({}); | |
} | |
if (queryParams.get('category')) { |
cc @jen-huang and @neptunian for thoughts on dealing with the params/state
@elasticmachine merge upstream |
update security solution empty page
💔 Build Failed
Failed CI StepsTest FailuresKibana Pipeline / kibana-xpack-agent / re-orders columns via drag and drop.Events Viewer Events columns re-orders columns via drag and dropStandard Out
Stack Trace
Kibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/dashboard/_async_dashboard·ts.dashboard sample data dashboard "before all" hook for "should launch sample flights data set dashboard"Standard Out
Stack Trace
Kibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/dashboard/_async_dashboard·ts.dashboard sample data dashboard "after all" hook for "toggle from Discover to Dashboard attempt 2"Standard Out
Stack Trace
Build metrics@kbn/optimizer bundle module count
History
To update your PR or re-run it, just comment with: |
* master: (53 commits) [Composable template] Details panel + delete functionality (elastic#70814) [Uptime] Ping list body scroll (elastic#70781) moving indexPattern.delete() to indexPatterns.delete(indexPattern) (elastic#70430) Adapt expected response of advanced settings feature control for cloud tests (elastic#70793) skip flaky suite (elastic#70885) skip flaky suite (elastic#67814) skip flaky suite (elastic#70906) Revert "reenable regression and classification functional tests (elastic#70661)" (elastic#70908) Added UI validation when creating a Webhook connector with invalid URL (elastic#70025) [Security Solution] Change default index pattern (elastic#70797) ServiceNow push to Incident generic implementation (supporting both Case specific and generic Alerts) (elastic#68464) add button link to ingest (elastic#70142) reenable regression and classification functional tests (elastic#70661) [Component templates] Form wizard (elastic#69732) [Ingest Manager] Copy changes (elastic#70828) Adding test user to maps functional tests - PR 1 (elastic#70649) [Ingest Manager] Support limiting integrations on an agent config (elastic#70542) skip flaky suite (elastic#70880) [Metrics UI] Fix a bug in Metric Threshold query filter construction (elastic#70672) upgrade caniuse-lite database (elastic#70833) ...
* master: (46 commits) [Composable template] Details panel + delete functionality (elastic#70814) [Uptime] Ping list body scroll (elastic#70781) moving indexPattern.delete() to indexPatterns.delete(indexPattern) (elastic#70430) Adapt expected response of advanced settings feature control for cloud tests (elastic#70793) skip flaky suite (elastic#70885) skip flaky suite (elastic#67814) skip flaky suite (elastic#70906) Revert "reenable regression and classification functional tests (elastic#70661)" (elastic#70908) Added UI validation when creating a Webhook connector with invalid URL (elastic#70025) [Security Solution] Change default index pattern (elastic#70797) ServiceNow push to Incident generic implementation (supporting both Case specific and generic Alerts) (elastic#68464) add button link to ingest (elastic#70142) reenable regression and classification functional tests (elastic#70661) [Component templates] Form wizard (elastic#69732) [Ingest Manager] Copy changes (elastic#70828) Adding test user to maps functional tests - PR 1 (elastic#70649) [Ingest Manager] Support limiting integrations on an agent config (elastic#70542) skip flaky suite (elastic#70880) [Metrics UI] Fix a bug in Metric Threshold query filter construction (elastic#70672) upgrade caniuse-lite database (elastic#70833) ...
* actions/feature: (46 commits) [Composable template] Details panel + delete functionality (elastic#70814) [Uptime] Ping list body scroll (elastic#70781) moving indexPattern.delete() to indexPatterns.delete(indexPattern) (elastic#70430) Adapt expected response of advanced settings feature control for cloud tests (elastic#70793) skip flaky suite (elastic#70885) skip flaky suite (elastic#67814) skip flaky suite (elastic#70906) Revert "reenable regression and classification functional tests (elastic#70661)" (elastic#70908) Added UI validation when creating a Webhook connector with invalid URL (elastic#70025) [Security Solution] Change default index pattern (elastic#70797) ServiceNow push to Incident generic implementation (supporting both Case specific and generic Alerts) (elastic#68464) add button link to ingest (elastic#70142) reenable regression and classification functional tests (elastic#70661) [Component templates] Form wizard (elastic#69732) [Ingest Manager] Copy changes (elastic#70828) Adding test user to maps functional tests - PR 1 (elastic#70649) [Ingest Manager] Support limiting integrations on an agent config (elastic#70542) skip flaky suite (elastic#70880) [Metrics UI] Fix a bug in Metric Threshold query filter construction (elastic#70672) upgrade caniuse-lite database (elastic#70833) ...
update security solution empty page (#70142) Co-authored-by: Elastic Machine <[email protected]>
Summary
In a future PR, need to
https://github.com/elastic/endpoint-app-team/issues/453
BEFORE
AFTER
Checklist
Delete any items that are not applicable to this PR.
For maintainers