Fix nested query duplicate (#249)

malloydata · Nov 21, 2024 · 5b7df62 · 5b7df62
1 parent ce7ce74
commit 5b7df62
Show file tree

Hide file tree

Showing 4 changed files with 72 additions and 4 deletions.
diff --git a/packages/query-composer/example/example.tsx b/packages/query-composer/example/example.tsx
@@ -59,7 +59,6 @@ const App = () => {
         <div
           style={{
             backgroundColor: 'var(--malloy-composer-background)',
-            padding: 8,
           }}
         >
           <ExploreQueryEditor

diff --git a/packages/query-composer/src/components/NestQueryActionMenu/NestQueryActionMenu.tsx b/packages/query-composer/src/components/NestQueryActionMenu/NestQueryActionMenu.tsx
@@ -246,8 +246,13 @@ export const NestQueryActionMenu: React.FC<NestQueryActionMenuProps> = ({
           iconName: 'duplicate',
           iconColor: 'other',
           isEnabled: !isExpanded,
-          onClick: () =>
-            queryModifiers.replaceWithDefinition(stagePath, fieldIndex),
+          onClick: () => {
+            const {stageIndex, parts: oldParts} = stagePath;
+            const parts = oldParts ? [...oldParts] : [];
+            parts.pop();
+            const parentPath = {stageIndex, parts};
+            queryModifiers.replaceWithDefinition(parentPath, fieldIndex);
+          },
         },
         {
           kind: 'one_click',

diff --git a/packages/query-composer/src/components/QuerySummaryPanel/StateSummaryUI.tsx b/packages/query-composer/src/components/QuerySummaryPanel/StateSummaryUI.tsx
@@ -66,7 +66,7 @@ export const StageSummaryUI: React.FC<SummaryStageProps> = ({
     };
     window.addEventListener('keyup', handle);
     return () => window.removeEventListener('keyup', handle);
-  });
+  }, [currentFieldOrdering, queryModifiers, selectedFieldIndex, stagePath]);
 
   return (
     <FieldListDiv>

diff --git a/packages/query-composer/tests/core/query.spec.ts b/packages/query-composer/tests/core/query.spec.ts
@@ -182,6 +182,27 @@ run: names -> {
   group_by: 
     name
     state
+}`);
+    });
+
+    it('preserves order_by', () => {
+      qb.addOrderBy({stageIndex: 0}, 1);
+      expect(qb.getQueryStringForNotebook()).toEqual(`\
+run: names -> {
+  group_by: 
+    name
+    state
+  aggregate: population
+  order_by: state
+}`);
+      qb.reorderFields({stageIndex: 0}, [2, 0, 1]);
+      expect(qb.getQueryStringForNotebook()).toEqual(`\
+run: names -> {
+  aggregate: population
+  group_by: 
+    name
+    state
+  order_by: state
 }`);
     });
   });
@@ -477,6 +498,49 @@ run: names -> {
     });
   });
 
+  describe('expanding stages', () => {
+    beforeEach(() => {
+      qb.addField({stageIndex: 0}, 'by_state');
+    });
+
+    it('auto expands when applying new properties', () => {
+      expect(qb.getQueryStringForNotebook()).toEqual(`\
+run: names -> {
+  nest: by_state
+}`);
+      qb.addLimit({stageIndex: 0, parts: [{stageIndex: 0, fieldIndex: 0}]}, 13);
+      expect(qb.getQueryStringForNotebook()).toEqual(`\
+run: names -> {
+  nest: by_state is {
+    group_by: state
+    aggregate: births_per_100k
+    limit: 13
+  }
+}`);
+    });
+
+    it('explicitly expands a stage', () => {
+      expect(qb.getQueryStringForNotebook()).toEqual(`\
+run: names -> {
+  nest: by_state
+}`);
+      qb.replaceWithDefinition(
+        {
+          stageIndex: 0,
+          parts: [],
+        },
+        0
+      );
+      expect(qb.getQueryStringForNotebook()).toEqual(`\
+run: names -> {
+  nest: by_state is {
+    group_by: state
+    aggregate: births_per_100k
+  }
+}`);
+    });
+  });
+
   describe('updateSource', () => {
     it('updates the source', () => {
       expect(qb.getSource()?.as).toEqual('names');