Просмотр исходного кода

fix(mobile-exp): Use unique value for key in tag breakdown (#41223)

There are cases where the name is the same for multiple values, the data
should use the payload value as the key to prevent items duplicating 
when changing tabs
Nar Saynorath 2 лет назад
Родитель
Сommit
6a29a789cf

+ 73 - 0
static/app/components/group/tagFacets/index.spec.tsx

@@ -21,6 +21,7 @@ describe('Tag Facets', function () {
           topValues: [
           topValues: [
             {
             {
               name: 'org.mozilla.ios.Fennec@106.0',
               name: 'org.mozilla.ios.Fennec@106.0',
+              value: 'org.mozilla.ios.Fennec@106.0',
               count: 30,
               count: 30,
             },
             },
           ],
           ],
@@ -35,6 +36,7 @@ describe('Tag Facets', function () {
             },
             },
             {
             {
               name: 'iOS 16.0',
               name: 'iOS 16.0',
+              value: 'iOS 16.0',
               count: 10,
               count: 10,
             },
             },
           ],
           ],
@@ -44,26 +46,32 @@ describe('Tag Facets', function () {
           topValues: [
           topValues: [
             {
             {
               name: 'iPhone15',
               name: 'iPhone15',
+              value: 'iPhone15',
               count: 7,
               count: 7,
             },
             },
             {
             {
               name: 'Android Phone',
               name: 'Android Phone',
+              value: 'Android Phone',
               count: 10,
               count: 10,
             },
             },
             {
             {
               name: 'iPhone12',
               name: 'iPhone12',
+              value: 'iPhone12',
               count: 13,
               count: 13,
             },
             },
             {
             {
               name: 'iPhone11',
               name: 'iPhone11',
+              value: 'iPhone11',
               count: 15,
               count: 15,
             },
             },
             {
             {
               name: 'iPhone10',
               name: 'iPhone10',
+              value: 'iPhone10',
               count: 18,
               count: 18,
             },
             },
             {
             {
               name: 'Other device',
               name: 'Other device',
+              value: 'Other device',
               count: 2,
               count: 2,
             },
             },
           ],
           ],
@@ -72,6 +80,10 @@ describe('Tag Facets', function () {
     });
     });
   });
   });
 
 
+  afterEach(function () {
+    MockApiClient.clearMockResponses();
+  });
+
   it('does not display anything if no tag values recieved', async function () {
   it('does not display anything if no tag values recieved', async function () {
     tagsMock = MockApiClient.addMockResponse({
     tagsMock = MockApiClient.addMockResponse({
       url: '/issues/1/tags/',
       url: '/issues/1/tags/',
@@ -233,6 +245,7 @@ describe('Tag Facets', function () {
           topValues: [
           topValues: [
             {
             {
               name: 'abcdef123456',
               name: 'abcdef123456',
+              value: 'abcdef123456',
               readable: 'Galaxy S22',
               readable: 'Galaxy S22',
               count: 2,
               count: 2,
             },
             },
@@ -259,4 +272,64 @@ describe('Tag Facets', function () {
     expect(screen.getByText('Galaxy S22')).toBeInTheDocument();
     expect(screen.getByText('Galaxy S22')).toBeInTheDocument();
     expect(screen.getByText('100%')).toBeInTheDocument();
     expect(screen.getByText('100%')).toBeInTheDocument();
   });
   });
+
+  it('does not duplicate release values with same name', async function () {
+    tagsMock = MockApiClient.addMockResponse({
+      url: '/issues/1/tags/',
+      body: {
+        release: {
+          key: 'release',
+          topValues: [
+            {
+              name: '1.0',
+              value: 'something@1.0',
+              count: 30,
+            },
+            {
+              name: '1.0',
+              value: '1.0',
+              count: 10,
+            },
+          ],
+        },
+        os: {
+          key: 'os',
+          topValues: [
+            {
+              name: 'Android 12',
+              value: 'Android 12',
+              count: 20,
+            },
+          ],
+        },
+      },
+    });
+
+    render(
+      <TagFacets
+        environments={[]}
+        groupId="1"
+        tagKeys={MOBILE_TAGS}
+        tagFormatter={MOBILE_TAGS_FORMATTER}
+      />,
+      {
+        organization,
+      }
+    );
+    await waitFor(() => {
+      expect(tagsMock).toHaveBeenCalled();
+    });
+
+    expect(screen.getByText('Android 12')).toBeInTheDocument();
+    userEvent.click(screen.getByText('release'));
+    expect(screen.getAllByText('1.0')).toHaveLength(2);
+
+    // Test that the tag isn't being duplicated to the os tab
+    userEvent.click(screen.getByText('os'));
+    expect(screen.queryByText('1.0')).not.toBeInTheDocument();
+
+    // Test that the tag hasn't been duplicated in the release tab
+    userEvent.click(screen.getByText('release'));
+    expect(screen.getAllByText('1.0')).toHaveLength(2);
+  });
 });
 });

+ 1 - 1
static/app/components/group/tagFacets/index.tsx

@@ -138,7 +138,7 @@ export function TagFacets({
         event?.tags.find(({key}) => key === state.selectedTag)?.value === value;
         event?.tags.find(({key}) => key === state.selectedTag)?.value === value;
       return {
       return {
         name,
         name,
-        value: name,
+        value,
         count,
         count,
         url,
         url,
         active: isTagValueOfCurrentEvent,
         active: isTagValueOfCurrentEvent,

+ 1 - 1
static/app/components/group/tagFacets/tagBreakdown.tsx

@@ -35,7 +35,7 @@ function TagBreakdown({segments, maxItems, selectedTag, colors}: TagBreakdownPro
       />
       />
       {visibleSegments.map((segment, index) => {
       {visibleSegments.map((segment, index) => {
         return (
         return (
-          <BreakdownRow key={segment.name}>
+          <BreakdownRow key={segment.value}>
             <LegendIcon color={colors[index]} />
             <LegendIcon color={colors[index]} />
             <Tooltip title={segment.tooltip}>
             <Tooltip title={segment.tooltip}>
               <TagLabel active={segment.active}>{segment.name}</TagLabel>
               <TagLabel active={segment.active}>{segment.name}</TagLabel>