Make query input editable, remove modal

5 files changed, 294 insertions, 150 deletions

diff --git a/client/js/app/src/app/pages/querybuilder/context/__test__/query-builder-provider.test.jsx b/client/js/app/src/app/pages/querybuilder/context/__test__/query-builder-provider.test.jsx
new file mode 100644
index 00000000000..d37347a5580
--- /dev/null
+++ b/client/js/app/src/app/pages/querybuilder/context/__test__/query-builder-provider.test.jsx
@@ -0,0 +1,190 @@
+import {
+  ACTION,
+  reducer,
+} from 'app/pages/querybuilder/context/query-builder-provider';
+import parameters from 'app/pages/querybuilder/context/parameters';
+import { cloneDeep, omitBy } from 'lodash';
+
+const state = reducer();
+
+test('default state', () => {
+  const fixed = { ...state, params: hideTypes(state.params) };
+  expect(fixed).toEqual({
+    http: {},
+    params: {
+      children: [{ id: '0', input: '', type: 'yql' }],
+    },
+    query: {
+      input: '{\n    "yql": ""\n}',
+    },
+    request: {
+      body: '{\n    "yql": ""\n}',
+      fullUrl: 'http://localhost:8080/search/',
+      method: 'POST',
+      url: 'http://localhost:8080/search/',
+    },
+  });
+});
+
+test('manipulates inputs', () => {
+  function assert(state, queryJson, querySearchParams, params) {
+    expect(hideTypes(state.params).children).toEqual(params);
+    expect({ ...state.query, input: JSON.parse(state.query.input) }).toEqual(
+      queryJson
+    );
+
+    const spState = reducer(state, { action: ACTION.SET_METHOD, data: 'GET' });
+    expect(hideTypes(spState.params).children).toEqual(params);
+    expect(spState.query).toEqual(querySearchParams);
+  }
+
+  const s1 = reduce(state, [[ACTION.INPUT_ADD, { type: 'hits' }]]);
+  assert(s1, { input: { yql: '', hits: null } }, { input: 'yql=&hits=' }, [
+    { id: '0', input: '', type: 'yql' },
+    { id: '1', input: '', type: 'hits' },
+  ]);
+
+  const s2 = reduce(s1, [
+    [ACTION.INPUT_UPDATE, { id: '1', input: '12' }],
+    [ACTION.INPUT_ADD, { type: 'ranking' }],
+    [ACTION.INPUT_UPDATE, { id: '1', type: 'offset' }],
+    [ACTION.INPUT_REMOVE, '0'],
+    [ACTION.INPUT_ADD, { id: '2', type: 'location' }],
+    [ACTION.INPUT_ADD, { id: '2', type: 'matchPhase' }],
+    [ACTION.INPUT_UPDATE, { id: '2.0', input: 'us' }],
+  ]);
+  assert(
+    s2,
+    { input: { offset: 12, ranking: { location: 'us', matchPhase: {} } } },
+    { input: 'offset=12&ranking.location=us' },
+    [
+      { id: '1', input: '12', type: 'offset' },
+      {
+        id: '2',
+        input: '',
+        type: 'ranking',
+        children: [
+          { id: '2.0', input: 'us', type: 'location' },
+          { id: '2.1', input: '', type: 'matchPhase', children: [] },
+        ],
+      },
+    ]
+  );
+
+  assert(
+    reduce(s2, [[ACTION.INPUT_UPDATE, { id: '2', type: 'noCache' }]]),
+    { input: { offset: 12, noCache: false } },
+    { input: 'offset=12&noCache=' },
+    [
+      { id: '1', input: '12', type: 'offset' },
+      { id: '2', input: '', type: 'noCache' },
+    ]
+  );
+
+  assert(
+    reduce(s2, [[ACTION.INPUT_REMOVE, '2']]),
+    { input: { offset: 12 } },
+    { input: 'offset=12' },
+    [{ id: '1', input: '12', type: 'offset' }]
+  );
+});
+
+test('set query', () => {
+  const query = (method, input) =>
+    reduce(state, [
+      [ACTION.INPUT_REMOVE, '0'],
+      [ACTION.SET_METHOD, method],
+      [ACTION.SET_QUERY, input],
+    ]);
+  function assert(inputJson, inputSearchParams, params) {
+    const s2 = query('POST', inputJson);
+    expect(hideTypes(s2.params).children).toEqual(params);
+    expect(s2.query.input).toEqual(inputJson);
+    expect(s2.query.error).toBeUndefined();
+
+    if (inputSearchParams == null) return;
+    const s3 = query('GET', inputSearchParams);
+    expect(hideTypes(s3.params).children).toEqual(params);
+    expect(s3.query.input).toEqual(inputSearchParams);
+    expect(s3.query.error).toBeUndefined();
+  }
+
+  function error(method, input, error) {
+    const s = query(method, input);
+    expect(s.params.children).toEqual([]);
+    expect(s.query.input).toEqual(input);
+    expect(s.query.error).toEqual(error);
+  }
+
+  assert('{"yql":"abc"}', '?yql=abc', [{ id: '0', input: 'abc', type: 'yql' }]);
+
+  assert(
+    '{"hits":12,"ranking":{"location":"us","matchPhase":{"attribute":"[\\"a b\\"]"}},"noCache":true,"offset":""}',
+    'hits=12&ranking.location=us&noCache=true&ranking.matchPhase.attribute=%5B%22a+b%22%5D&offset',
+    [
+      { id: '0', input: '12', type: 'hits' },
+      {
+        id: '1',
+        input: '',
+        type: 'ranking',
+        children: [
+          { id: '1.0', input: 'us', type: 'location' },
+          {
+            id: '1.1',
+            input: '',
+            type: 'matchPhase',
+            children: [{ id: '1.1.0', input: '["a b"]', type: 'attribute' }],
+          },
+        ],
+      },
+      { id: '2', input: 'true', type: 'noCache' },
+      { id: '3', input: '', type: 'offset' },
+    ]
+  );
+
+  assert('{"ranking":{"matchPhase":{}}}', null, [
+    {
+      id: '0',
+      input: '',
+      type: 'ranking',
+      children: [
+        {
+          id: '0.0',
+          input: '',
+          type: 'matchPhase',
+          children: [],
+        },
+      ],
+    },
+  ]);
+
+  let msg = "Unknown property 'asd' on root level";
+  error('POST', '{"asd":123}', msg);
+  error('GET', 'asd=123', msg);
+
+  msg = "Unknown property 'asd' under 'matchPhase'";
+  error('POST', '{"ranking":{"matchPhase":{"asd":123}}}', msg);
+  error('GET', 'ranking.matchPhase.asd=123', msg);
+
+  error('POST', '{"yql":"test}', 'Unexpected end of JSON input');
+
+  msg =
+    "Property 'ranking' cannot have value, supported children: features,freshness,listFeatures,location,matchPhase,profile,properties,queryCache,sorting";
+  error('POST', '{"ranking":123}', msg);
+  error('GET', 'ranking=123', msg);
+
+  error('POST', '{"yql":{}}', "Expected property 'yql' to be String");
+});
+
+function hideTypes({ type, children, ...copy }) {
+  if (type.name) copy.type = type.name;
+  if (children) copy.children = children.map(hideTypes);
+  return copy;
+}
+
+function reduce(state, operations) {
+  return operations.reduce(
+    (acc, [action, data]) => reducer(acc, { action, data }),
+    state
+  );
+}
diff --git a/client/js/app/src/app/pages/querybuilder/context/query-builder-provider.jsx b/client/js/app/src/app/pages/querybuilder/context/query-builder-provider.jsx
index 98a040159da..1b1faa39e59 100644
--- a/client/js/app/src/app/pages/querybuilder/context/query-builder-provider.jsx
+++ b/client/js/app/src/app/pages/querybuilder/context/query-builder-provider.jsx
@@ -1,4 +1,4 @@
-import { cloneDeep, last } from 'lodash';
+import { last, set } from 'lodash';
 import React, { useReducer } from 'react';
 import { createContext, useContextSelector } from 'use-context-selector';
 import parameters from 'app/pages/querybuilder/context/parameters';
