✨ OSIDB-3434: Reusable CvssCalculator for affect fields #443

C-Valen · 2024-09-26T14:10:19Z

OSIDB-3434: Reusable CvssCalculator for affect fields

Checklist:

Commits consolidated
Changelog updated
Test cases added/updated
Jira ticket updated

Summary:

Refactors CvssCalculator component to make it reusable in different layouts and uses it on the CVSS fields of the affect table.

Changes:

Fix minor wrong types issue
Refactors CvssCalculator component
Creates a OverlayedCvssCalculator component
Applies new component to the affect section
Minor affect section adjustments to support the calculator
Update test cases and snapshots

Demo:

OSIM-CvssCalculator-OnAffects-Demo.mp4

*UPDATE:
Refactor made to reduce the width of the CVSS columns in the affect table. In order to achieve that, now it is only displayed the CVSS Score by default, and the vector in a tooltip when hovering the cell. In edit mode the CVSS Vector input has been moved into the calculator's popup div, that way the column can be kept in reduced size while supporting all the functionalities of the CVSS Calculator.

*UPDATEv2:
It has been added a padding bottom on the flaw affects section to fix some UI bugs caused on the overplayed calculator, cause part of it was outside of the section.

Normal CVSS Score display:

CVSS Vector tooltip:

Edit mode with calculator closed:

Edit mode with calculator opened:

Closes OSIDB-3434
Closes OSIDB-3511

C-Valen · 2024-11-06T09:58:28Z

Update:

Commits squashed for reduced noise and better organization
Changelog corrected

MrMarble · 2024-11-06T12:07:41Z

src/components/FlawAffects/FlawAffectsTable.vue

