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

[Security Solution]Missing Inspect modal Title for all 3 view of Alerts charts #136372

Closed
ghost opened this issue Jul 14, 2022 · 6 comments
Closed
Assignees
Labels
bug Fixes for quality problems that affect the customer experience fixed impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. QA:Validated Issue has been validated by QA Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team v8.4.0

Comments

@ghost
Copy link

ghost commented Jul 14, 2022

Describe the bug
Missing Inspect modal Title for all 3 view of Alerts charts

Build Details:

Version:8.4.0-SNAPSHOT
Commit:8a945789baa48244340cdff5fdf45fc9f67d3ad3
Build:54479

Steps

  • Create a kibana and have a Alert Generated on it
  • Go to Alert Page and select trend view and click on three dot in top right corner of chart
  • click on inspect button
  • Observed that Chart view that is Trend in not there in inspect modal title
  • Same issue is occuring for Table and Tree map

Expected Result
In 8.3.0 we used to show Trend in title of inspect modal

image

Screen-Shoot

image

image

image

@ghost ghost added bug Fixes for quality problems that affect the customer experience triage_needed Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. labels Jul 14, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-solution (Team: SecuritySolution)

@ghost ghost added the impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. label Jul 14, 2022
@ghost ghost assigned ghost and MadameSheema and unassigned ghost Jul 14, 2022
@MadameSheema MadameSheema added Team:Threat Hunting Security Solution Threat Hunting Team Team:Threat Hunting:Investigations Security Solution Investigations Team labels Jul 14, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/security-threat-hunting (Team:Threat Hunting)

andrew-goldstein added a commit to andrew-goldstein/kibana that referenced this issue Aug 1, 2022
This PR contains fixes for the following issues:

- elastic#136377 was fixed by using the `key_as_string` value in the search strategy response to format timestamps for the Treemap and Table views, per the screenshots below:

**Treemap with timestamp formatting:**

