Browse Source

fix(analytics): properly type organization for analytics events (#25423)

Stephen Cefali 3 years ago
parent
commit
06b4bf1a04

+ 2 - 20
static/app/components/demo/demoHeader.tsx

@@ -1,21 +1,15 @@
 import React from 'react';
 import styled from '@emotion/styled';
-import createReactClass from 'create-react-class';
-import Reflux from 'reflux';
 
 import Button from 'app/components/button';
 import ButtonBar from 'app/components/buttonBar';
 import ExternalLink from 'app/components/links/externalLink';
 import {IconSentryFull} from 'app/icons';
 import {t} from 'app/locale';
-import OrganizationStore from 'app/stores/organizationStore';
 import space from 'app/styles/space';
-import {Organization} from 'app/types';
 import {trackAdvancedAnalyticsEvent} from 'app/utils/advancedAnalytics';
 
-type Props = {organization?: Organization};
-
-function DemoHeader({organization}: Props) {
+export default function DemoHeader() {
   return (
     <Wrapper>
       <LogoSvg />
@@ -25,7 +19,7 @@ function DemoHeader({organization}: Props) {
         </StyledExternalLink>
         <GetStarted
           onClick={() =>
-            trackAdvancedAnalyticsEvent('growth.demo_click_get_started', {}, organization)
+            trackAdvancedAnalyticsEvent('growth.demo_click_get_started', {}, null)
           }
           href="https://sentry.io/signup/"
         >
@@ -36,18 +30,6 @@ function DemoHeader({organization}: Props) {
   );
 }
 
-//can't use withOrganization here since we aren't within the OrganizationContext
-export default createReactClass<Omit<Props, 'organization'>>({
-  displayName: 'DemoHeader',
-  mixins: [Reflux.connect(OrganizationStore, 'organization') as any],
-  render() {
-    const organization = this.state.organization?.organization as
-      | Organization
-      | undefined;
-    return <DemoHeader organization={organization} />;
-  },
-});
-
 //Note many of the colors don't come from the theme as they come from the marketing site
 const Wrapper = styled('div')`
   padding-right: ${space(3)};

+ 5 - 8
static/app/utils/advancedAnalytics.tsx

@@ -42,7 +42,7 @@ type AnalyticsKey = keyof EventParameters;
 export function trackAdvancedAnalyticsEvent<T extends AnalyticsKey>(
   eventKey: T,
   analyticsParams: EventParameters[T],
-  org?: Organization, //we should pass in org whenever we can but not every place guarantees this
+  org: Organization | null, // if org is undefined, event won't be recorded
   options?: {startSession: boolean},
   mapValuesFn?: (params: object) => object
 ) {
@@ -74,11 +74,14 @@ export function trackAdvancedAnalyticsEvent<T extends AnalyticsKey>(
       // e.g. unencoded "%"
     }
 
+    //if org is null, we want organization_id to be null
+    const organization_id = org ? org.id : org;
+
     let params = {
       eventKey,
       eventName,
       analytics_session_id: sessionId,
-      organization_id: org?.id,
+      organization_id,
       role: org?.role,
       custom_referrer,
       ...analyticsParams,
@@ -88,12 +91,6 @@ export function trackAdvancedAnalyticsEvent<T extends AnalyticsKey>(
       params = mapValuesFn(params) as any;
     }
 
-    //TODO(steve): remove once we pass in org always
-    if (hasAnalyticsDebug() && !org) {
-      // eslint-disable-next-line no-console
-      console.warn(`Organization absent from event ${eventKey}`);
-    }
-
     //could put this into a debug method or for the main trackAnalyticsEvent event
     if (hasAnalyticsDebug()) {
       // eslint-disable-next-line no-console

+ 1 - 1
static/app/utils/integrationUtil.tsx

@@ -41,7 +41,7 @@ const mapIntegrationParams = analyticsParams => {
 export function trackIntegrationEvent<T extends IntegrationAnalyticsKey>(
   eventKey: T,
   analyticsParams: EventParameters[T],
-  org?: Organization,
+  org: Organization, // integration events should always be tied to an org
   options?: Parameters<typeof trackAdvancedAnalyticsEvent>[3]
 ) {
   return trackAdvancedAnalyticsEvent(

+ 1 - 0
static/app/views/integrationOrganizationLink.tsx

@@ -180,6 +180,7 @@ export default class IntegrationOrganizationLink extends AsyncView<Props, State>
           <AddIntegration
             provider={provider}
             onInstall={this.onInstallWithInstallationId}
+            organization={organization}
           >
             {addIntegrationWithInstallationId => (
               <ButtonWrapper>

+ 1 - 1
static/app/views/organizationIntegrations/addIntegration.tsx

@@ -13,7 +13,7 @@ type Props = {
   provider: IntegrationProvider;
   onInstall: (data: IntegrationWithConfig) => void;
   account?: string;
-  organization?: Organization; //for analytics
+  organization: Organization; //for analytics
   analyticsParams?: {
     view:
       | 'integrations_directory_integration_detail'

+ 0 - 67
static/app/views/organizationIntegrations/migrationWarnings.tsx

@@ -1,67 +0,0 @@
-import React from 'react';
-import groupBy from 'lodash/groupBy';
-
-import AlertLink from 'app/components/alertLink';
-import AsyncComponent from 'app/components/asyncComponent';
-import {tct} from 'app/locale';
-import {Integration, IntegrationProvider, Repository} from 'app/types';
-import AddIntegration from 'app/views/organizationIntegrations/addIntegration';
-
-type Props = {
-  orgId: string;
-  providers: IntegrationProvider[];
-  onInstall: (integration: Integration) => void;
-} & AsyncComponent['props'];
-
-type State = {
-  unmigratableRepos: Repository[];
-} & AsyncComponent['state'];
-
-export default class MigrationWarnings extends AsyncComponent<Props, State> {
-  getEndpoints(): ReturnType<AsyncComponent['getEndpoints']> {
-    const {orgId} = this.props;
-
-    return [['unmigratableRepos', `/organizations/${orgId}/repos/?status=unmigratable`]];
-  }
-
-  get unmigratableReposByOrg() {
-    // Group by [GitHub|BitBucket|VSTS] Org name
-    return groupBy(this.state.unmigratableRepos, repo => repo.name.split('/')[0]);
-  }
-
-  render() {
-    return Object.entries(this.unmigratableReposByOrg).map(
-      ([orgName, repos]: [string, Repository[]]) => {
-        // Repos use 'visualstudio', Integrations use 'vsts'. Normalize to 'vsts'.
-        const key =
-          repos[0].provider.id === 'visualstudio' ? 'vsts' : repos[0].provider.id;
-        const provider = this.props.providers.find(p => p.key === key);
-
-        // The Org might not have access to this Integration yet.
-        if (!provider) {
-          return '';
-        }
-
-        return (
-          <AddIntegration
-            key={provider.key}
-            provider={provider}
-            onInstall={this.props.onInstall}
-          >
-            {onClick => (
-              <AlertLink href="" onClick={() => onClick()}>
-                {tct(
-                  "Your [orgName] repositories can't send commit data to Sentry. Add a [orgName] [providerName] instance here.",
-                  {
-                    orgName,
-                    providerName: provider.name,
-                  }
-                )}
-              </AlertLink>
-            )}
-          </AddIntegration>
-        );
-      }
-    );
-  }
-}

+ 1 - 0
static/app/views/settings/organizationIntegrations/configureIntegration.tsx

@@ -104,6 +104,7 @@ class ConfigureIntegration extends AsyncView<Props, State> {
           provider={provider}
           onInstall={this.onUpdateIntegration}
           account={integration.domainName}
+          organization={this.props.organization}
         >
           {onClick => (
             <Button