fix(categorize): handle drag targets correctly PD-5514

packages/categorize/configure/src/design/__tests__/index.test.jsx

@@ -105,4 +105,118 @@ describe('Design', () => {
       expect(container).toBeInTheDocument();
     });
   });
+
+  describe('onDragEnd', () => {
+    const createInstance = (modelExtras = {}) => {
+      const instance = new Design({
+        model: model({
+          allowAlternateEnabled: true,
+          ...modelExtras,
+        }),
+        onChange,
+      });
+
+      instance.setState = jest.fn();
+      instance.removeChoiceFromSource = jest.fn();
+      instance.moveChoice = jest.fn();
+      instance.addChoiceToCategory = jest.fn();
+      instance.moveChoiceInAlternate = jest.fn();
+      instance.addChoiceToAlternateCategory = jest.fn();
+
+      return instance;
+    };
+
+    const eventFor = ({ activeData, overData }) => ({
+      active: activeData ? { data: { current: activeData } } : null,
+      over: overData ? { data: { current: overData } } : null,
+    });
+
+    it('routes choice-preview with no target to removeChoiceFromSource', () => {
+      const instance = createInstance();
+      const activeData = {
+        type: 'choice-preview',
+        id: 'c1-cat1-0',
+        categoryId: '1',
+        choiceIndex: 0,
+      };
+
+      instance.onDragEnd(eventFor({ activeData }));
+
+      expect(instance.removeChoiceFromSource).toHaveBeenCalledWith(
+        activeData,
+        0,
+        expect.objectContaining({
+          allowAlternateEnabled: true,
+          categories: expect.any(Array),
+          choices: expect.any(Array),
+        }),
+      );
+    });
+
+    it('routes choice-preview dropped on choice pool to removeChoiceFromSource', () => {
+      const instance = createInstance();
+      const activeData = {
+        type: 'choice-preview',
+        id: 'c1-cat1-0',
+        categoryId: '1',
+        choiceIndex: 0,
+      };
+      const overData = { type: 'choice' };
+
+      instance.onDragEnd(eventFor({ activeData, overData }));
+
+      expect(instance.removeChoiceFromSource).toHaveBeenCalled();
+      expect(instance.moveChoice).not.toHaveBeenCalled();
+    });
+
+    it('routes choice-preview dropped on category to moveChoice', () => {
+      const instance = createInstance();
+      const activeData = {
+        type: 'choice-preview',
+        id: 'c1-cat1-0',
+        categoryId: '1',
+        choiceIndex: 2,
+      };
+      const overData = { type: 'category', id: '2' };
+
+      instance.onDragEnd(eventFor({ activeData, overData }));
+
+      expect(instance.moveChoice).toHaveBeenCalledWith('c1', '1', '2', 2);
+    });
+
+    it('routes new choice dropped on category to addChoiceToCategory', () => {
+      const instance = createInstance();
+      const activeData = { type: 'choice', id: '9' };
+      const overData = { type: 'category', id: '2' };
+
+      instance.onDragEnd(eventFor({ activeData, overData }));
+
+      expect(instance.addChoiceToCategory).toHaveBeenCalledWith({ id: '9' }, '2');
+    });
+
+    it('routes choice-preview dropped on category-alternate to moveChoiceInAlternate', () => {
+      const instance = createInstance();
+      const activeData = {
+        type: 'choice-preview',
+        id: 'c1-cat1-0',
+        categoryId: '1',
+        choiceIndex: 1,
+      };
+      const overData = { type: 'category-alternate', id: '2', alternateResponseIndex: 3 };
+
+      instance.onDragEnd(eventFor({ activeData, overData }));
+
+      expect(instance.moveChoiceInAlternate).toHaveBeenCalledWith('c1', '1', '2', 1, 3);
+    });
+
+    it('routes new choice dropped on category-alternate to addChoiceToAlternateCategory', () => {
+      const instance = createInstance({ allowAlternateEnabled: true });
+      const activeData = { type: 'choice', id: '11' };
+      const overData = { type: 'category-alternate', id: '2', alternateResponseIndex: 4 };
+
+      instance.onDragEnd(eventFor({ activeData, overData }));
+
+      expect(instance.addChoiceToAlternateCategory).toHaveBeenCalledWith({ id: '11' }, '2', 4);
+    });
+  });
 });

packages/categorize/configure/src/design/index.jsx

