fix(issueDetailsPage/contextSummary): Prefer client_os over os for eventContextSummary (#14515)

* fix(issueDetailsPage/contextSummary): Prefer client_os over os for eventContextSummary

* feat: Add annotation where OS is from

* ref: Apply suggestions from jauer

* Revert "feat: Add annotation where OS is from"

This reverts commit ca993ac.

Author: untitaker

src/sentry/static/sentry/app/components/events/contextSummary.jsx

@@ -234,12 +234,12 @@ export class GpuSummary extends React.Component {
 const MIN_CONTEXTS = 3;
 const MAX_CONTEXTS = 4;
 const KNOWN_CONTEXTS = [
-  {key: 'user', Component: UserSummary},
-  {key: 'browser', Component: GenericSummary, unknownTitle: t('Unknown Browser')},
-  {key: 'runtime', Component: GenericSummary, unknownTitle: t('Unknown Runtime')},
-  {key: 'os', Component: OsSummary},
-  {key: 'device', Component: DeviceSummary},
-  {key: 'gpu', Component: GpuSummary},
+  {keys: ['user'], Component: UserSummary},
+  {keys: ['browser'], Component: GenericSummary, unknownTitle: t('Unknown Browser')},
+  {keys: ['runtime'], Component: GenericSummary, unknownTitle: t('Unknown Runtime')},
+  {keys: ['client_os', 'os'], Component: OsSummary},
+  {keys: ['device'], Component: DeviceSummary},
+  {keys: ['gpu'], Component: GpuSummary},
 ];
 
 class EventContextSummary extends React.Component {
@@ -253,14 +253,19 @@ class EventContextSummary extends React.Component {
 
     // Add defined contexts in the declared order, until we reach the limit
     // defined by MAX_CONTEXTS.
-    let contexts = KNOWN_CONTEXTS.map(({key, Component, ...props}) => {
+    let contexts = KNOWN_CONTEXTS.map(({keys, Component, ...props}) => {
       if (contextCount >= MAX_CONTEXTS) {
         return null;
       }
-      const data = evt.contexts[key] || evt[key];
-      if (objectIsEmpty(data)) {
+
+      const [key, data] = keys
+        .map(k => [k, evt.contexts[k] || evt[k]])
+        .find(([_k, d]) => !objectIsEmpty(d)) || [null, null];
+
+      if (!key) {
         return null;
       }
+
       contextCount += 1;
       return <Component key={key} data={data} {...props} />;
     });
@@ -273,15 +278,15 @@ class EventContextSummary extends React.Component {
     if (contextCount < MIN_CONTEXTS) {
       // Add contents in the declared order until we have at least MIN_CONTEXTS
       // contexts in our list.
-      contexts = KNOWN_CONTEXTS.map(({key, Component, ...props}, index) => {
+      contexts = KNOWN_CONTEXTS.map(({keys, Component, ...props}, index) => {
         if (contexts[index]) {
           return contexts[index];
         }
         if (contextCount >= MIN_CONTEXTS) {
           return null;
         }
         contextCount += 1;
-        return <Component key={key} data={{}} {...props} />;
+        return <Component key={keys[0]} data={{}} {...props} />;
       });
     }
 

tests/js/spec/components/events/__snapshots__/contextSummary.spec.jsx.snap

@@ -2,6 +2,55 @@
 
 exports[`ContextSummary render() should bail out with empty contexts 1`] = `""`;
 
+exports[`ContextSummary render() should prefer client_os over os 1`] = `
+<div
+  className="context-summary"
+>
+  <UserSummary
+    data={
+      Object {
+        "email": "[email protected]",
+        "id": "1",
+      }
+    }
+    key="user"
+  />
+  <GenericSummary
+    data={
+      Object {
+        "name": "Chrome",
+        "version": "65.0.3325",
+      }
+    }
+    key="browser"
+    unknownTitle="Unknown Browser"
+  />
+  <GenericSummary
+    data={
+      Object {
+        "name": "Electron",
+        "type": "runtime",
+        "version": "1.7.13",
+      }
+    }
+    key="runtime"
+    unknownTitle="Unknown Runtime"
+  />
+  <OsSummary
+    data={
+      Object {
+        "build": "17E199",
+        "kernel_version": "17.5.0",
+        "name": "Mac OS X",
+        "type": "os",
+        "version": "10.13.4",
+      }
+    }
+    key="client_os"
+  />
+</div>
+`;
+
 exports[`ContextSummary render() should render at least three contexts 1`] = `
 <div
   className="context-summary"
@@ -34,6 +83,55 @@ exports[`ContextSummary render() should render at least three contexts 1`] = `
 </div>
 `;
 
+exports[`ContextSummary render() should render client_os too 1`] = `
+<div
+  className="context-summary"
+>
+  <UserSummary
+    data={
+      Object {
+        "email": "[email protected]",
+        "id": "1",
+      }
+    }
+    key="user"
+  />
+  <GenericSummary
+    data={
+      Object {
+        "name": "Chrome",
+        "version": "65.0.3325",
+      }
+    }
+    key="browser"
+    unknownTitle="Unknown Browser"
+  />
+  <GenericSummary
+    data={
+      Object {
+        "name": "Electron",
+        "type": "runtime",
+        "version": "1.7.13",
+      }
+    }
+    key="runtime"
+    unknownTitle="Unknown Runtime"
+  />
+  <OsSummary
+    data={
+      Object {
+        "build": "17E199",
+        "kernel_version": "17.5.0",
+        "name": "Mac OS X",
+        "type": "os",
+        "version": "10.13.4",
+      }
+    }
+    key="client_os"
+  />
+</div>
+`;
+
 exports[`ContextSummary render() should render nothing with a single user context 1`] = `""`;
 
 exports[`ContextSummary render() should render nothing without contexts 1`] = `""`;

tests/js/spec/components/events/contextSummary.spec.jsx

@@ -26,6 +26,14 @@ const CONTEXT_OS = {
   name: 'Mac OS X',
 };
 
+const CONTEXT_OS_SERVER = {
+  kernel_version: '4.3.0',
+  version: '4.3.0',
+  type: 'os',
+  build: '123123123',
+  name: 'Linux',
+};
+
 const CONTEXT_RUNTIME = {
   version: '1.7.13',
   type: 'runtime',
@@ -103,6 +111,37 @@ describe('ContextSummary', function() {
       expect(wrapper).toMatchSnapshot();
     });
 
+    it('should prefer client_os over os', () => {
+      const event = {
+        id: '',
+        user: CONTEXT_USER,
+        contexts: {
+          client_os: CONTEXT_OS,
+          os: CONTEXT_OS_SERVER,
+          browser: CONTEXT_BROWSER,
+          runtime: CONTEXT_RUNTIME,
+        },
+      };
+
+      const wrapper = shallow(<ContextSummary event={event} />);
+      expect(wrapper).toMatchSnapshot();
+    });
+
+    it('should render client_os too', () => {
+      const event = {
+        id: '',
+        user: CONTEXT_USER,
+        contexts: {
+          client_os: CONTEXT_OS,
+          browser: CONTEXT_BROWSER,
+          runtime: CONTEXT_RUNTIME,
+        },
+      };
+
+      const wrapper = shallow(<ContextSummary event={event} />);
+      expect(wrapper).toMatchSnapshot();
+    });
+
     it('should skip non-default named contexts', () => {
       const event = {
         id: '',