Fix quoted filter string with bug counter

Sascha created 4 years ago

The parse function dropped user given quotes. This resulted into a wrong query
to the backend, which lead to an error. This error prevented the webui from
displaying the proper bug count.

Change summary

webui/src/__tests__/query.ts           | 61 ++++++++++++++++++++-------
webui/src/pages/list/Filter.tsx        | 44 +++++++++----------
webui/src/pages/list/FilterToolbar.tsx | 13 +++++
3 files changed, 76 insertions(+), 42 deletions(-)

Detailed changes

webui/src/tests/query.ts 🔗

@@ -7,49 +7,76 @@ it('parses a simple query', () => {
 });
 
 it('parses a query with multiple filters', () => {
-  expect(parse('foo:bar baz:foo-bar')).toEqual({
+  expect(parse(`foo:bar baz:foo-bar`)).toEqual({
     foo: ['bar'],
     baz: ['foo-bar'],
   });
 });
 
 it('parses a quoted query', () => {
-  expect(parse('foo:"bar"')).toEqual({
-    foo: ['bar'],
+  expect(parse(`foo:"bar"`)).toEqual({
+    foo: [`"bar"`],
   });
 
-  expect(parse("foo:'bar'")).toEqual({
-    foo: ['bar'],
+  expect(parse(`foo:'bar'`)).toEqual({
+    foo: [`'bar'`],
+  });
+
+  expect(parse(`label:abc freetext`)).toEqual({
+    label: [`abc`],
+    freetext: [''],
+  });
+
+  expect(parse(`label:abc with "quotes" in freetext`)).toEqual({
+    label: [`abc`],
+    with: [''],
+    quotes: [''],
+    in: [''],
+    freetext: [''],
+  });
+
+  expect(parse(`label:'multi word label'`)).toEqual({
+    label: [`'multi word label'`],
+  });
+
+  expect(parse(`label:"multi word label"`)).toEqual({
+    label: [`"multi word label"`],
+  });
+
+  expect(parse(`label:'multi word label with "nested" quotes'`)).toEqual({
+    label: [`'multi word label with "nested" quotes'`],
   });
 
-  expect(parse('foo:\'bar "nested" quotes\'')).toEqual({
-    foo: ['bar "nested" quotes'],
+  expect(parse(`label:"multi word label with 'nested' quotes"`)).toEqual({
+    label: [`"multi word label with 'nested' quotes"`],
   });
 
-  expect(parse("foo:'escaped\\' quotes'")).toEqual({
-    foo: ["escaped' quotes"],
+  expect(parse(`foo:'escaped\\' quotes'`)).toEqual({
+    foo: [`'escaped\\' quotes'`],
   });
 });
 
 it('parses a query with repetitions', () => {
-  expect(parse('foo:bar foo:baz')).toEqual({
+  expect(parse(`foo:bar foo:baz`)).toEqual({
     foo: ['bar', 'baz'],
   });
 });
 
 it('parses a complex query', () => {
-  expect(parse('foo:bar foo:baz baz:"foobar" idont:\'know\'')).toEqual({
+  expect(parse(`foo:bar foo:baz baz:"foobar" idont:'know'`)).toEqual({
     foo: ['bar', 'baz'],
-    baz: ['foobar'],
-    idont: ['know'],
+    baz: [`"foobar"`],
+    idont: [`'know'`],
   });
 });
 
 it('quotes values', () => {
-  expect(quote('foo')).toEqual('foo');
-  expect(quote('foo bar')).toEqual('"foo bar"');
-  expect(quote('foo "bar"')).toEqual(`'foo "bar"'`);
-  expect(quote(`foo "bar" 'baz'`)).toEqual(`"foo \\"bar\\" 'baz'"`);
+  expect(quote(`foo`)).toEqual(`foo`);
+  expect(quote(`foo bar`)).toEqual(`"foo bar"`);
+  expect(quote(`foo "bar"`)).toEqual(`"foo "bar""`);
+  expect(quote(`foo 'bar'`)).toEqual(`"foo "bar""`);
+  expect(quote(`'foo'`)).toEqual(`"foo"`);
+  expect(quote(`foo "bar" 'baz'`)).toEqual(`"foo "bar" "baz""`);
 });
 
 it('stringifies params', () => {

webui/src/pages/list/Filter.tsx 🔗

@@ -35,52 +35,50 @@ const ITEM_HEIGHT = 48;
 export type Query = { [key: string]: string[] };
 
 function parse(query: string): Query {
-  // TODO: extract the rest of the query?
   const params: Query = {};
-
-  // TODO: support escaping without quotes
-  const re = /(\w+):([A-Za-z0-9-]+|(["'])(([^\3]|\\.)*)\3)+/g;
+  let re = new RegExp(/(\w+)(:('.*'|".*"|\S*))?/, 'g');
   let matches;
   while ((matches = re.exec(query)) !== null) {
     if (!params[matches[1]]) {
       params[matches[1]] = [];
     }
-
-    let value;
-    if (matches[4]) {
-      value = matches[4];
+    if (matches[3] !== undefined) {
+      params[matches[1]].push(matches[3]);
     } else {
-      value = matches[2];
+      params[matches[1]].push('');
     }
-    value = value.replace(/\\(.)/g, '$1');
-    params[matches[1]].push(value);
   }
   return params;
 }
 
 function quote(value: string): string {
-  const hasSingle = value.includes("'");
-  const hasDouble = value.includes('"');
   const hasSpaces = value.includes(' ');
-  if (!hasSingle && !hasDouble && !hasSpaces) {
-    return value;
-  }
+  const isDoubleQuotedRegEx = RegExp(/^'.*'$/);
+  const isSingleQuotedRegEx = RegExp(/^".*"$/);
+  const isQuoted = () =>
+    isDoubleQuotedRegEx.test(value) || isSingleQuotedRegEx.test(value);
 
-  if (!hasDouble) {
-    return `"${value}"`;
+  //Test if label name contains whitespace between quotes. If no quoates but
+  //whitespace, then quote string.
+  if (!isQuoted() && hasSpaces) {
+    value = `"${value}"`;
   }
 
-  if (!hasSingle) {
-    return `'${value}'`;
+  //Convert single quote (tick) to double quote. This way quoting is always
+  //uniform and can be relied upon by the label menu
+  const hasSingle = value.includes(`'`);
+  if (hasSingle) {
+    value = value.replace(/'/g, `"`);
   }
 
-  value = value.replace(/"/g, '\\"');
-  return `"${value}"`;
+  return value;
 }
 
 function stringify(params: Query): string {
   const parts: string[][] = Object.entries(params).map(([key, values]) => {
-    return values.map((value) => `${key}:${quote(value)}`);
+    return values.map((value) =>
+      value.length > 0 ? `${key}:${quote(value)}` : key
+    );
   });
   return new Array<string>().concat(...parts).join(' ');
 }

webui/src/pages/list/FilterToolbar.tsx 🔗

@@ -56,6 +56,16 @@ function CountingFilter({ query, children, ...props }: CountingFilterProps) {
   );
 }
 
+function quoteLabel(value: string) {
+  const hasUnquotedColon = RegExp(/^[^'"].*:.*[^'"]$/);
+  if (hasUnquotedColon.test(value)) {
+    //quote values which contain a colon but are not quoted.
+    //E.g. abc:abc becomes "abc:abc"
+    return `"${value}"`;
+  }
+  return value;
+}
+
 type Props = {
   query: string;
   queryLocation: (query: string) => LocationDescriptor;
@@ -87,7 +97,7 @@ function FilterToolbar({ query, queryLocation }: Props) {
     labelsData.repository.validLabels.nodes
   ) {
     labels = labelsData.repository.validLabels.nodes.map((node) => [
-      node.name,
+      quoteLabel(node.name),
       node.name,
       node.color,
     ]);
@@ -131,7 +141,6 @@ function FilterToolbar({ query, queryLocation }: Props) {
     [key]: [],
   });
 
-  // TODO: author/label filters
   return (
     <Toolbar className={classes.toolbar}>
       <CountingFilter

Change summary

Detailed changes

webui/src/__tests__/query.ts 🔗

webui/src/pages/list/Filter.tsx 🔗

webui/src/pages/list/FilterToolbar.tsx 🔗

webui/src/tests/query.ts 🔗