@@ -226,55 +226,69 @@ export class Design extends React.Component {
     });
   };
 
-  onDragEnd = (event) => {
-    const { active, over } = event;
-
+  onDragEnd = ({ active, over }) => {
     this.setState({ activeDragItem: null });
+    if (!active) return;
+
+    const { model } = this.props;
+    const { allowAlternateEnabled, categories = [], choices = [] } = model;
+
+    const activeData = active?.data?.current;
+    const overData = over?.data?.current;
+
+    if (!activeData) return;
 
-    if (!over || !active) {
+    const choiceIndex = activeData.choiceIndex || 0;
+    const overType = overData?.type;
+    const isPreview = activeData.type === 'choice-preview';
+    const isNewChoice = activeData.type === 'choice';
+
+    const choiceId =
+      activeData.choice?.id || (typeof activeData.id === 'string' ? activeData.id.split('-')[0] : activeData.id);
+
+    if (isPreview && (!overData || overType === 'choice')) {
+      this.removeChoiceFromSource(activeData, choiceIndex, { allowAlternateEnabled, categories, choices });
       return;
     }
 
-    const { model } = this.props;
-    const { allowAlternateEnabled } = model;
-    const activeData = active.data.current;
-    const overData = over.data.current;
-
-    // moving a choice between categories (correct response)
-    if (activeData.type === 'choice-preview' && overData.type === 'category') {
-      // Extract original choice.id - if DraggableChoice uses the unique id in data, extract the first part
-      // Format: ${choice.id}-${categoryId}-${choiceIndex} or ${choice.id}-${categoryId}-${choiceIndex}-alt-${alternateResponseIndex}
-      const choiceId =
-        activeData.choice?.id || (typeof activeData.id === 'string' ? activeData.id.split('-')[0] : activeData.id);
-      this.moveChoice(choiceId, activeData.categoryId, overData.id, activeData.choiceIndex || 0);
+    if (isPreview && overType === 'category') {
+      return this.moveChoice(choiceId, activeData.categoryId, overData.id, choiceIndex);
     }
 
-    // placing a choice into a category (correct response)
-    if (activeData.type === 'choice' && overData.type === 'category') {
-      this.addChoiceToCategory({ id: activeData.id }, overData.id);
+    if (isNewChoice && overType === 'category') {
+      return this.addChoiceToCategory({ id: activeData.id }, overData.id);
     }
 
-    // moving a choice between categories (alternate response)
-    if (activeData.type === 'choice-preview' && overData.type === 'category-alternate') {
-      const toAlternateIndex = overData.alternateResponseIndex;
-      // Extract original choice.id - if DraggableChoice uses the unique id in data, extract the first part
-      const choiceId =
-        activeData.choice?.id || (typeof activeData.id === 'string' ? activeData.id.split('-')[0] : activeData.id);
-      this.moveChoiceInAlternate(
+    if (isPreview && overType === 'category-alternate') {
+      return this.moveChoiceInAlternate(
         choiceId,
         activeData.categoryId,
         overData.id,
-        activeData.choiceIndex || 0,
-        toAlternateIndex,
+        choiceIndex,
+        overData.alternateResponseIndex,
       );
     }
 
-    // placing a choice into a category (alternate response)
-    if (allowAlternateEnabled && activeData.type === 'choice' && overData.type === 'category-alternate') {
-      const choiceId = activeData.id;
-      const categoryId = overData.id;
-      const toAlternateResponseIndex = overData.alternateResponseIndex;
-      this.addChoiceToAlternateCategory({ id: choiceId }, categoryId, toAlternateResponseIndex);
+    if (allowAlternateEnabled && isNewChoice && overType === 'category-alternate') {
+      return this.addChoiceToAlternateCategory({ id: activeData.id }, overData.id, overData.alternateResponseIndex);
+    }
+  };
+
+  removeChoiceFromSource = (activeData, choiceIndex, { allowAlternateEnabled, categories, choices }) => {
+    const isAlternateSource = activeData.alternateResponseIndex !== undefined;
+
+    if (!isAlternateSource) {
+      this.deleteChoiceFromCategory(activeData.categoryId, activeData.choiceId, choiceIndex);
+      return;
+    }
+
+    if (!allowAlternateEnabled) return;
+
+    const category = categories?.find((c) => c.id === activeData.categoryId);
+    const choice = choices?.find((c) => c.id === activeData.choiceId);
+
+    if (category && choice) {
+      this.deleteChoiceFromAlternateCategory(category, choice, choiceIndex, activeData.alternateResponseIndex);
     }
   };
 
@@ -299,9 +313,9 @@ export class Design extends React.Component {
     });
   };
 
-  deleteChoiceFromCategory = (category, choice, choiceIndex) => {
+  deleteChoiceFromCategory = (categoryId, choiceId, choiceIndex) => {
     const { model } = this.props;
-    const correctResponse = removeChoiceFromCategory(choice.id, category.id, choiceIndex, model.correctResponse);
+    const correctResponse = removeChoiceFromCategory(choiceId, categoryId, choiceIndex, model.correctResponse);
 
     this.updateModel({ correctResponse });
   };

packages/categorize/src/categorize/__tests__/index.test.jsx

@@ -2,6 +2,7 @@ import React from 'react';
 import { render } from '@testing-library/react';
 import { ThemeProvider, createTheme } from '@mui/material/styles';
 import { Categorize } from '../index';
+import CategorizeProvider from '../index';
 
 jest.mock('@pie-lib/drag', () => ({
   uid: {
@@ -98,7 +99,7 @@ describe('categorize', () => {
     return render(
       <ThemeProvider theme={theme}>
         <Categorize {...props} />
-      </ThemeProvider>
+      </ThemeProvider>,
     );
   };
 
@@ -137,4 +138,102 @@ describe('categorize', () => {
       expect(container).toBeInTheDocument();
     });
   });
+
+  describe('provider onDragEnd', () => {
+    const createProvider = (extras = {}) => {
+      const instance = new CategorizeProvider({
+        ...defaultProps,
+        onAnswersChange: jest.fn(),
+        onShowCorrectToggle: jest.fn(),
+        resumeMathObserver: jest.fn(),
+        ...extras,
+      });
+
+      instance.setState = jest.fn();
+      instance.categorizeRef = {
+        removeChoice: jest.fn(),
+        dropChoice: jest.fn(),
+      };
+
+      return instance;
+    };
+
+    const dndEvent = ({ activeData, overId, overData }) => ({
+      active: activeData ? { data: { current: activeData } } : null,
+      over: overId || overData ? { id: overId, data: { current: overData } } : null,
+    });
+
+    it('removes choice from source when dropped outside valid target', () => {
+      const provider = createProvider();
+      const event = dndEvent({
+        activeData: {
+          id: 'c1',
+          type: 'choice',
+          categoryId: 'cat-1',
+          choiceIndex: 0,
+          value: 'v1',
+          itemType: 'categorize',
+        },
+      });
+
+      provider.onDragEnd(event);
+
+      expect(provider.categorizeRef.removeChoice).toHaveBeenCalledWith(
+        expect.objectContaining({
+          id: 'c1',
+          categoryId: 'cat-1',
+          choiceIndex: 0,
+        }),
+      );
+      expect(provider.categorizeRef.dropChoice).not.toHaveBeenCalled();
+    });
+
+    it('removes choice from source when dropped on choices-board', () => {
+      const provider = createProvider();
+      const event = dndEvent({
+        overId: 'choices-board',
+        overData: { itemType: 'categorize' },
+        activeData: {
+          id: 'c2',
+          type: 'choice',
+          categoryId: 'cat-2',
+          choiceIndex: 1,
+          value: 'v2',
+          itemType: 'categorize',
+        },
+      });
+
+      provider.onDragEnd(event);
+
+      expect(provider.categorizeRef.removeChoice).toHaveBeenCalled();
+      expect(provider.categorizeRef.dropChoice).not.toHaveBeenCalled();
+    });
+
+    it('drops choice into category when valid category target exists', () => {
+      const provider = createProvider();
+      const event = dndEvent({
+        overId: 'cat-target',
+        overData: { itemType: 'categorize' },
+        activeData: {
+          id: 'c3',
+          type: 'choice',
+          categoryId: 'cat-source',
+          choiceIndex: 2,
+          value: 'v3',
+          itemType: 'categorize',
+        },
+      });
+
+      provider.onDragEnd(event);
+
+      expect(provider.categorizeRef.dropChoice).toHaveBeenCalledWith(
+        'cat-target',
+        expect.objectContaining({
+          id: 'c3',
+          categoryId: 'cat-source',
+          choiceIndex: 2,
+        }),
+      );
+    });
+  });
 });