fix(recordings): render labels in 'key=value' form (cryostatio#1313)

tthvo · Jul 30, 2024 · 508d03e · 508d03e
1 parent 9526cb5
commit 508d03e
Show file tree

Hide file tree

Showing 16 changed files with 86 additions and 61 deletions.
diff --git a/src/app/RecordingMetadata/ClickableLabel.tsx b/src/app/RecordingMetadata/ClickableLabel.tsx
@@ -14,7 +14,7 @@
  * limitations under the License.
  */
 
-import { KeyValue } from '@app/Shared/Services/api.types';
+import { KeyValue, keyValueToString } from '@app/Shared/Services/api.types';
 import { Label } from '@patternfly/react-core';
 import * as React from 'react';
 
@@ -47,16 +47,17 @@ export const ClickableLabel: React.FC<ClickableLabelCellProps> = ({ label, isSel
   return (
     <>
       <Label
-        aria-label={`${label.key}: ${label.value}`}
+        aria-label={keyValueToString(label)}
         style={style}
+        isTruncated
         onMouseEnter={handleHoveredOrFocused}
         onMouseLeave={handleNonHoveredOrFocused}
         onFocus={handleHoveredOrFocused}
         onClick={handleLabelClicked}
         key={label.key}
         color={labelColor}
       >
-        {`${label.key}: ${label.value}`}
+        {keyValueToString(label)}
       </Label>
     </>
   );

diff --git a/src/app/RecordingMetadata/LabelCell.tsx b/src/app/RecordingMetadata/LabelCell.tsx
@@ -15,11 +15,10 @@
  */
 
 import { UpdateFilterOptions } from '@app/Shared/Redux/Filters/Common';
-import { KeyValue } from '@app/Shared/Services/api.types';
+import { KeyValue, keyValueToString } from '@app/Shared/Services/api.types';
 import { Label, Text } from '@patternfly/react-core';
 import * as React from 'react';
 import { ClickableLabel } from './ClickableLabel';
-import { getLabelDisplay } from './utils';
 
 export interface LabelCellProps {
   target: string;
@@ -36,7 +35,7 @@ export const LabelCell: React.FC<LabelCellProps> = ({ target, labels, clickableO
     (label: KeyValue) => {
       if (clickableOptions) {
         const labelFilterSet = new Set(clickableOptions.labelFilters);
-        return labelFilterSet.has(getLabelDisplay(label));
+        return labelFilterSet.has(keyValueToString(label));
       }
       return false;
     },
@@ -52,7 +51,7 @@ export const LabelCell: React.FC<LabelCellProps> = ({ target, labels, clickableO
       if (clickableOptions) {
         clickableOptions.updateFilters(target, {
           filterKey: 'Label',
-          filterValue: getLabelDisplay(clickedLabel),
+          filterValue: keyValueToString(clickedLabel),
           deleted: isLabelSelected(clickedLabel),
         });
       }
@@ -72,8 +71,8 @@ export const LabelCell: React.FC<LabelCellProps> = ({ target, labels, clickableO
               onLabelClick={onLabelSelectToggle}
             />
           ) : (
-            <Label aria-label={`${label.key}: ${label.value}`} key={label.key} color={getLabelColor(label)}>
-              {`${label.key}: ${label.value}`}
+            <Label aria-label={keyValueToString(label)} key={label.key} color={getLabelColor(label)}>
+              {keyValueToString(label)}
             </Label>
           ),
         )

diff --git a/src/app/RecordingMetadata/utils.ts b/src/app/RecordingMetadata/utils.ts
@@ -48,8 +48,6 @@ export const parseLabelsFromFile = (file: File): Observable<KeyValue[]> => {
   );
 };
 
-export const getLabelDisplay = (label: KeyValue) => `${label.key}:${label.value}`;
-
 export const LabelPattern = /^\S+$/;
 
 export const getValidatedOption = (isValid: boolean) => {

diff --git a/src/app/Recordings/ActiveRecordingsTable.tsx b/src/app/Recordings/ActiveRecordingsTable.tsx
@@ -36,9 +36,11 @@ import {
   ActiveRecording,
   AnalysisResult,
   CategorizedRuleEvaluations,
+  KeyValue,
   NotificationCategory,
   RecordingState,
   Target,
+  keyValueToString,
 } from '@app/Shared/Services/api.types';
 import { ServiceContext } from '@app/Shared/Services/Services';
 import { useDayjs } from '@app/utils/hooks/useDayjs';
@@ -902,6 +904,18 @@ export const ActiveRecordingRow: React.FC<ActiveRecordingRowProps> = ({
     handleLoadAnalysis();
   }, [isExpanded, handleLoadAnalysis]);
 
+  const recordingOptions = (recording: ActiveRecording): KeyValue[] => {
+    const options: KeyValue[] = [];
+    options.push({ key: 'toDisk', value: String(recording.toDisk) });
+    if (recording.maxAge) {
+      options.push({ key: 'maxAge', value: `${recording.maxAge / 1000}s` });
+    }
+    if (recording.maxSize) {
+      options.push({ key: 'maxSize', value: formatBytes(recording.maxAge) });
+    }
+    return options;
+  };
+
   const parentRow = React.useMemo(() => {
     const RecordingDuration = (props: { duration: number }) => {
       const str = React.useMemo(
@@ -952,19 +966,11 @@ export const ActiveRecordingRow: React.FC<ActiveRecordingRowProps> = ({
         </Td>
         <Td key={`active-table-row-${index}_6`} dataLabel={tableColumns[4].title}>
           <LabelGroup isVertical style={{ padding: '0.2em' }}>
-            <Label color="blue" key="toDisk">
-              toDisk: {String(recording.toDisk)}
-            </Label>
-            {recording.maxAge ? (
-              <Label color="blue" key="maxAge">
-                maxAge: {recording.maxAge / 1000}s
+            {recordingOptions(recording).map((options) => (
+              <Label color="blue" key={options.key} isTruncated>
+                {keyValueToString(options)}
               </Label>
-            ) : undefined}
-            {recording.maxSize ? (
-              <Label color="blue" key="maxSize">
-                maxSize: {formatBytes(recording.maxSize)}
-              </Label>
-            ) : undefined}
+            ))}
           </LabelGroup>
         </Td>
         <Td key={`active-table-row-${index}_7`} dataLabel={tableColumns[5].title}>

diff --git a/src/app/Recordings/Filters/LabelFilter.tsx b/src/app/Recordings/Filters/LabelFilter.tsx
@@ -14,8 +14,7 @@
  * limitations under the License.
  */
 
-import { getLabelDisplay } from '@app/RecordingMetadata/utils';
-import { Recording } from '@app/Shared/Services/api.types';
+import { Recording, keyValueToString } from '@app/Shared/Services/api.types';
 import { Label, Select, SelectOption, SelectVariant } from '@patternfly/react-core';
 import * as React from 'react';
 
@@ -42,7 +41,7 @@ export const LabelFilter: React.FC<LabelFilterProps> = ({ recordings, filteredLa
     const labels = new Set<string>();
     recordings.forEach((r) => {
       if (!r || !r.metadata || !r.metadata.labels) return;
-      r.metadata.labels.map((label) => labels.add(getLabelDisplay(label)));
+      r.metadata.labels.map((label) => labels.add(keyValueToString(label)));
     });
     return Array.from(labels)
       .filter((l) => !filteredLabels.includes(l))

diff --git a/src/app/Recordings/RecordingFilters.tsx b/src/app/Recordings/RecordingFilters.tsx
@@ -20,7 +20,7 @@ import {
   allowedArchivedRecordingFilters,
 } from '@app/Shared/Redux/Filters/RecordingFilterSlice';
 import { recordingUpdateCategoryIntent, RootState, StateDispatch } from '@app/Shared/Redux/ReduxStore';
-import { Recording, RecordingState } from '@app/Shared/Services/api.types';
+import { KeyValue, Recording, RecordingState, keyValueToString } from '@app/Shared/Services/api.types';
 import { useDayjs } from '@app/utils/hooks/useDayjs';
 import dayjs from '@i18n/datetime';
 import {
@@ -294,7 +294,7 @@ export const filterRecordings = (recordings: any[], filters: RecordingFiltersCat
   }
   if (filters.Label.length) {
     filtered = filtered.filter((recording) => {
-      const recordingLabels = recording.metadata.labels.map((label) => `${label.key}:${label.value}`);
+      const recordingLabels = recording.metadata.labels.map((label: KeyValue) => keyValueToString(label));
       return filters.Label.some((filterLabel) => recordingLabels.includes(filterLabel));
     });
   }

diff --git a/src/app/Topology/Toolbar/TopologyFilters.tsx b/src/app/Topology/Toolbar/TopologyFilters.tsx
@@ -212,7 +212,13 @@ export const TopologyFilter: React.FC<{ isDisabled?: boolean }> = ({ isDisabled,
                   },
                 }}
               >
-                {isLabelOrAnnotation(cat) ? <Label color="grey">{opt}</Label> : opt}
+                {isLabelOrAnnotation(cat) ? (
+                  <Label color="grey" isTruncated>
+                    {opt}
+                  </Label>
+                ) : (
+                  opt
+                )}
               </SelectOption>
             ))}
           </SelectGroup>
@@ -285,7 +291,13 @@ export const TopologyFilter: React.FC<{ isDisabled?: boolean }> = ({ isDisabled,
               },
             }}
           >
-            {isLabelOrAnnotation(cat) ? <Label color="grey">{opt}</Label> : opt}
+            {isLabelOrAnnotation(cat) ? (
+              <Label color="grey" isTruncated>
+                {opt}
+              </Label>
+            ) : (
+              opt
+            )}
           </SelectOption>
         );
       });

diff --git a/src/test/RecordingMetadata/BulkEditLabels.test.tsx b/src/test/RecordingMetadata/BulkEditLabels.test.tsx
@@ -173,7 +173,7 @@ describe('<BulkEditLabels />', () => {
       },
     });
 
-    const label = screen.getByLabelText('someLabel: someValue');
+    const label = screen.getByLabelText('someLabel=someValue');
     expect(label).toBeInTheDocument();
     expect(label).toBeVisible();
     expect(label.onclick).toBeNull();
@@ -262,11 +262,11 @@ describe('<BulkEditLabels />', () => {
       },
     });
 
-    const newLabel = screen.getByLabelText('someNewLabel: someNewValue');
+    const newLabel = screen.getByLabelText('someNewLabel=someNewValue');
     expect(newLabel).toBeInTheDocument();
     expect(newLabel).toBeVisible();
 
-    const oldLabel = screen.getByLabelText('someLabel: someValue');
+    const oldLabel = screen.getByLabelText('someLabel=someValue');
     expect(oldLabel).toBeInTheDocument();
     expect(oldLabel).toBeVisible();
   });
@@ -283,11 +283,11 @@ describe('<BulkEditLabels />', () => {
       },
     });
 
-    const newLabel = screen.getByLabelText('someNewLabel: someNewValue');
+    const newLabel = screen.getByLabelText('someNewLabel=someNewValue');
     expect(newLabel).toBeInTheDocument();
     expect(newLabel).toBeVisible();
 
-    const oldLabel = screen.getByLabelText('someLabel: someValue');
+    const oldLabel = screen.getByLabelText('someLabel=someValue');
     expect(oldLabel).toBeInTheDocument();
     expect(oldLabel).toBeVisible();
   });

diff --git a/src/test/RecordingMetadata/ClickableLabel.test.tsx b/src/test/RecordingMetadata/ClickableLabel.test.tsx
@@ -23,7 +23,7 @@ const mockLabel = {
   key: 'someLabel',
   value: 'someValue',
 } as KeyValue;
-const mockLabelAsString = 'someLabel: someValue';
+const mockLabelAsString = 'someLabel=someValue';
 
 const onLabelClick = jest.fn((_label: KeyValue) => {
   /**Do nothing. Used for checking renders */

diff --git a/src/test/RecordingMetadata/LabelCell.test.tsx b/src/test/RecordingMetadata/LabelCell.test.tsx
@@ -37,12 +37,10 @@ const mockAnotherLabel = { key: 'anotherLabel', value: 'anotherValue' } as KeyVa
 const mockLabelList = [mockLabel, mockAnotherLabel];
 
 // For display
-const mockLabelStringDisplayList = [
-  `${mockLabel.key}: ${mockLabel.value}`,
-  `${mockAnotherLabel.key}: ${mockAnotherLabel.value}`,
+const mockLabelStringList = [
+  `${mockLabel.key}=${mockLabel.value}`,
+  `${mockAnotherLabel.key}=${mockAnotherLabel.value}`,
 ];
-// For filters and labeling elements
-const mockLabelStringList = mockLabelStringDisplayList.map((s: string) => s.replace(' ', ''));
 
 describe('<LabelCell />', () => {
   let onUpdateLabels: (target: string, updateFilterOptions: UpdateFilterOptions) => void;
@@ -84,7 +82,7 @@ describe('<LabelCell />', () => {
       },
     });
 
-    for (const labelAsString of mockLabelStringDisplayList) {
+    for (const labelAsString of mockLabelStringList) {
       const displayedLabel = screen.getByLabelText(labelAsString);
 
       expect(displayedLabel).toBeInTheDocument();
@@ -117,7 +115,7 @@ describe('<LabelCell />', () => {
 
     let count = 0;
     let index = 0;
-    for (const labelAsString of mockLabelStringDisplayList) {
+    for (const labelAsString of mockLabelStringList) {
       const displayedLabel = screen.getByLabelText(labelAsString);
 
       expect(displayedLabel).toBeInTheDocument();

diff --git a/src/test/RecordingMetadata/__snapshots__/BulkEditLabels.test.tsx.snap b/src/test/RecordingMetadata/__snapshots__/BulkEditLabels.test.tsx.snap
@@ -32,13 +32,13 @@ exports[`<BulkEditLabels /> renders correctly 1`] = `
     className="pf-l-stack__item"
   >
     <span
-      aria-label="someLabel: someValue"
+      aria-label="someLabel=someValue"
       className="pf-c-label"
     >
       <span
         className="pf-c-label__content"
       >
-        someLabel: someValue
+        someLabel=someValue
       </span>
     </span>
   </div>

diff --git a/src/test/RecordingMetadata/__snapshots__/ClickableLabel.test.tsx.snap b/src/test/RecordingMetadata/__snapshots__/ClickableLabel.test.tsx.snap
@@ -2,7 +2,7 @@
 
 exports[`<ClickableLabel /> renders correctly 1`] = `
 <span
-  aria-label="someLabel: someValue"
+  aria-label="someLabel=someValue"
   className="pf-c-label pf-m-blue"
   onClick={[Function]}
   onFocus={[Function]}
@@ -13,7 +13,11 @@ exports[`<ClickableLabel /> renders correctly 1`] = `
   <span
     className="pf-c-label__content"
   >
-    someLabel: someValue
+    <span
+      className="pf-c-label__text"
+    >
+      someLabel=someValue
+    </span>
   </span>
 </span>
 `;
diff --git a/src/test/RecordingMetadata/__snapshots__/LabelCell.test.tsx.snap b/src/test/RecordingMetadata/__snapshots__/LabelCell.test.tsx.snap
@@ -3,7 +3,7 @@
 exports[`<LabelCell /> renders correctly 1`] = `
 Array [
   <span
-    aria-label="someLabel: someValue"
+    aria-label="someLabel=someValue"
     className="pf-c-label"
     onClick={[Function]}
     onFocus={[Function]}
@@ -14,11 +14,15 @@ Array [
     <span
       className="pf-c-label__content"
     >
-      someLabel: someValue
+      <span
+        className="pf-c-label__text"
+      >
+        someLabel=someValue
+      </span>
     </span>
   </span>,
   <span
-    aria-label="anotherLabel: anotherValue"
+    aria-label="anotherLabel=anotherValue"
     className="pf-c-label"
     onClick={[Function]}
     onFocus={[Function]}
@@ -29,7 +33,11 @@ Array [
     <span
       className="pf-c-label__content"
     >
-      anotherLabel: anotherValue
+      <span
+        className="pf-c-label__text"
+      >
+        anotherLabel=anotherValue
+      </span>
     </span>
   </span>,
 ]

diff --git a/src/test/Recordings/ActiveRecordingsTable.test.tsx b/src/test/Recordings/ActiveRecordingsTable.test.tsx
@@ -245,7 +245,7 @@ describe('<ActiveRecordingsTable />', () => {
     expect(state).toBeVisible();
 
     mockRecordingLabels.forEach((entry) => {
-      const label = screen.getByText(`${entry.key}: ${entry.value}`);
+      const label = screen.getByText(`${entry.key}=${entry.value}`);
       expect(label).toBeInTheDocument();
       expect(label).toBeVisible();
     });
@@ -285,8 +285,8 @@ describe('<ActiveRecordingsTable />', () => {
       preloadedState: preloadedState,
     });
 
-    expect(screen.getByText('someLabel: someUpdatedValue')).toBeInTheDocument();
-    expect(screen.queryByText('someLabel: someValue')).not.toBeInTheDocument();
+    expect(screen.getByText('someLabel=someUpdatedValue')).toBeInTheDocument();
+    expect(screen.queryByText('someLabel=someValue')).not.toBeInTheDocument();
   });
 
   it('stops a Recording after receiving a notification', async () => {

diff --git a/src/test/Recordings/ArchivedRecordingsTable.test.tsx b/src/test/Recordings/ArchivedRecordingsTable.test.tsx
@@ -291,7 +291,7 @@ describe('<ArchivedRecordingsTable />', () => {
     }); */
 
     for (const entry of mockRecordingLabels) {
-      const label = await screen.findByText(`${entry.key}: ${entry.value}`);
+      const label = await screen.findByText(`${entry.key}=${entry.value}`);
       expect(label).toBeInTheDocument();
       expect(label).toBeVisible();
     }
@@ -333,8 +333,8 @@ describe('<ArchivedRecordingsTable />', () => {
       },
       preloadedState: preloadedState,
     });
-    expect(screen.getByText('someLabel: someUpdatedValue')).toBeInTheDocument();
-    expect(screen.queryByText('someLabel: someValue')).not.toBeInTheDocument();
+    expect(screen.getByText('someLabel=someUpdatedValue')).toBeInTheDocument();
+    expect(screen.queryByText('someLabel=someValue')).not.toBeInTheDocument();
   });
 
   it('removes a Recording after receiving a notification', async () => {