fix(dashboard): fixup Automated Analysis Card #968

maxcao13 · 2023-04-19T08:59:31Z

Welcome to Cryostat! 👋

Before contributing, make sure you have:

Read the contributing guidelines
Linked a relevant issue which this PR resolves
Linked any other relevant issues, PR's, or documentation, if any
Resolved all conflicts, if any
Rebased your branch PR on top of the latest upstream main branch
Attached at least one of the following labels to the PR: [chore, ci, docs, feat, fix, test]
Signed the last commit: git commit --amend --signoff

Related: https://github.com/cryostatio/cryostat/pull/1457
Depends on: https://github.com/cryostatio/cryostat/pull/1457

Description of the change:

Fixes icons for AA results to fit the result (warning = warningTriangleIcon, error = exclamation point).

This PR also has a hack to separate new JMC IResult fields within description of Rule results.

Signed-off-by: Max Cao <macao@redhat.com>

github-actions · 2023-04-19T09:06:49Z

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-968-339e5fe37cf15248310584aea90af2329c5ee3d9 sh smoketest.sh

github-actions · 2023-04-19T17:39:52Z

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-968-339e5fe37cf15248310584aea90af2329c5ee3d9 sh smoketest.sh

tthvo · 2023-04-19T18:50:26Z

Seems like another flanky test with jest mocks being called more than expected :((

Signed-off-by: Max Cao <macao@redhat.com>

github-actions · 2023-04-19T19:41:57Z

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-968-9db5b8495eb3763d1622735c6106afd56ff911ba sh smoketest.sh

maxcao13 · 2023-04-19T19:56:19Z

I think I'm going to do a little more refactoring on the card. Some of the rules/results changes from updating to JMC 8.2 are making me consider some changes.

Signed-off-by: Max Cao <macao@redhat.com>

maxcao13 · 2023-04-20T01:04:39Z

Sort of a hack, but it's a little much to change the backend code to account for all of the JMC rule changes at this point in development but thoughts on these changes?

ClickableAALabel tooltip:

List view:

I found something interesting though, seems like some code somwhere is not picking up on this messageKey:

https://github.com/openjdk/jmc/blob/master/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/resources/org/openjdk/jmc/flightrecorder/rules/jdk/messages/internal/messages.properties#L632

Signed-off-by: Max Cao <macao@redhat.com>

maxcao13 · 2023-04-20T01:23:24Z

Not sure where these pom property ci issues are coming from...

github-actions · 2023-04-20T06:22:52Z

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-968-f32cbc411024a62bd16c0d89952ca29768737243 sh smoketest.sh

src/app/Dashboard/AutomatedAnalysis/ClickableAutomatedAnalysisLabel.tsx

andrewazores · 2023-04-20T08:56:58Z

I found something interesting though, seems like some code somwhere is not picking up on this messageKey:

That's definitely weird. I'm not sure what's causing that exactly, but it's either something in JMC itself or in how I wrote the part that populates those messages:

cryostatio/cryostat-core@2336321#diff-9f4e83d307a0c834eb25732bd9c56dc920c2a4869c5611143864ee0d85d384faR229

That populateMessage() call is what should be replacing these message keys as well as filling in variable interpolations.

Signed-off-by: Max Cao <macao@redhat.com>

github-actions · 2023-04-20T18:40:57Z

Test image available:

CRYOSTAT_IMAGE=ghcr.io/cryostatio/cryostat-web:pr-968-70de5d83e5871cad61d7a34ab878773dd706aeb4 sh smoketest.sh

maxcao13 · 2023-04-20T21:07:01Z

I found something interesting though, seems like some code somwhere is not picking up on this messageKey:

That's definitely weird. I'm not sure what's causing that exactly, but it's either something in JMC itself or in how I wrote the part that populates those messages:

cryostatio/cryostat-core@2336321#diff-9f4e83d307a0c834eb25732bd9c56dc920c2a4869c5611143864ee0d85d384faR229

That populateMessage() call is what should be replacing these message keys as well as filling in variable interpolations.

https://github.com/openjdk/jmc/blob/1fb69d47fa54bbf98e2463c0b07be2272b6f8a87/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/io/SocketReadRule.java#L135

I think it's a bug on JMC. They don't call getString so it returns the key instead.

Messages.SocketReadRuleFactory_TEXT_NO_EVENTS -> Messages.getString(Messages.SocketReadRuleFactory_TEXT_NO_EVENTS)

tthvo · 2023-04-21T04:04:44Z

Good find! Seems like it. Next few line shows the correct way.

https://github.com/openjdk/jmc/blob/1fb69d47fa54bbf98e2463c0b07be2272b6f8a87/core/org.openjdk.jmc.flightrecorder.rules.jdk/src/main/java/org/openjdk/jmc/flightrecorder/rules/jdk/io/SocketReadRule.java#L167

* fix aa result icons as well Signed-off-by: Max Cao <macao@redhat.com> * revert directoryName field Signed-off-by: Max Cao <macao@redhat.com> * summary and explanation, change description wrapping to breakWord Signed-off-by: Max Cao <macao@redhat.com> * remove summary on N/A scores Signed-off-by: Max Cao <macao@redhat.com> * update snapshot Signed-off-by: Max Cao <macao@redhat.com> * split on double newlines Signed-off-by: Max Cao <macao@redhat.com> * snapshots, eslint Signed-off-by: Max Cao <macao@redhat.com> --------- Signed-off-by: Max Cao <macao@redhat.com> (cherry picked from commit 8147b49)

tthvo · 2023-04-21T04:15:10Z

src/app/Dashboard/dashboard-utils.tsx

+
+export const transformAADescription = (description: string): JSX.Element => {
+  const splitDesc = description.split('\n\n');
+  const boldRegex = /^([^:]+:\s?)/; // match text up to and including the first colon


Lets try this:

^([^:]+):\s*(.*)

Tester: https://regex101.com/r/iP2RoX/1

This way, u can pull the matched group out easily.

// Must check matched first const [_, boldText, content] = matched;

also, we need to check the content (hide it maybe?) as it might be empty (not sure when but best to check).

Opps nvmind :)) I guess this could be a nice refactoring idea then :)

