Browse Source

ref(sampling): Send uniform rule always last position array (#36421)

Priscila Oliveira 2 years ago
parent
commit
c5c9a9d64d

+ 0 - 5
static/app/types/sampling.tsx

@@ -172,11 +172,6 @@ export type SamplingRule = {
    * Indicates if the rule is enabled for server-side sampling
    */
   active?: boolean;
-  /**
-   * A rule without a condition (Else case) always have to be 'pinned'
-   * to the bottom of the list and cannot be sorted.
-   */
-  bottomPinned?: boolean;
 };
 
 export type SamplingDistribution = {

+ 1 - 1
static/app/views/settings/project/sampling/rules/draggableList/index.tsx

@@ -74,7 +74,7 @@ export function DraggableList({
             id={itemId}
             index={index}
             renderItem={renderItem}
-            disabled={disabled || items[index].bottomPinned}
+            disabled={disabled || items[index].condition.inner.length === 0}
             wrapperStyle={wrapperStyle}
           />
         ))}

+ 2 - 3
static/app/views/settings/project/sampling/rules/index.tsx

@@ -109,7 +109,6 @@ export class Rules extends PureComponent<Props, State> {
     const items = rules.map(rule => ({
       ...rule,
       id: String(rule.id),
-      disabled: !rule.condition.inner.length,
     }));
 
     return (
@@ -163,12 +162,12 @@ export class Rules extends PureComponent<Props, State> {
                 operator={
                   itemsRule.id === items[0].id
                     ? SamplingRuleOperator.IF
-                    : itemsRule.disabled
+                    : currentRule.condition.inner.length === 0
                     ? SamplingRuleOperator.ELSE
                     : SamplingRuleOperator.ELSE_IF
                 }
                 hideGrabButton={items.length === 1}
-                rule={{...currentRule, bottomPinned: itemsRule.bottomPinned}}
+                rule={currentRule}
                 onEditRule={onEditRule(currentRule)}
                 onDeleteRule={onDeleteRule(currentRule)}
                 noPermission={disabled}

+ 2 - 1
static/app/views/settings/project/sampling/rules/rule/index.tsx

@@ -9,6 +9,7 @@ import {t} from 'sentry/locale';
 import space from 'sentry/styles/space';
 import {SamplingRule, SamplingRuleOperator} from 'sentry/types/sampling';
 
+import {isUniformRule} from '../../../server-side-sampling/utils';
 import {getInnerNameLabel} from '../../utils';
 import {layout} from '../utils';
 
@@ -57,7 +58,7 @@ export function Rule({
   }, [dragging, sorting, state.isMenuActionsOpen]);
 
   return (
-    <Columns disabled={rule.bottomPinned || noPermission}>
+    <Columns disabled={isUniformRule(rule) || noPermission}>
       {hideGrabButton ? (
         <Column />
       ) : (

+ 106 - 0
static/app/views/settings/project/server-side-sampling/draggableRuleList.tsx

@@ -0,0 +1,106 @@
+import {useState} from 'react';
+import {createPortal} from 'react-dom';
+import {DndContext, DragOverlay} from '@dnd-kit/core';
+import {arrayMove, SortableContext, verticalListSortingStrategy} from '@dnd-kit/sortable';
+
+import {SamplingRule} from 'sentry/types/sampling';
+
+import {DraggableRuleListItem, DraggableRuleListItemProps} from './draggableRuleListItem';
+import {
+  DraggableRuleListSortableItem,
+  SortableItemProps,
+} from './draggableRuleListSortableItem';
+import {isUniformRule} from './utils';
+
+export type DraggableRuleListUpdateItemsProps = {
+  activeIndex: string;
+  overIndex: string;
+  reorderedItems: Array<string>;
+};
+
+type Props = Pick<SortableItemProps, 'disabled' | 'wrapperStyle'> &
+  Pick<DraggableRuleListItemProps, 'renderItem'> & {
+    items: Array<
+      Omit<SamplingRule, 'id'> & {
+        id: string;
+      }
+    >;
+    onUpdateItems: (props: DraggableRuleListUpdateItemsProps) => void;
+  };
+
+type State = {
+  activeId?: string;
+};
+
+export function DraggableRuleList({
+  items,
+  onUpdateItems,
+  renderItem,
+  disabled = false,
+  wrapperStyle = () => ({}),
+}: Props) {
+  const [state, setState] = useState<State>({});
+
+  const itemIds = items.map(item => item.id);
+  const getIndex = itemIds.indexOf.bind(itemIds);
+  const activeIndex = state.activeId ? getIndex(state.activeId) : -1;
+
+  return (
+    <DndContext
+      onDragStart={({active}) => {
+        if (!active) {
+          return;
+        }
+
+        setState({activeId: active.id});
+      }}
+      onDragEnd={({over}) => {
+        setState({activeId: undefined});
+
+        if (over) {
+          const overIndex = getIndex(over.id);
+
+          if (activeIndex !== overIndex) {
+            onUpdateItems({
+              activeIndex,
+              overIndex,
+              reorderedItems: arrayMove(itemIds, activeIndex, overIndex),
+            });
+          }
+        }
+      }}
+      onDragCancel={() => setState({activeId: undefined})}
+    >
+      <SortableContext items={itemIds} strategy={verticalListSortingStrategy}>
+        {itemIds.map((itemId, index) => (
+          <DraggableRuleListSortableItem
+            key={itemId}
+            id={itemId}
+            index={index}
+            renderItem={renderItem}
+            disabled={
+              disabled || isUniformRule({...items[index], id: Number(items[index].id)})
+            }
+            wrapperStyle={wrapperStyle}
+          />
+        ))}
+      </SortableContext>
+      {createPortal(
+        <DragOverlay>
+          {state.activeId ? (
+            <DraggableRuleListItem
+              value={itemIds[activeIndex]}
+              renderItem={renderItem}
+              wrapperStyle={wrapperStyle({
+                index: activeIndex,
+                isDragging: true,
+                isSorting: false,
+              })}
+            />
+          ) : null}
+        </DragOverlay>,
+        document.body
+      )}
+    </DndContext>
+  );
+}

+ 109 - 0
static/app/views/settings/project/server-side-sampling/draggableRuleListItem.tsx

@@ -0,0 +1,109 @@
+import {DraggableSyntheticListeners} from '@dnd-kit/core';
+import {useSortable} from '@dnd-kit/sortable';
+import {Transform} from '@dnd-kit/utilities';
+import styled from '@emotion/styled';
+
+type UseSortableOutputProps = ReturnType<typeof useSortable>;
+
+export type DraggableRuleListItemProps = {
+  renderItem(args: {
+    dragging: boolean;
+    sorting: boolean;
+    value: DraggableRuleListItemProps['value'];
+    attributes?: DraggableRuleListItemProps['attributes'];
+    index?: DraggableRuleListItemProps['index'];
+    listeners?: DraggableRuleListItemProps['listeners'];
+    transform?: DraggableRuleListItemProps['transform'];
+    transition?: DraggableRuleListItemProps['transition'];
+  }): React.ReactElement | null;
+  value: React.ReactNode;
+  attributes?: UseSortableOutputProps['attributes'];
+  dragging?: boolean;
+  forwardRef?: React.Ref<HTMLDivElement>;
+  index?: number;
+  listeners?: DraggableSyntheticListeners;
+  sorting?: boolean;
+  transform?: Transform | null;
+  transition?: string | null;
+  wrapperStyle?: React.CSSProperties;
+};
+
+export function DraggableRuleListItem({
+  value,
+  dragging,
+  index,
+  transform,
+  listeners,
+  sorting,
+  transition,
+  forwardRef,
+  attributes,
+  renderItem,
+  wrapperStyle,
+}: DraggableRuleListItemProps) {
+  return (
+    <Wrapper
+      data-test-id="sampling-rule"
+      ref={forwardRef}
+      style={
+        {
+          ...wrapperStyle,
+          transition,
+          '--translate-x': transform ? `${Math.round(transform.x)}px` : undefined,
+          '--translate-y': transform ? `${Math.round(transform.y)}px` : undefined,
+          '--scale-x': transform?.scaleX ? `${transform.scaleX}` : undefined,
+          '--scale-y': transform?.scaleY ? `${transform.scaleY}` : undefined,
+        } as React.CSSProperties
+      }
+    >
+      <InnerWrapper>
+        {renderItem({
+          dragging: Boolean(dragging),
+          sorting: Boolean(sorting),
+          listeners,
+          transform,
+          transition,
+          value,
+          index,
+          attributes,
+        })}
+      </InnerWrapper>
+    </Wrapper>
+  );
+}
+
+const boxShadowBorder = '0 0 0 calc(1px / var(--scale-x, 1)) rgba(63, 63, 68, 0.05)';
+const boxShadowCommon = '0 1px calc(3px / var(--scale-x, 1)) 0 rgba(34, 33, 81, 0.15)';
+const boxShadow = `${boxShadowBorder}, ${boxShadowCommon}`;
+
+const Wrapper = styled('div')`
+  transform: translate3d(var(--translate-x, 0), var(--translate-y, 0), 0)
+    scaleX(var(--scale-x, 1)) scaleY(var(--scale-y, 1));
+  transform-origin: 0 0;
+  touch-action: manipulation;
+  --box-shadow: ${boxShadow};
+  --box-shadow-picked-up: ${boxShadowBorder}, -1px 0 15px 0 rgba(34, 33, 81, 0.01),
+    0px 15px 15px 0 rgba(34, 33, 81, 0.25);
+`;
+
+const InnerWrapper = styled('div')`
+  background-color: ${p => p.theme.background};
+
+  animation: pop 200ms cubic-bezier(0.18, 0.67, 0.6, 1.22);
+  box-shadow: var(--box-shadow-picked-up);
+  opacity: 1;
+
+  :focus {
+    box-shadow: 0 0px 4px 1px rgba(76, 159, 254, 1), ${boxShadow};
+  }
+
+  @keyframes pop {
+    0% {
+      transform: scale(1);
+      box-shadow: var(--box-shadow);
+    }
+    100% {
+      box-shadow: var(--box-shadow-picked-up);
+    }
+  }
+`;

+ 50 - 0
static/app/views/settings/project/server-side-sampling/draggableRuleListSortableItem.tsx

@@ -0,0 +1,50 @@
+import {useSortable} from '@dnd-kit/sortable';
+
+import {DraggableRuleListItem} from './draggableRuleListItem';
+
+export type SortableItemProps = Pick<
+  React.ComponentProps<typeof DraggableRuleListItem>,
+  'renderItem' | 'index'
+> & {
+  id: string;
+  index: number;
+  wrapperStyle(args: {
+    index: number;
+    isDragging: boolean;
+    isSorting: boolean;
+  }): React.CSSProperties;
+  disabled?: boolean;
+};
+
+export function DraggableRuleListSortableItem({
+  id,
+  index,
+  renderItem,
+  disabled,
+  wrapperStyle,
+}: SortableItemProps) {
+  const {
+    attributes,
+    isSorting,
+    isDragging,
+    listeners,
+    setNodeRef,
+    transform,
+    transition,
+  } = useSortable({id, disabled});
+
+  return (
+    <DraggableRuleListItem
+      forwardRef={setNodeRef}
+      value={id}
+      sorting={isSorting}
+      renderItem={renderItem}
+      index={index}
+      transform={transform}
+      transition={transition}
+      listeners={listeners}
+      attributes={attributes}
+      wrapperStyle={wrapperStyle({index, isDragging, isSorting})}
+    />
+  );
+}

+ 8 - 1
static/app/views/settings/project/server-side-sampling/modals/specificConditionsModal/index.tsx

@@ -1,6 +1,7 @@
 import {Fragment, KeyboardEvent, useEffect, useState} from 'react';
 import {createFilter} from 'react-select';
 import styled from '@emotion/styled';
+import partition from 'lodash/partition';
 
 import {addErrorMessage, addSuccessMessage} from 'sentry/actionCreators/indicator';
 import {ModalRenderProps} from 'sentry/actionCreators/modal';
@@ -153,12 +154,18 @@ export function SpecificConditionsModal({
       ? rules.map(existingRule => (existingRule.id === rule.id ? newRule : existingRule))
       : [...rules, newRule];
 
+    // Make sure that a uniform rule is always send in the last position of the rules array
+    const [uniformRule, specificRules] = partition(newRules, isUniformRule);
+
     setIsSaving(true);
 
     try {
       const response = await api.requestPromise(
         `/projects/${organization.slug}/${project.slug}/`,
-        {method: 'PUT', data: {dynamicSampling: {rules: newRules}}}
+        {
+          method: 'PUT',
+          data: {dynamicSampling: {rules: [...specificRules, ...uniformRule]}},
+        }
       );
       ProjectStore.onUpdateSuccess(response);
       addSuccessMessage(

+ 9 - 7
static/app/views/settings/project/server-side-sampling/rule.tsx

@@ -56,17 +56,18 @@ export function Rule({
   const isUniform = isUniformRule(rule);
   const canDelete = !noPermission && !isUniform;
   const canActivate = !upgradeSdkForProjects.length;
+  const canDrag = !isUniform && !noPermission;
 
   return (
     <Fragment>
-      <GrabColumn disabled={rule.bottomPinned || noPermission}>
+      <GrabColumn disabled={!canDrag}>
         {hideGrabButton ? null : (
           <Tooltip
             title={
               noPermission
-                ? t('You do not have permission to reorder rules.')
-                : operator === SamplingRuleOperator.ELSE
-                ? t('Rules without conditions cannot be reordered.')
+                ? t('You do not have permission to reorder rules')
+                : isUniform
+                ? t('Uniform rules cannot be reordered')
                 : undefined
             }
             containerDisplayMode="flex"
@@ -75,6 +76,7 @@ export function Rule({
               {...listeners}
               {...grabAttributes}
               aria-label={dragging ? t('Drop Rule') : t('Drag Rule')}
+              aria-disabled={!canDrag}
             >
               <IconGrabbable />
             </IconGrabbableWrapper>
@@ -171,7 +173,7 @@ export function Rule({
           >
             <Tooltip
               disabled={!noPermission}
-              title={t('You do not have permission to edit sampling rules.')}
+              title={t('You do not have permission to edit sampling rules')}
               containerDisplayMode="block"
             >
               {t('Edit')}
@@ -190,8 +192,8 @@ export function Rule({
               disabled={canDelete}
               title={
                 isUniform
-                  ? t("You can't delete the uniform rule.")
-                  : t('You do not have permission to delete sampling rules.')
+                  ? t("You can't delete the uniform rule")
+                  : t('You do not have permission to delete sampling rules')
               }
               containerDisplayMode="block"
             >

+ 9 - 13
static/app/views/settings/project/server-side-sampling/serverSideSampling.tsx

@@ -35,14 +35,13 @@ import SettingsPageHeader from 'sentry/views/settings/components/settingsPageHea
 import TextBlock from 'sentry/views/settings/components/text/textBlock';
 import PermissionAlert from 'sentry/views/settings/organization/permissionAlert';
 
-import {DraggableList, UpdateItemsProps} from '../sampling/rules/draggableList';
-
 import {SpecificConditionsModal} from './modals/specificConditionsModal';
 import {responsiveModal} from './modals/styles';
 import {UniformRateModal} from './modals/uniformRateModal';
 import useProjectStats from './utils/useProjectStats';
 import useSamplingDistribution from './utils/useSamplingDistribution';
 import useSdkVersions from './utils/useSdkVersions';
+import {DraggableRuleList, DraggableRuleListUpdateItemsProps} from './draggableRuleList';
 import {Promo} from './promo';
 import {
   ActiveColumn,
@@ -112,7 +111,6 @@ export function ServerSideSampling({project}: Props) {
   const items = rules.map(rule => ({
     ...rule,
     id: String(rule.id),
-    bottomPinned: !rule.condition.inner.length,
   }));
 
   const recommendedSdkUpgrades = projects
@@ -191,11 +189,12 @@ export function ServerSideSampling({project}: Props) {
     );
   }
 
-  async function handleSortRules({overIndex, reorderedItems: ruleIds}: UpdateItemsProps) {
+  async function handleSortRules({
+    overIndex,
+    reorderedItems: ruleIds,
+  }: DraggableRuleListUpdateItemsProps) {
     if (!rules[overIndex].condition.inner.length) {
-      addErrorMessage(
-        t('Rules with conditions cannot be below rules without conditions')
-      );
+      addErrorMessage(t('Specific rules cannot be below uniform rules'));
       return;
     }
 
@@ -368,7 +367,7 @@ export function ServerSideSampling({project}: Props) {
           )}
           {!!rules.length && (
             <Fragment>
-              <DraggableList
+              <DraggableRuleList
                 disabled={!hasAccess}
                 items={items}
                 onUpdateItems={handleSortRules}
@@ -415,15 +414,12 @@ export function ServerSideSampling({project}: Props) {
                         operator={
                           itemsRule.id === items[0].id
                             ? SamplingRuleOperator.IF
-                            : itemsRule.bottomPinned
+                            : isUniformRule(currentRule)
                             ? SamplingRuleOperator.ELSE
                             : SamplingRuleOperator.ELSE_IF
                         }
                         hideGrabButton={items.length === 1}
-                        rule={{
-                          ...currentRule,
-                          bottomPinned: itemsRule.bottomPinned,
-                        }}
+                        rule={currentRule}
                         onEditRule={() => handleEditRule(currentRule)}
                         onDeleteRule={() => handleDeleteRule(currentRule)}
                         onActivate={() => handleActivateToggle(currentRule.id)}

Some files were not shown because too many files changed in this diff