@@ -27,6 +27,7 @@ const emit = defineEmits<{
  'affect:revert': [value: ZodAffectType];
  'affect:toggle-selection': [value: ZodAffectType];
  'affect:track': [value: ZodAffectType];
+  'affect:updateCvss': [affect: ZodAffectType, newVector: string, newScore: null | number, cvssScoreIndex: number];


Since we have this event declared multiple times as is passed to multiple children, what do you guys think about exporting the type? That way is only declared once and can be used for the handler as well?
@superbuggy

As discussed on meeting this will be handled in an independent task/PR (OSIDB-3668)

MrMarble · 2024-11-06T12:22:50Z

src/composables/useFlawAffectsModel.ts

+        affect.cvss_scores[cvssScoreIndex].score = newScore;
+      } else if (newVector !== '') {
+        affect.cvss_scores.push({
+          issuer: 'RH',


Suggested change

issuer: 'RH',

issuer: IssuerEnum.Rh,

MrMarble · 2024-11-06T12:23:00Z

src/composables/useFlawAffectsModel.ts

+      } else if (newVector !== '') {
+        affect.cvss_scores.push({
+          issuer: 'RH',
+          cvss_version: 'V3',


Suggested change

cvss_version: 'V3',

cvss_version: CVSS_V3,

MrMarble · 2024-11-06T12:24:00Z

src/composables/useFlawAffectsModel.ts

+    const cvssId = affect.cvss_scores[cvssScoreIndex]?.uuid;
+    if (newVector === '' && cvssScoreIndex !== -1 && affect.uuid && cvssId) {
+      affect.cvss_scores[cvssScoreIndex].vector = '';
+      affect.cvss_scores[cvssScoreIndex].score = null;
+      affectCvssToDelete.value[affect.uuid] = cvssId;
+    } else {
+      if (affect.uuid && affectCvssToDelete.value[affect.uuid]) {
+        delete affectCvssToDelete.value[affect.uuid];
+      }
+      if (cvssScoreIndex !== -1) {
+        affect.cvss_scores[cvssScoreIndex].vector = newVector;
+        affect.cvss_scores[cvssScoreIndex].score = newScore;
+      } else if (newVector !== '') {
+        affect.cvss_scores.push({
+          issuer: 'RH',
+          cvss_version: 'V3',
+          comment: '',
+          score: newScore,
+          vector: newVector,
+          embargoed: affect.embargoed,
+          alerts: [],
+        });
+      }
+    }


I find this function a bit hard to follow, could you change it a bit? Maybe use an early return instead of the else and maybe add a short comment on what each branch of the conditionals are intended to do

MrMarble · 2024-11-06T12:39:08Z

src/components/FlawAffects/FlawAffectsTableRow.vue

-  },
-});
+function affectCvss(affect: ZodAffectType) {
+  return affect.cvss_scores.find(({ cvss_version, issuer }) => issuer === 'RH' && cvss_version === 'V3');


Suggested change

return affect.cvss_scores.find(({ cvss_version, issuer }) => issuer === 'RH' && cvss_version === 'V3');

return affect.cvss_scores.find(({ cvss_version, issuer }) => issuer === IssuerEnum.Rh && cvss_version === CSSV_V3);

MrMarble · 2024-11-06T13:26:18Z

src/components/CvssCalculator/CvssCalculatorOverlayed.vue

Why do we need a new component only to show it on a overlay? Can't just mount the same CvssCalculator on a div and move the div to the correct position?

The only change I see is that we are triggering an event on change, we could just trigger that event on a parent with a watch and use the same CvssCalculator

MrMarble · 2024-11-06T13:27:29Z

src/components/FlawAffects/FlawAffectsTableRow.vue

-  },
-});
+function affectCvss(affect: ZodAffectType) {
+  return affect.cvss_scores.find(({ cvss_version, issuer }) => issuer === 'RH' && cvss_version === 'V3');


Suggested change

return affect.cvss_scores.find(({ cvss_version, issuer }) => issuer === 'RH' && cvss_version === 'V3');

return affect.cvss_scores.find(({ cvss_version, issuer }) => issuer === IssuerEnum.Rh && cvss_version === CVSS_V3);

MrMarble · 2024-11-11T08:20:57Z

src/components/CvssSection.vue

@@ -23,7 +23,9 @@ const props = defineProps<{
 const showAllCvss = ref(false);

 const otherCvss = computed(() => props.allCvss.filter(cvssItem =>
-  (!(cvssItem.cvss_version === 'V3' && (cvssItem.issuer === IssuerEnum.Rh || cvssItem.issuer === IssuerEnum.Nist)))));
+  (
+    !(cvssItem.cvss_version === 'CVSS_V3' && (cvssItem.issuer === IssuerEnum.Rh || cvssItem.issuer === IssuerEnum.Nist))


Suggested change

!(cvssItem.cvss_version === 'CVSS_V3' && (cvssItem.issuer === IssuerEnum.Rh || cvssItem.issuer === IssuerEnum.Nist))

!(cvssItem.cvss_version === CVSS_V3 && (cvssItem.issuer === IssuerEnum.Rh || cvssItem.issuer === IssuerEnum.Nist))

MrMarble · 2024-11-11T08:21:18Z

src/components/FlawAffects/FlawAffectsTableRow.vue

-  },
-});
+function affectCvss(affect: ZodAffectType) {
+  return affect.cvss_scores.find(({ cvss_version, issuer }) => issuer === IssuerEnum.Rh && cvss_version === 'CVSS_V3');


Suggested change

return affect.cvss_scores.find(({ cvss_version, issuer }) => issuer === IssuerEnum.Rh && cvss_version === 'CVSS_V3');

return affect.cvss_scores.find(({ cvss_version, issuer }) => issuer === IssuerEnum.Rh && cvss_version === CVSS_V3);

MrMarble · 2024-11-11T08:21:54Z

src/composables/useFlawAffectsModel.ts

+    } else if (newVector !== '') {
+      affect.cvss_scores.push({
+        issuer: IssuerEnum.Rh,
+        cvss_version: 'CVSS_V3',


Suggested change

cvss_version: 'CVSS_V3',

cvss_version: CVSS_V3,

🔥 Remove unused class 🐛 Fix wrong element attrs

MrMarble

LGTM

superbuggy · 2024-11-12T15:05:52Z

src/composables/useCvssCalculator.ts

+    'background-color': hslForBackground,
+  };
+};
+


Is this just used in the buttons?

No, also in the vector

superbuggy · 2024-11-12T15:06:36Z

src/components/CvssCalculator/CvssFactorBtns.vue

@@ -0,0 +1,170 @@
+<script setup lang="ts">


Could we change the name to CvssFactorButtons.vue?

superbuggy · 2024-11-12T15:08:55Z

src/components/CvssCalculator/CvssCalculatorOverlayed.vue

+
+function reset() {
+  cvssScore.value = null;
+  emit('updateAffectCvss', '', null);


Should this be emit('affect:cvss:update', '', null)?

As commented off thread, it needs to be discussed what convention we get into and apply it to all emits

✨ Use new CvssCalculator components

✅ Update snapshots ✅ Add overlayed calculator test suite

C-Valen added the enhancement New feature or request label Sep 26, 2024

C-Valen self-assigned this Sep 26, 2024

C-Valen marked this pull request as draft September 26, 2024 14:10

C-Valen force-pushed the feature/OSIDB-3434-cvss-calculator-on-affects branch from 95143c5 to 46e47e4 Compare October 1, 2024 11:26

C-Valen marked this pull request as ready for review October 1, 2024 11:33

C-Valen requested review from superbuggy and MrMarble October 1, 2024 11:33

C-Valen force-pushed the feature/OSIDB-3434-cvss-calculator-on-affects branch from 19fa6d0 to af0ae1d Compare October 1, 2024 11:38

C-Valen marked this pull request as draft October 7, 2024 11:33

C-Valen force-pushed the feature/OSIDB-3434-cvss-calculator-on-affects branch from d03e8e3 to 80ff4b2 Compare October 10, 2024 11:29

C-Valen force-pushed the feature/OSIDB-3434-cvss-calculator-on-affects branch 2 times, most recently from 2a39697 to 2e39eb9 Compare November 4, 2024 15:34

C-Valen marked this pull request as ready for review November 4, 2024 15:37

C-Valen force-pushed the feature/OSIDB-3434-cvss-calculator-on-affects branch 3 times, most recently from 0f1e0ab to 17ee7bd Compare November 6, 2024 09:55

MrMarble reviewed Nov 6, 2024

View reviewed changes

C-Valen force-pushed the feature/OSIDB-3434-cvss-calculator-on-affects branch 3 times, most recently from c935959 to 8170bda Compare November 7, 2024 11:58

C-Valen requested a review from MrMarble November 7, 2024 12:03

C-Valen force-pushed the feature/OSIDB-3434-cvss-calculator-on-affects branch from 8170bda to af41930 Compare November 7, 2024 12:07

MrMarble reviewed Nov 11, 2024

View reviewed changes

C-Valen added 2 commits November 11, 2024 10:10

🏷️ Add missing types

9a6d2f9

♻️ Refactor Cvss Calculator for reusability

e322fef

🔥 Remove unused class 🐛 Fix wrong element attrs

C-Valen force-pushed the feature/OSIDB-3434-cvss-calculator-on-affects branch from af41930 to 0b0d9ae Compare November 11, 2024 09:11

C-Valen requested a review from MrMarble November 11, 2024 09:12

MrMarble previously approved these changes Nov 11, 2024

View reviewed changes

superbuggy reviewed Nov 12, 2024

View reviewed changes

C-Valen added 3 commits November 12, 2024 16:27

✨ Provide new overlayed CvssCalculator component

8466476

✨ Use new CvssCalculator components

✅ Update test cases import and snapshots

ee7cc77

✅ Update snapshots ✅ Add overlayed calculator test suite

📝 OSIDB-3434 update

f257219

C-Valen dismissed MrMarble’s stale review via f257219 November 12, 2024 15:35

C-Valen force-pushed the feature/OSIDB-3434-cvss-calculator-on-affects branch from 0b0d9ae to f257219 Compare November 12, 2024 15:35

superbuggy approved these changes Nov 12, 2024

View reviewed changes

MrMarble approved these changes Nov 12, 2024

View reviewed changes

C-Valen merged commit c4593e3 into main Nov 12, 2024
5 checks passed

C-Valen deleted the feature/OSIDB-3434-cvss-calculator-on-affects branch November 12, 2024 15:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ OSIDB-3434: Reusable CvssCalculator for affect fields #443

✨ OSIDB-3434: Reusable CvssCalculator for affect fields #443

C-Valen commented Sep 26, 2024 •

edited

Loading

C-Valen commented Nov 6, 2024

MrMarble Nov 6, 2024

superbuggy Nov 6, 2024

C-Valen Nov 7, 2024

MrMarble Nov 6, 2024

MrMarble Nov 6, 2024

MrMarble Nov 6, 2024

MrMarble Nov 6, 2024

MrMarble Nov 6, 2024

MrMarble Nov 6, 2024

MrMarble Nov 11, 2024

MrMarble Nov 11, 2024

MrMarble Nov 11, 2024

MrMarble left a comment

superbuggy Nov 12, 2024

C-Valen Nov 12, 2024

superbuggy Nov 12, 2024

C-Valen Nov 12, 2024

superbuggy Nov 12, 2024

C-Valen Nov 12, 2024

	return affect.cvss_scores.find(({ cvss_version, issuer }) => issuer === 'RH' && cvss_version === 'V3');
	return affect.cvss_scores.find(({ cvss_version, issuer }) => issuer === IssuerEnum.Rh && cvss_version === CSSV_V3);

	!(cvssItem.cvss_version === 'CVSS_V3' && (cvssItem.issuer === IssuerEnum.Rh \|\| cvssItem.issuer === IssuerEnum.Nist))
	!(cvssItem.cvss_version === CVSS_V3 && (cvssItem.issuer === IssuerEnum.Rh \|\| cvssItem.issuer === IssuerEnum.Nist))

✨ OSIDB-3434: Reusable CvssCalculator for affect fields #443

✨ OSIDB-3434: Reusable CvssCalculator for affect fields #443

Conversation

C-Valen commented Sep 26, 2024 • edited Loading

OSIDB-3434: Reusable CvssCalculator for affect fields

Checklist:

Summary:

Changes:

Demo:

C-Valen commented Nov 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MrMarble left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

C-Valen commented Sep 26, 2024 •

edited

Loading