![treemap_with_timestamp_formatting](https://user-images.githubusercontent.com/4459398/182256532-7df8d39b-4e09-440f-aae7-c6e3720e0906.png)

**Table with timestamp formatting:**

![table_with_timestamp_formatting](https://user-images.githubusercontent.com/4459398/182256817-ee8605a7-d9d3-4127-8753-aaad65832308.png)

- elastic#136387 was fixed by reducing the mininum font size to `4px` for group names, per [this example](https://elastic.github.io/elastic-charts/?path=/story/treemap--two-layers-stress-test&globals=theme:light;background:white) from Elastic charts. This change:
  - Reduces the chances of a label getting hidden when the browser is resized, but cannot eliminate it, because at a certain point, Elastic Charts must hide the label in order to (still) render the group
  - Will result in some groups having very small labels where the group previously would not have them, per the screenshot below:

![4px_labels](https://user-images.githubusercontent.com/4459398/182255505-bc77c184-df5a-4ff6-830a-c7c5b6127d25.png)

- elastic#136372 was fixed by adding the word `Trend`, `Table`, and `Treemap` to the inspect modal for each of the respective chart types, per the screenshots below:

**Inspect Trend:**

![inspect_trend](https://user-images.githubusercontent.com/4459398/182257336-5d988e6a-a436-48b6-ad59-72b64e86d89b.png)

**Inspect Table:**
![inspect_table](https://user-images.githubusercontent.com/4459398/182257396-6e0d01e3-e493-4765-ac60-fee790a52633.png)

**Inspect Treemap:**
![inspect_treemap](https://user-images.githubusercontent.com/4459398/182257457-d3b58f18-e393-4307-b319-a2e1019b0329.png)

- [review feedback](elastic#126896 (review)) where the `Tree map isn't displayed for certain group by top fields, like Ransomeware.version`, was fixed as suggested by displaying an empty state message with a reason why the treemap cannot be displayed:

**Before:**
![empty-state-before](https://user-images.githubusercontent.com/2946766/178567785-82d03c51-3e3c-49f6-ac86-d13ecaf3aef0.png)

**After:**
![empty-state-after](https://user-images.githubusercontent.com/4459398/182254376-437d3ce8-c4bd-4b43-a456-2a4460db4565.png)

- [review feedback](elastic#126896 (review)) where there were `Extra left margins when the Table view is selected` was fixed by removing the extra margin:

**Before:**
![left-margin-before](https://user-images.githubusercontent.com/2946766/178568473-c8162631-4340-4858-8d93-3466461f97d0.png)

**After:**
![left-margin-after](https://user-images.githubusercontent.com/4459398/182253474-1684094f-4f8d-43fe-860b-addb599ed4f1.png)
andrew-goldstein added a commit that referenced this issue Aug 2, 2022
## [Security Solution] Alerts Treemap fixes

This PR contains fixes for the following issues:

- #136377 was fixed by using the `key_as_string` value in the search strategy response to format timestamps for the Treemap and Table views, per the screenshots below:

**Treemap with timestamp formatting:**

![treemap_with_timestamp_formatting](https://user-images.githubusercontent.com/4459398/182256532-7df8d39b-4e09-440f-aae7-c6e3720e0906.png)

**Table with timestamp formatting:**

![table_with_timestamp_formatting](https://user-images.githubusercontent.com/4459398/182256817-ee8605a7-d9d3-4127-8753-aaad65832308.png)

- #136387 was fixed by reducing the mininum font size to `4px` for group names, per [this example](https://elastic.github.io/elastic-charts/?path=/story/treemap--two-layers-stress-test&globals=theme:light;background:white) from Elastic charts. This change:
  - Reduces the chances of a label getting hidden when the browser is resized, but cannot eliminate it, because at a certain point, Elastic Charts must hide the label in order to (still) render the group
  - Will result in some groups having very small labels where the group previously would not have them, per the screenshot below:

![4px_labels](https://user-images.githubusercontent.com/4459398/182255505-bc77c184-df5a-4ff6-830a-c7c5b6127d25.png)

- #136372 was fixed by adding the word `Trend`, `Table`, and `Treemap` to the inspect modal for each of the respective chart types, per the screenshots below:

**Inspect Trend:**

![inspect_trend](https://user-images.githubusercontent.com/4459398/182257336-5d988e6a-a436-48b6-ad59-72b64e86d89b.png)

**Inspect Table:**
![inspect_table](https://user-images.githubusercontent.com/4459398/182257396-6e0d01e3-e493-4765-ac60-fee790a52633.png)

**Inspect Treemap:**
![inspect_treemap](https://user-images.githubusercontent.com/4459398/182257457-d3b58f18-e393-4307-b319-a2e1019b0329.png)

- [review feedback](#126896 (review)) where the `Tree map isn't displayed for certain group by top fields, like Ransomeware.version`, was fixed as suggested by displaying an empty state message with a reason why the treemap cannot be displayed:

**Before:**
![empty-state-before](https://user-images.githubusercontent.com/2946766/178567785-82d03c51-3e3c-49f6-ac86-d13ecaf3aef0.png)

**After:**
![empty-state-after](https://user-images.githubusercontent.com/4459398/182254376-437d3ce8-c4bd-4b43-a456-2a4460db4565.png)

- [review feedback](#126896 (review)) where there were `Extra left margins when the Table view is selected` was fixed by removing the extra margin:

**Before:**
![left-margin-before](https://user-images.githubusercontent.com/2946766/178568473-c8162631-4340-4858-8d93-3466461f97d0.png)

**After:**
![left-margin-after](https://user-images.githubusercontent.com/4459398/182253474-1684094f-4f8d-43fe-860b-addb599ed4f1.png)
kibanamachine pushed a commit that referenced this issue Aug 2, 2022
## [Security Solution] Alerts Treemap fixes

This PR contains fixes for the following issues:

- #136377 was fixed by using the `key_as_string` value in the search strategy response to format timestamps for the Treemap and Table views, per the screenshots below:

**Treemap with timestamp formatting:**

![treemap_with_timestamp_formatting](https://user-images.githubusercontent.com/4459398/182256532-7df8d39b-4e09-440f-aae7-c6e3720e0906.png)

**Table with timestamp formatting:**

![table_with_timestamp_formatting](https://user-images.githubusercontent.com/4459398/182256817-ee8605a7-d9d3-4127-8753-aaad65832308.png)

- #136387 was fixed by reducing the mininum font size to `4px` for group names, per [this example](https://elastic.github.io/elastic-charts/?path=/story/treemap--two-layers-stress-test&globals=theme:light;background:white) from Elastic charts. This change:
  - Reduces the chances of a label getting hidden when the browser is resized, but cannot eliminate it, because at a certain point, Elastic Charts must hide the label in order to (still) render the group
  - Will result in some groups having very small labels where the group previously would not have them, per the screenshot below:

![4px_labels](https://user-images.githubusercontent.com/4459398/182255505-bc77c184-df5a-4ff6-830a-c7c5b6127d25.png)

- #136372 was fixed by adding the word `Trend`, `Table`, and `Treemap` to the inspect modal for each of the respective chart types, per the screenshots below:

**Inspect Trend:**

![inspect_trend](https://user-images.githubusercontent.com/4459398/182257336-5d988e6a-a436-48b6-ad59-72b64e86d89b.png)

**Inspect Table:**
![inspect_table](https://user-images.githubusercontent.com/4459398/182257396-6e0d01e3-e493-4765-ac60-fee790a52633.png)

**Inspect Treemap:**
![inspect_treemap](https://user-images.githubusercontent.com/4459398/182257457-d3b58f18-e393-4307-b319-a2e1019b0329.png)

- [review feedback](#126896 (review)) where the `Tree map isn't displayed for certain group by top fields, like Ransomeware.version`, was fixed as suggested by displaying an empty state message with a reason why the treemap cannot be displayed:

**Before:**
![empty-state-before](https://user-images.githubusercontent.com/2946766/178567785-82d03c51-3e3c-49f6-ac86-d13ecaf3aef0.png)

**After:**
![empty-state-after](https://user-images.githubusercontent.com/4459398/182254376-437d3ce8-c4bd-4b43-a456-2a4460db4565.png)

- [review feedback](#126896 (review)) where there were `Extra left margins when the Table view is selected` was fixed by removing the extra margin:

**Before:**
![left-margin-before](https://user-images.githubusercontent.com/2946766/178568473-c8162631-4340-4858-8d93-3466461f97d0.png)

**After:**
![left-margin-after](https://user-images.githubusercontent.com/4459398/182253474-1684094f-4f8d-43fe-860b-addb599ed4f1.png)

(cherry picked from commit a5a7d74)
kibanamachine added a commit that referenced this issue Aug 2, 2022
## [Security Solution] Alerts Treemap fixes

This PR contains fixes for the following issues:

- #136377 was fixed by using the `key_as_string` value in the search strategy response to format timestamps for the Treemap and Table views, per the screenshots below:

**Treemap with timestamp formatting:**

![treemap_with_timestamp_formatting](https://user-images.githubusercontent.com/4459398/182256532-7df8d39b-4e09-440f-aae7-c6e3720e0906.png)

**Table with timestamp formatting:**

![table_with_timestamp_formatting](https://user-images.githubusercontent.com/4459398/182256817-ee8605a7-d9d3-4127-8753-aaad65832308.png)

- #136387 was fixed by reducing the mininum font size to `4px` for group names, per [this example](https://elastic.github.io/elastic-charts/?path=/story/treemap--two-layers-stress-test&globals=theme:light;background:white) from Elastic charts. This change:
  - Reduces the chances of a label getting hidden when the browser is resized, but cannot eliminate it, because at a certain point, Elastic Charts must hide the label in order to (still) render the group
  - Will result in some groups having very small labels where the group previously would not have them, per the screenshot below:

![4px_labels](https://user-images.githubusercontent.com/4459398/182255505-bc77c184-df5a-4ff6-830a-c7c5b6127d25.png)

- #136372 was fixed by adding the word `Trend`, `Table`, and `Treemap` to the inspect modal for each of the respective chart types, per the screenshots below:

**Inspect Trend:**

![inspect_trend](https://user-images.githubusercontent.com/4459398/182257336-5d988e6a-a436-48b6-ad59-72b64e86d89b.png)

**Inspect Table:**
![inspect_table](https://user-images.githubusercontent.com/4459398/182257396-6e0d01e3-e493-4765-ac60-fee790a52633.png)

**Inspect Treemap:**
![inspect_treemap](https://user-images.githubusercontent.com/4459398/182257457-d3b58f18-e393-4307-b319-a2e1019b0329.png)

- [review feedback](#126896 (review)) where the `Tree map isn't displayed for certain group by top fields, like Ransomeware.version`, was fixed as suggested by displaying an empty state message with a reason why the treemap cannot be displayed:

**Before:**
![empty-state-before](https://user-images.githubusercontent.com/2946766/178567785-82d03c51-3e3c-49f6-ac86-d13ecaf3aef0.png)

**After:**
![empty-state-after](https://user-images.githubusercontent.com/4459398/182254376-437d3ce8-c4bd-4b43-a456-2a4460db4565.png)

- [review feedback](#126896 (review)) where there were `Extra left margins when the Table view is selected` was fixed by removing the extra margin:

**Before:**
![left-margin-before](https://user-images.githubusercontent.com/2946766/178568473-c8162631-4340-4858-8d93-3466461f97d0.png)

**After:**
![left-margin-after](https://user-images.githubusercontent.com/4459398/182253474-1684094f-4f8d-43fe-860b-addb599ed4f1.png)

(cherry picked from commit a5a7d74)

Co-authored-by: Andrew Goldstein <[email protected]>
@andrew-goldstein
Copy link
Contributor

@andrew-goldstein
Copy link
Contributor

@karanbirsingh-qasource would you be willing to test the fix for this issue?

@ghost
Copy link
Author

ghost commented Aug 3, 2022

sure @andrew-goldstein

@ghost
Copy link
Author

ghost commented Aug 8, 2022

Hi @andrew-goldstein

we have validated this issue on 8.4.0 BC2 and found the issue to be fixed now ✔️ .

Build Details:

Version:8.4.0 BC2
Commit:9e9e0d6a685cbc2858a85a357f93dcb76259fdee
Build:55166

Screen-Shoot:

image

image

image

Hence we are closing this issue and adding "QA:Validated"

thanks !!

@ghost ghost closed this as completed Aug 8, 2022
@ghost ghost added the QA:Validated Issue has been validated by QA label Aug 8, 2022
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience fixed impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. QA:Validated Issue has been validated by QA Team: SecuritySolution Security Solutions Team working on SIEM, Endpoint, Timeline, Resolver, etc. Team:Threat Hunting:Investigations Security Solution Investigations Team Team:Threat Hunting Security Solution Threat Hunting Team v8.4.0
Projects
None yet
Development

No branches or pull requests

4 participants