* fix aa result icons as well Signed-off-by: Max Cao <macao@redhat.com> * revert directoryName field Signed-off-by: Max Cao <macao@redhat.com> * summary and explanation, change description wrapping to breakWord Signed-off-by: Max Cao <macao@redhat.com> * remove summary on N/A scores Signed-off-by: Max Cao <macao@redhat.com> * update snapshot Signed-off-by: Max Cao <macao@redhat.com> * split on double newlines Signed-off-by: Max Cao <macao@redhat.com> * snapshots, eslint Signed-off-by: Max Cao <macao@redhat.com> --------- Signed-off-by: Max Cao <macao@redhat.com> (cherry picked from commit 8147b49) Co-authored-by: Max Cao <macao@redhat.com>

fix aa result icons as well

339e5fe

Signed-off-by: Max Cao <macao@redhat.com>

maxcao13 added the fix label Apr 19, 2023

maxcao13 requested review from andrewazores and tthvo April 19, 2023 08:59

mergify bot added the safe-to-test label Apr 19, 2023

andrewazores added the backport label Apr 19, 2023

revert directoryName field

9db5b84

Signed-off-by: Max Cao <macao@redhat.com>

maxcao13 changed the title ~~fix(archives): fix archived directory get response form + fix AA card result icons~~ fix(dashboard): fix AA card result icons Apr 19, 2023

summary and explanation, change description wrapping to breakWord

c3927a0

Signed-off-by: Max Cao <macao@redhat.com>

remove summary on N/A scores

ead8031

Signed-off-by: Max Cao <macao@redhat.com>

maxcao13 changed the title ~~fix(dashboard): fix AA card result icons~~ fix(dashboard): fixup Automated Analysis Card Apr 20, 2023

update snapshot

f32cbc4

Signed-off-by: Max Cao <macao@redhat.com>

tthvo reviewed Apr 20, 2023

View reviewed changes

src/app/Dashboard/AutomatedAnalysis/ClickableAutomatedAnalysisLabel.tsx Outdated Show resolved Hide resolved

maxcao13 added 2 commits April 20, 2023 14:31

split on double newlines

900a2f8

Signed-off-by: Max Cao <macao@redhat.com>

snapshots, eslint

70de5d8

Signed-off-by: Max Cao <macao@redhat.com>

This was referenced Apr 20, 2023

[Story] Localize and refactor Automated Analysis Rules throughout codebase cryostatio/cryostat-legacy#1464

Open

[Task] Support localization of Automated Analysis cryostatio/cryostat-core#210

Open

andrewazores approved these changes Apr 21, 2023

View reviewed changes

andrewazores merged commit 8147b49 into cryostatio:main Apr 21, 2023

mergify bot mentioned this pull request Apr 21, 2023

fix(dashboard): fixup Automated Analysis Card (backport #968) #974

Merged

tthvo reviewed Apr 21, 2023

View reviewed changes

maxcao13 deleted the archive-directory-front-end-handling branch April 21, 2023 21:46

maxcao13 mentioned this pull request Apr 25, 2023

fix(aa): change AutomatedAnalysisCard severity score from 50 to 25 #983

Merged

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(dashboard): fixup Automated Analysis Card #968

fix(dashboard): fixup Automated Analysis Card #968

maxcao13 commented Apr 19, 2023 •

edited

Loading

github-actions bot commented Apr 19, 2023

github-actions bot commented Apr 19, 2023

tthvo commented Apr 19, 2023

github-actions bot commented Apr 19, 2023

maxcao13 commented Apr 19, 2023

maxcao13 commented Apr 20, 2023

maxcao13 commented Apr 20, 2023

github-actions bot commented Apr 20, 2023

andrewazores commented Apr 20, 2023

github-actions bot commented Apr 20, 2023

maxcao13 commented Apr 20, 2023 •

edited

Loading

tthvo commented Apr 21, 2023

tthvo Apr 21, 2023

tthvo Apr 21, 2023

tthvo Apr 21, 2023

fix(dashboard): fixup Automated Analysis Card #968

fix(dashboard): fixup Automated Analysis Card #968

Conversation

maxcao13 commented Apr 19, 2023 • edited Loading

Welcome to Cryostat! 👋

Before contributing, make sure you have:

Description of the change:

github-actions bot commented Apr 19, 2023

github-actions bot commented Apr 19, 2023

tthvo commented Apr 19, 2023

github-actions bot commented Apr 19, 2023

maxcao13 commented Apr 19, 2023

maxcao13 commented Apr 20, 2023

ClickableAALabel tooltip:

List view:

maxcao13 commented Apr 20, 2023

github-actions bot commented Apr 20, 2023

andrewazores commented Apr 20, 2023

github-actions bot commented Apr 20, 2023

maxcao13 commented Apr 20, 2023 • edited Loading

tthvo commented Apr 21, 2023

tthvo Apr 21, 2023

Choose a reason for hiding this comment

tthvo Apr 21, 2023

Choose a reason for hiding this comment

tthvo Apr 21, 2023

Choose a reason for hiding this comment

maxcao13 commented Apr 19, 2023 •

edited

Loading

maxcao13 commented Apr 20, 2023 •

edited

Loading