Browse Source

fix(project-dsn-config): Fix Slider Rate limit wrong data (#29405)

Priscila Oliveira 3 years ago
parent
commit
8cff3bc746

+ 5 - 0
static/app/utils/formatters.tsx

@@ -70,6 +70,7 @@ export function getDuration(
       abbreviation ? tn('mo', 'mos', result) : ` ${tn('month', 'months', result)}`
     }`;
   }
+
   if (value >= WEEK) {
     const {label, result} = roundWithFixed(msValue / WEEK, fixedDigits);
     if (extraShort) {
@@ -80,12 +81,14 @@ export function getDuration(
     }
     return `${label} ${tn('week', 'weeks', result)}`;
   }
+
   if (value >= 172800000) {
     const {label, result} = roundWithFixed(msValue / DAY, fixedDigits);
     return `${label}${
       abbreviation || extraShort ? t('d') : ` ${tn('day', 'days', result)}`
     }`;
   }
+
   if (value >= 7200000) {
     const {label, result} = roundWithFixed(msValue / HOUR, fixedDigits);
     if (extraShort) {
@@ -96,6 +99,7 @@ export function getDuration(
     }
     return `${label} ${tn('hour', 'hours', result)}`;
   }
+
   if (value >= 120000) {
     const {label, result} = roundWithFixed(msValue / MINUTE, fixedDigits);
     if (extraShort) {
@@ -106,6 +110,7 @@ export function getDuration(
     }
     return `${label} ${tn('minute', 'minutes', result)}`;
   }
+
   if (value >= SECOND) {
     const {label, result} = roundWithFixed(msValue / SECOND, fixedDigits);
     if (extraShort || abbreviation) {

+ 157 - 137
static/app/views/settings/project/projectKeys/details/keyRateLimitsForm.tsx

@@ -1,40 +1,31 @@
 import * as React from 'react';
 import {RouteComponentProps} from 'react-router';
 import styled from '@emotion/styled';
+import sortBy from 'lodash/sortBy';
 
 import Feature from 'app/components/acl/feature';
 import FeatureDisabled from 'app/components/acl/featureDisabled';
 import {Panel, PanelAlert, PanelBody, PanelHeader} from 'app/components/panels';
 import {IconFlag} from 'app/icons';
-import {t} from 'app/locale';
+import {t, tct, tn} from 'app/locale';
 import space from 'app/styles/space';
+import {defined} from 'app/utils';
+import {getExactDuration} from 'app/utils/formatters';
 import InputControl from 'app/views/settings/components/forms/controls/input';
 import RangeSlider from 'app/views/settings/components/forms/controls/rangeSlider';
 import Form from 'app/views/settings/components/forms/form';
 import FormField from 'app/views/settings/components/forms/formField';
 import {ProjectKey} from 'app/views/settings/project/projectKeys/types';
 
-const RATE_LIMIT_FORMAT_MAP = new Map([
-  [0, 'None'],
-  [60, '1 minute'],
-  [300, '5 minutes'],
-  [900, '15 minutes'],
-  [3600, '1 hour'],
-  [7200, '2 hours'],
-  [14400, '4 hours'],
-  [21600, '6 hours'],
-  [43200, '12 hours'],
-  [86400, '24 hours'],
-]);
+const PREDEFINED_RATE_LIMIT_VALUES = [
+  0, 60, 300, 900, 3600, 7200, 14400, 21600, 43200, 86400,
+];
 
 type RateLimitValue = {
   window: number;
   count: number;
 };
 
-// This value isn't actually any, but the various angles on the types don't line up.
-const formatRateLimitWindow = (val: any) => RATE_LIMIT_FORMAT_MAP.get(val);
-
 type Props = {
   data: ProjectKey;
   disabled: boolean;
@@ -50,150 +41,179 @@ type Props = {
   'params'
 >;
 
-class KeyRateLimitsForm extends React.Component<Props> {
-  handleChangeWindow = (
-    onChange,
-    onBlur,
+function KeyRateLimitsForm({data, disabled, params}: Props) {
+  function handleChangeWindow(
+    onChange: (value: RateLimitValue, event: React.ChangeEvent<HTMLInputElement>) => void,
+    onBlur: (value: RateLimitValue, event: React.ChangeEvent<HTMLInputElement>) => void,
     currentValueObj: RateLimitValue,
     value: number,
-    e: React.ChangeEvent<HTMLInputElement>
-  ) => {
-    const valueObj = {
-      ...currentValueObj,
-      window: value,
-    };
-    onChange(valueObj, e);
-    onBlur(valueObj, e);
-  };
+    event: React.ChangeEvent<HTMLInputElement>
+  ) {
+    const valueObj = {...currentValueObj, window: value};
+
+    onChange(valueObj, event);
+    onBlur(valueObj, event);
+  }
 
-  handleChangeCount = (
-    cb,
+  function handleChangeCount(
+    callback: (value: RateLimitValue, event: React.ChangeEvent<HTMLInputElement>) => void,
     value: RateLimitValue,
-    e: React.ChangeEvent<HTMLInputElement>
-  ) => {
+    event: React.ChangeEvent<HTMLInputElement>
+  ) {
     const valueObj = {
       ...value,
-      count: e.target.value,
+      count: Number(event.target.value),
     };
 
-    cb(valueObj, e);
-  };
-
-  render() {
-    const {data, disabled} = this.props;
-    const {keyId, orgId, projectId} = this.props.params;
-    const apiEndpoint = `/projects/${orgId}/${projectId}/keys/${keyId}/`;
-
-    const disabledAlert = ({features}) => (
-      <FeatureDisabled
-        alert={PanelAlert}
-        features={features}
-        featureName={t('Key Rate Limits')}
-      />
-    );
-
-    return (
-      <Form saveOnBlur apiEndpoint={apiEndpoint} apiMethod="PUT" initialData={data}>
-        <Feature
-          features={['projects:rate-limits']}
-          hookName="feature-disabled:rate-limits"
-          renderDisabled={({children, ...props}) =>
-            typeof children === 'function' &&
-            children({...props, renderDisabled: disabledAlert})
-          }
-        >
-          {({hasFeature, features, organization, project, renderDisabled}) => (
-            <Panel>
-              <PanelHeader>{t('Rate Limits')}</PanelHeader>
-
-              <PanelBody>
-                <PanelAlert type="info" icon={<IconFlag size="md" />}>
-                  {t(
-                    `Rate limits provide a flexible way to manage your error
-                      volume. If you have a noisy project or environment you
-                      can configure a rate limit for this key to reduce the
-                      number of errors processed. To manage your transaction
-                      volume, we recommend adjusting your sample rate in your
-                      SDK configuration.`
-                  )}
-                </PanelAlert>
-                {!hasFeature &&
-                  typeof renderDisabled === 'function' &&
-                  renderDisabled({
-                    organization,
-                    project,
-                    features,
-                    hasFeature,
-                    children: null,
-                  })}
-                <FormField
-                  className="rate-limit-group"
-                  name="rateLimit"
-                  label={t('Rate Limit')}
-                  disabled={disabled || !hasFeature}
-                  validate={({form}) => {
-                    // TODO(TS): is validate actually doing anything because it's an unexpected prop
-                    const isValid =
-                      form &&
-                      form.rateLimit &&
-                      typeof form.rateLimit.count !== 'undefined' &&
-                      typeof form.rateLimit.window !== 'undefined';
-
-                    if (isValid) {
-                      return [];
-                    }
-
-                    return [['rateLimit', t('Fill in both fields first')]];
-                  }}
-                  formatMessageValue={(value: RateLimitValue) => {
-                    return t(
-                      '%s errors in %s',
-                      value.count,
-                      formatRateLimitWindow(value.window)
-                    );
-                  }}
-                  help={t(
-                    'Apply a rate limit to this credential to cap the amount of errors accepted during a time window.'
-                  )}
-                  inline={false}
-                >
-                  {({onChange, onBlur, value}) => (
+    callback(valueObj, event);
+  }
+
+  function getAllowedRateLimitValues(currentRateLimit?: number) {
+    const {rateLimit} = data;
+    const {window} = rateLimit ?? {};
+
+    // The slider should display other values if they are set via the API, but still offer to select only the predefined values
+    if (defined(window)) {
+      // If the API returns a value not found in the predefined values and the user selects another value through the UI,
+      // he will no longer be able to reselect the "custom" value in the slider
+      if (currentRateLimit !== window) {
+        return PREDEFINED_RATE_LIMIT_VALUES;
+      }
+
+      // If the API returns a value not found in the predefined values, that value will be added to the slider
+      if (!PREDEFINED_RATE_LIMIT_VALUES.includes(window)) {
+        return sortBy([...PREDEFINED_RATE_LIMIT_VALUES, window]);
+      }
+    }
+
+    return PREDEFINED_RATE_LIMIT_VALUES;
+  }
+
+  const {keyId, orgId, projectId} = params;
+  const apiEndpoint = `/projects/${orgId}/${projectId}/keys/${keyId}/`;
+
+  const disabledAlert = ({features}) => (
+    <FeatureDisabled
+      alert={PanelAlert}
+      features={features}
+      featureName={t('Key Rate Limits')}
+    />
+  );
+
+  return (
+    <Form saveOnBlur apiEndpoint={apiEndpoint} apiMethod="PUT" initialData={data}>
+      <Feature
+        features={['projects:rate-limits']}
+        hookName="feature-disabled:rate-limits"
+        renderDisabled={({children, ...props}) =>
+          typeof children === 'function' &&
+          children({...props, renderDisabled: disabledAlert})
+        }
+      >
+        {({hasFeature, features, organization, project, renderDisabled}) => (
+          <Panel>
+            <PanelHeader>{t('Rate Limits')}</PanelHeader>
+
+            <PanelBody>
+              <PanelAlert type="info" icon={<IconFlag size="md" />}>
+                {t(
+                  `Rate limits provide a flexible way to manage your error
+                    volume. If you have a noisy project or environment you
+                    can configure a rate limit for this key to reduce the
+                    number of errors processed. To manage your transaction
+                    volume, we recommend adjusting your sample rate in your
+                    SDK configuration.`
+                )}
+              </PanelAlert>
+              {!hasFeature &&
+                typeof renderDisabled === 'function' &&
+                renderDisabled({
+                  organization,
+                  project,
+                  features,
+                  hasFeature,
+                  children: null,
+                })}
+              <FormField
+                name="rateLimit"
+                label={t('Rate Limit')}
+                disabled={disabled || !hasFeature}
+                validate={({form}) => {
+                  // TODO(TS): is validate actually doing anything because it's an unexpected prop
+                  const isValid =
+                    form &&
+                    form.rateLimit &&
+                    typeof form.rateLimit.count !== 'undefined' &&
+                    typeof form.rateLimit.window !== 'undefined';
+
+                  if (isValid) {
+                    return [];
+                  }
+
+                  return [['rateLimit', t('Fill in both fields first')]];
+                }}
+                formatMessageValue={({count, window}: RateLimitValue) =>
+                  tct('[errors] in [timeWindow]', {
+                    errors: tn('%s error ', '%s errors ', count),
+                    timeWindow:
+                      window === 0 ? t('no time window') : getExactDuration(window),
+                  })
+                }
+                help={t(
+                  'Apply a rate limit to this credential to cap the amount of errors accepted during a time window.'
+                )}
+                inline={false}
+              >
+                {({onChange, onBlur, value}) => {
+                  const window = typeof value === 'object' ? value.window : undefined;
+                  return (
                     <RateLimitRow>
                       <InputControl
                         type="number"
                         name="rateLimit.count"
                         min={0}
-                        value={value && value.count}
+                        value={typeof value === 'object' ? value.count : undefined}
                         placeholder={t('Count')}
                         disabled={disabled || !hasFeature}
-                        onChange={this.handleChangeCount.bind(this, onChange, value)}
-                        onBlur={this.handleChangeCount.bind(this, onBlur, value)}
+                        onChange={event => handleChangeCount(onChange, value, event)}
+                        onBlur={event => handleChangeCount(onBlur, value, event)}
                       />
                       <EventsIn>{t('event(s) in')}</EventsIn>
                       <RangeSlider
                         name="rateLimit.window"
-                        allowedValues={Array.from(RATE_LIMIT_FORMAT_MAP.keys())}
-                        value={value && value.window}
+                        allowedValues={getAllowedRateLimitValues(window)}
+                        value={window}
                         placeholder={t('Window')}
-                        formatLabel={formatRateLimitWindow}
+                        formatLabel={rangeValue => {
+                          if (typeof rangeValue === 'number') {
+                            if (rangeValue === 0) {
+                              return t('None');
+                            }
+                            return getExactDuration(rangeValue);
+                          }
+                          return undefined;
+                        }}
                         disabled={disabled || !hasFeature}
-                        onChange={this.handleChangeWindow.bind(
-                          this,
-                          onChange,
-                          onBlur,
-                          value
-                        )}
+                        onChange={(rangeValue, event) =>
+                          handleChangeWindow(
+                            onChange,
+                            onBlur,
+                            value,
+                            Number(rangeValue),
+                            event
+                          )
+                        }
                       />
                     </RateLimitRow>
-                  )}
-                </FormField>
-              </PanelBody>
-            </Panel>
-          )}
-        </Feature>
-      </Form>
-    );
-  }
+                  );
+                }}
+              </FormField>
+            </PanelBody>
+          </Panel>
+        )}
+      </Feature>
+    </Form>
+  );
 }
 
 export default KeyRateLimitsForm;

+ 4 - 1
static/app/views/settings/project/projectKeys/types.tsx

@@ -6,7 +6,10 @@ export type ProjectKey = {
   };
   name: string;
   projectId: number;
-  rateLimit: number | null;
+  rateLimit: {
+    window: number;
+    count: number;
+  } | null;
   label: string;
   dsn: {
     cdn: string;

+ 0 - 6
static/less/shared/forms.less

@@ -735,12 +735,6 @@ output {
   margin-bottom: 15px;
 }
 
-.rate-limit-group {
-  .control-group {
-    margin-bottom: 0;
-  }
-}
-
 .form-actions {
   border-top: 1px solid #e9ebec;
   background: none;