@@ -18,6 +18,11 @@ export const ACTION = Object.freeze({
   INPUT_REMOVE: 12,
 });
 
+function cloneParams(params) {
+  if (!params.children) return { ...params };
+  return { ...params, children: params.children.map(cloneParams) };
+}
+
 function inputsToSearchParams(inputs, parent) {
   return inputs.reduce((acc, { input, type: { name }, children }) => {
     const key = parent ? `${parent}.${name}` : name;
@@ -28,6 +33,14 @@ function inputsToSearchParams(inputs, parent) {
   }, {});
 }
 
+function searchParamsToInputs(search) {
+  const json = [...new URLSearchParams(search).entries()].reduce(
+    (acc, [key, value]) => set(acc, key, value),
+    {}
+  );
+  return jsonToInputs(json);
+}
+
 function inputsToJson(inputs) {
   return Object.fromEntries(
     inputs.map(({ children, input, type: { name, type } }) => [
@@ -37,23 +50,32 @@ function inputsToJson(inputs) {
   );
 }
 
-export function jsonToInputs(json, parent = root) {
+function jsonToInputs(json, parent = root) {
   return Object.entries(json).map(([key, value], i) => {
     const node = {
       id: parent.id ? `${parent.id}.${i}` : i.toString(),
       type: parent.type.children[key],
-      parent,
     };
     if (!node.type) {
       const location = parent.type.name
-        ? `under ${parent.type.name}`
+        ? `under '${parent.type.name}'`
         : 'on root level';
       throw new Error(`Unknown property '${key}' ${location}`);
     }
-    if (typeof value === 'object') {
+    if (value != null && typeof value === 'object') {
+      if (!node.type.children)
+        throw new Error(`Expected property '${key}' to be ${node.type.type}`);
       node.input = '';
       node.children = jsonToInputs(value, node);
-    } else node.input = value.toString();
+    } else {
+      if (node.type.children)
+        throw new Error(
+          `Property '${key}' cannot have value, supported children: ${Object.keys(
+            node.type.children
+          ).sort()}`
+        );
+      node.input = value?.toString();
+    }
     return node;
   });
 }
@@ -66,8 +88,8 @@ function parseInput(input, type) {
 }
 
 function inputAdd(params, { id: parentId, type: typeName }) {
-  const inputs = cloneDeep(params.children);
-  const parent = parentId ? findInput(inputs, parentId) : params;
+  const cloned = cloneParams(params);
+  const parent = findInput(cloned, parentId);
 
   const nextId =
     parseInt(last(last(parent.children)?.id?.split('.')) ?? -1) + 1;
@@ -75,58 +97,73 @@ function inputAdd(params, { id: parentId, type: typeName }) {
   const type = parent.type.children[typeName];
 
   parent.children.push(
-    Object.assign(
-      { id, input: '', type, parent },
-      type.children && { children: [] }
-    )
+    Object.assign({ id, input: '', type }, type.children && { children: [] })
   );
-  return { ...params, children: inputs };
+
+  return cloned;
 }
 
-function inputUpdate(params, { id, ...props }) {
-  const keys = Object.keys(props);
-  if (keys.length !== 1)
-    throw new Error(`Expected to update exactly 1 input prop, got: ${keys}`);
-  if (!['input', 'type'].includes(keys[0]))
-    throw new Error(`Cannot update key ${keys[0]}`);
+function inputUpdate(params, { id, type, input }) {
+  const cloned = cloneParams(params);
+  const node = findInput(cloned, id);
+  if (type) {
+    const parent = findInput(cloned, id.substring(0, id.lastIndexOf('.')));
+    node.type = parent.type.children[type];
+  }
+  if (input) node.input = input;
 
-  const inputs = cloneDeep(params.children);
-  const node = Object.assign(findInput(inputs, id), props);
   if (node.type.children) node.children = [];
   else delete node.children;
-  return { ...params, children: inputs };
+  return cloned;
 }
 
-function findInput(inputs, id, Delete = false) {
+function findInput(params, id, Delete = false) {
+  if (!id) return params;
   let end = -1;
   while ((end = id.indexOf('.', end + 1)) > 0)
-    inputs = inputs.find((input) => input.id === id.substring(0, end)).children;
-  const index = inputs.findIndex((input) => input.id === id);
-  return Delete ? inputs.splice(index, 1)[0] : inputs[index];
+    params = params.children.find((input) => input.id === id.substring(0, end));
+  const index = params.children.findIndex((input) => input.id === id);
+  return Delete ? params.children.splice(index, 1)[0] : params.children[index];
 }
 
-function reducer(state, action) {
-  const result = preReducer(state, action);
-  const { request: sr, params: sp } = state;
-  const { request: rr, params: rp } = result;
-  if (sp.children !== rp.children || sr.method !== rr.method) {
-    result.query =
-      rr.method === 'POST'
-        ? JSON.stringify(inputsToJson(rp.children), null, 4)
-        : new URLSearchParams(inputsToSearchParams(rp.children)).toString();
+export function reducer(state, action) {
+  if (state == null) {
+    state = { http: {}, params: {}, query: {}, request: {} };
+
+    return [
+      [ACTION.SET_URL, 'http://localhost:8080/search/'],
+      [ACTION.SET_QUERY, 'yql='],
+      [ACTION.SET_METHOD, 'POST'],
+    ].reduce((s, [action, data]) => reducer(s, { action, data }), state);
   }
 
-  if (sr.url !== rr.url || state.query !== result.query) {
+  const result = preReducer(state, action);
+  const { request: sr, params: sp, query: sq } = state;
+  const { request: rr, params: rp, query: rq } = result;
+
+  if ((sp.children !== rp.children && sq === rq) || sr.method !== rr.method)
+    result.query = {
+      input:
+        rr.method === 'POST'
+          ? JSON.stringify(inputsToJson(rp.children), null, 4)
+          : new URLSearchParams(inputsToSearchParams(rp.children)).toString(),
+    };
+
+  const input = result.query.input;
+  if (sr.url !== rr.url || sq.input !== input || sr.method !== rr.method) {
     if (rr.method === 'POST') {
-      rr.fullUrl = rr.url;
-      rr.body = result.query;
+      result.request = { ...result.request, fullUrl: rr.url, body: input };
     } else {
       const url = new URL(rr.url);
-      url.search = result.query;
-      rr.fullUrl = url.toString();
-      rr.body = null;
+      url.search = input;
+      result.request = {
+        ...result.request,
+        fullUrl: url.toString(),
+        body: null,
+      };
     }
   }
+
   return result;
 }
 
@@ -134,10 +171,17 @@ function preReducer(state, { action, data }) {
   switch (action) {
     case ACTION.SET_QUERY: {
       try {
-        const children = jsonToInputs(JSON.parse(data));
-        return { ...state, params: { ...root, children } };
+        const children =
+          state.request.method === 'POST'
+            ? jsonToInputs(JSON.parse(data))
+            : searchParamsToInputs(data);
+        return {
+          ...state,
+          params: { ...root, children },
+          query: { input: data },
+        };
       } catch (error) {
-        return state;
+        return { ...state, query: { input: data, error: error.message } };
       }
     }
     case ACTION.SET_HTTP:
@@ -152,9 +196,9 @@ function preReducer(state, { action, data }) {
     case ACTION.INPUT_UPDATE:
       return { ...state, params: inputUpdate(state.params, data) };
     case ACTION.INPUT_REMOVE: {
-      const inputs = cloneDeep(state.params.children);
-      findInput(inputs, data, true);
-      return { ...state, params: { ...state.params, children: inputs } };
+      const cloned = cloneParams(state.params);
+      findInput(cloned, data, true);
+      return { ...state, params: cloned };
     }
 
     default:
@@ -163,15 +207,7 @@ function preReducer(state, { action, data }) {
 }
 
 export function QueryBuilderProvider({ children }) {
-  const [value, dispatch] = useReducer(
-    reducer,
-    {
-      request: { url: 'http://localhost:8080/search/', method: 'POST' },
-      http: {},
-      params: { ...root, children: [] },
-    },
-    (s) => reducer(s, { action: ACTION.SET_QUERY, data: '{"yql":""}' })
-  );
+  const [value, dispatch] = useReducer(reducer, null, reducer);
   _dispatch = dispatch;
   return <context.Provider value={value}>{children}</context.Provider>;
 }
diff --git a/client/js/app/src/app/pages/querybuilder/query-derived/paste-modal.jsx b/client/js/app/src/app/pages/querybuilder/query-derived/paste-modal.jsx
deleted file mode 100644
index 90fe6a51eb6..00000000000
--- a/client/js/app/src/app/pages/querybuilder/query-derived/paste-modal.jsx
+++ /dev/null
@@ -1,81 +0,0 @@
-import { Button, Modal, Stack, Textarea } from '@mantine/core';
-import React, { useState } from 'react';
-import { Icon } from 'app/components';
-import {
-  ACTION,
-  dispatch,
-  jsonToInputs,
-} from 'app/pages/querybuilder/context/query-builder-provider';
-
-function PasteForm({ close }) {
-  const [query, setQuery] = useState('');
-  const [error, setError] = useState('');
-
-  function onSubmit() {
-    try {
-      const json = JSON.parse(query);
-      try {
-        jsonToInputs(json);
-        dispatch(ACTION.SET_QUERY, query);
-        close();
-      } catch (error) {
-        setError(`Invalid Vespa query: ${error.message}`);
-      }
-    } catch (error) {
-      setError(`Invalid JSON: ${error.message}`);
-    }
-  }
-
-  return (
-    <Stack>
-      <Textarea
-        styles={(theme) => ({
-          input: {
-            fontFamily: theme.fontFamilyMonospace,
-            fontSize: theme.fontSizes.xs,
-          },
-        })}
-        placeholder="Your Vespa query JSON"
-        error={error}
-        minRows={21}
-        value={query}
-        onChange={({ target }) => setQuery(target.value)}
-        autosize
-      />
-      <Button leftIcon={<Icon name="paste" />} onClick={onSubmit}>
-        Save
-      </Button>
-    </Stack>
-  );
-}
-
-export function PasteModal() {
-  const [opened, setOpened] = useState(false);
-  const close = () => setOpened(false);
-
-  return (
-    <>
-      <Modal
-        opened={opened}
-        onClose={close}
-        title="Vespa Query"
-        overlayColor="white"
-        shadow="1px 3px 10px 2px rgb(0 0 0 / 20%)"
-        padding="xl"
-        size="xl"
-        centered
-      >
-        <PasteForm close={close} />
-      </Modal>
-      <Button
-        onClick={() => setOpened(true)}
-        leftIcon={<Icon name="paste" />}
-        variant="outline"
-        size="xs"
-        compact
-      >
-        Paste JSON
-      </Button>
-    </>
-  );
-}
diff --git a/client/js/app/src/app/pages/querybuilder/query-derived/query-derived.jsx b/client/js/app/src/app/pages/querybuilder/query-derived/query-derived.jsx
index e4626d194c5..fca06defc5d 100644
--- a/client/js/app/src/app/pages/querybuilder/query-derived/query-derived.jsx
+++ b/client/js/app/src/app/pages/querybuilder/query-derived/query-derived.jsx
@@ -7,19 +7,22 @@ import {
   CopyButton,
   Textarea,
 } from '@mantine/core';
-import { useQueryBuilderContext } from 'app/pages/querybuilder/context/query-builder-provider';
+import {
+  ACTION,
+  dispatch,
+  useQueryBuilderContext,
+} from 'app/pages/querybuilder/context/query-builder-provider';
 import { Icon } from 'app/components';
-import { PasteModal } from 'app/pages/querybuilder/query-derived/paste-modal';
 
 export function QueryDerived() {
-  const query = useQueryBuilderContext('query');
+  const { input, error } = useQueryBuilderContext('query');
 
   return (
     <Stack>
       <Group position="apart">
         <Badge variant="filled">Query</Badge>
         <Group spacing="xs">
-          <CopyButton value={query}>
+          <CopyButton value={input}>
             {({ copied, copy }) => (
               <Button
                 leftIcon={<Icon name={copied ? 'check' : 'copy'} />}
@@ -33,7 +36,6 @@ export function QueryDerived() {
               </Button>
             )}
           </CopyButton>
-          <PasteModal />
         </Group>
       </Group>
       <Textarea
@@ -43,7 +45,9 @@ export function QueryDerived() {
             fontSize: theme.fontSizes.xs,
           },
         })}
-        value={query}
+        value={input}
+        error={error}
+        onChange={({ target }) => dispatch(ACTION.SET_QUERY, target.value)}
         variant="unstyled"
         minRows={21}
         autosize
diff --git a/client/js/app/src/app/pages/querybuilder/query-filters/query-filters.jsx b/client/js/app/src/app/pages/querybuilder/query-filters/query-filters.jsx
index 2f59b0af86e..82e9fc6c8c1 100644
--- a/client/js/app/src/app/pages/querybuilder/query-filters/query-filters.jsx
+++ b/client/js/app/src/app/pages/querybuilder/query-filters/query-filters.jsx
@@ -33,12 +33,7 @@ function Input({ id, input, types, type, children }) {
         <Select
           sx={{ flex: 1 }}
           data={Object.values(options).map(({ name }) => name)}
-          onChange={(value) =>
-            dispatch(ACTION.INPUT_UPDATE, {
-              id,
-              type: types[value],
-            })
-          }
+          onChange={(type) => dispatch(ACTION.INPUT_UPDATE, { id, type })}
           value={type.name}
           searchable
         />
@@ -52,7 +47,7 @@ function Input({ id, input, types, type, children }) {
               })
             }
             placeholder={type.type}
-            value={input}
+            value={input ?? ''}
           />
         )}
         <ActionIcon onClick={() => dispatch(ACTION.INPUT_REMOVE, id)}>