Browse Source

Fix: Fixes stripping of $d in Explore urls (#18480)

Fixes: #18455
Hugo Häggmark 6 years ago
parent
commit
445f1dabcc

+ 0 - 72
public/app/features/explore/PlaceholdersBuffer.test.ts

@@ -1,72 +0,0 @@
-import PlaceholdersBuffer from './PlaceholdersBuffer';
-
-describe('PlaceholdersBuffer', () => {
-  it('does nothing if no placeholders are defined', () => {
-    const text = 'metric';
-    const buffer = new PlaceholdersBuffer(text);
-
-    expect(buffer.hasPlaceholders()).toBe(false);
-    expect(buffer.toString()).toBe(text);
-    expect(buffer.getNextMoveOffset()).toBe(0);
-  });
-
-  it('respects the traversal order of placeholders', () => {
-    const text = 'sum($2 offset $1) by ($3)';
-    const buffer = new PlaceholdersBuffer(text);
-
-    expect(buffer.hasPlaceholders()).toBe(true);
-    expect(buffer.toString()).toBe('sum( offset ) by ()');
-    expect(buffer.getNextMoveOffset()).toBe(12);
-
-    buffer.setNextPlaceholderValue('1h');
-
-    expect(buffer.hasPlaceholders()).toBe(true);
-    expect(buffer.toString()).toBe('sum( offset 1h) by ()');
-    expect(buffer.getNextMoveOffset()).toBe(-10);
-
-    buffer.setNextPlaceholderValue('metric');
-
-    expect(buffer.hasPlaceholders()).toBe(true);
-    expect(buffer.toString()).toBe('sum(metric offset 1h) by ()');
-    expect(buffer.getNextMoveOffset()).toBe(16);
-
-    buffer.setNextPlaceholderValue('label');
-
-    expect(buffer.hasPlaceholders()).toBe(false);
-    expect(buffer.toString()).toBe('sum(metric offset 1h) by (label)');
-    expect(buffer.getNextMoveOffset()).toBe(0);
-  });
-
-  it('respects the traversal order of adjacent placeholders', () => {
-    const text = '$1$3$2$4';
-    const buffer = new PlaceholdersBuffer(text);
-
-    expect(buffer.hasPlaceholders()).toBe(true);
-    expect(buffer.toString()).toBe('');
-    expect(buffer.getNextMoveOffset()).toBe(0);
-
-    buffer.setNextPlaceholderValue('1');
-
-    expect(buffer.hasPlaceholders()).toBe(true);
-    expect(buffer.toString()).toBe('1');
-    expect(buffer.getNextMoveOffset()).toBe(0);
-
-    buffer.setNextPlaceholderValue('2');
-
-    expect(buffer.hasPlaceholders()).toBe(true);
-    expect(buffer.toString()).toBe('12');
-    expect(buffer.getNextMoveOffset()).toBe(-1);
-
-    buffer.setNextPlaceholderValue('3');
-
-    expect(buffer.hasPlaceholders()).toBe(true);
-    expect(buffer.toString()).toBe('132');
-    expect(buffer.getNextMoveOffset()).toBe(1);
-
-    buffer.setNextPlaceholderValue('4');
-
-    expect(buffer.hasPlaceholders()).toBe(false);
-    expect(buffer.toString()).toBe('1324');
-    expect(buffer.getNextMoveOffset()).toBe(0);
-  });
-});

+ 0 - 112
public/app/features/explore/PlaceholdersBuffer.ts

@@ -1,112 +0,0 @@
-/**
- * Provides a stateful means of managing placeholders in text.
- *
- * Placeholders are numbers prefixed with the `$` character (e.g. `$1`).
- * Each number value represents the order in which a placeholder should
- * receive focus if multiple placeholders exist.
- *
- * Example scenario given `sum($3 offset $1) by($2)`:
- * 1. `sum( offset |) by()`
- * 2. `sum( offset 1h) by(|)`
- * 3. `sum(| offset 1h) by (label)`
- */
-export default class PlaceholdersBuffer {
-  private nextMoveOffset: number;
-  private orders: number[];
-  private parts: string[];
-
-  constructor(text: string) {
-    const result = this.parse(text);
-    const nextPlaceholderIndex = result.orders.length ? result.orders[0] : 0;
-    this.nextMoveOffset = this.getOffsetBetween(result.parts, 0, nextPlaceholderIndex);
-    this.orders = result.orders;
-    this.parts = result.parts;
-  }
-
-  clearPlaceholders() {
-    this.nextMoveOffset = 0;
-    this.orders = [];
-  }
-
-  getNextMoveOffset(): number {
-    return this.nextMoveOffset;
-  }
-
-  hasPlaceholders(): boolean {
-    return this.orders.length > 0;
-  }
-
-  setNextPlaceholderValue(value: string) {
-    if (this.orders.length === 0) {
-      return;
-    }
-    const currentPlaceholderIndex = this.orders[0];
-    this.parts[currentPlaceholderIndex] = value;
-    this.orders = this.orders.slice(1);
-    if (this.orders.length === 0) {
-      this.nextMoveOffset = 0;
-      return;
-    }
-    const nextPlaceholderIndex = this.orders[0];
-    // Case should never happen but handle it gracefully in case
-    if (currentPlaceholderIndex === nextPlaceholderIndex) {
-      this.nextMoveOffset = 0;
-      return;
-    }
-    const backwardMove = currentPlaceholderIndex > nextPlaceholderIndex;
-    const indices = backwardMove
-      ? { start: nextPlaceholderIndex + 1, end: currentPlaceholderIndex + 1 }
-      : { start: currentPlaceholderIndex + 1, end: nextPlaceholderIndex };
-    this.nextMoveOffset = (backwardMove ? -1 : 1) * this.getOffsetBetween(this.parts, indices.start, indices.end);
-  }
-
-  toString(): string {
-    return this.parts.join('');
-  }
-
-  private getOffsetBetween(parts: string[], startIndex: number, endIndex: number) {
-    return parts.slice(startIndex, endIndex).reduce((offset, part) => offset + part.length, 0);
-  }
-
-  private parse(text: string): ParseResult {
-    const placeholderRegExp = /\$(\d+)/g;
-    const parts = [];
-    const orders = [];
-    let textOffset = 0;
-    while (true) {
-      const match = placeholderRegExp.exec(text);
-      if (!match) {
-        break;
-      }
-      const part = text.slice(textOffset, match.index);
-      parts.push(part);
-      // Accounts for placeholders at text boundaries
-      if (part !== '') {
-        parts.push('');
-      }
-      const order = parseInt(match[1], 10);
-      orders.push({ index: parts.length - 1, order });
-      textOffset += part.length + match.length;
-    }
-    // Ensures string serialization still works if no placeholders were parsed
-    // and also accounts for the remainder of text with placeholders
-    parts.push(text.slice(textOffset));
-    return {
-      // Placeholder values do not necessarily appear sequentially so sort the
-      // indices to traverse in priority order
-      orders: orders.sort((o1, o2) => o1.order - o2.order).map(o => o.index),
-      parts,
-    };
-  }
-}
-
-type ParseResult = {
-  /**
-   * Indices to placeholder items in `parts` in traversal order.
-   */
-  orders: number[];
-  /**
-   * Parts comprising the original text with placeholders occupying distinct items.
-   */
-  parts: string[];
-};

+ 4 - 16
public/app/features/explore/QueryField.tsx

@@ -16,7 +16,6 @@ import NewlinePlugin from './slate-plugins/newline';
 
 import { TypeaheadWithTheme } from './Typeahead';
 import { makeFragment, makeValue } from '@grafana/ui';
-import PlaceholdersBuffer from './PlaceholdersBuffer';
 
 export const TYPEAHEAD_DEBOUNCE = 100;
 export const HIGHLIGHT_WAIT = 500;
@@ -74,7 +73,6 @@ export interface TypeaheadInput {
  */
 export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldState> {
   menuEl: HTMLElement | null;
-  placeholdersBuffer: PlaceholdersBuffer;
   plugins: any[];
   resetTimer: any;
   mounted: boolean;
@@ -83,7 +81,6 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
   constructor(props: QueryFieldProps, context: Context<any>) {
     super(props, context);
 
-    this.placeholdersBuffer = new PlaceholdersBuffer(props.initialQuery || '');
     this.updateHighlightsTimer = _.debounce(this.updateLogsHighlights, HIGHLIGHT_WAIT);
 
     // Base plugins
@@ -95,7 +92,7 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
       typeaheadIndex: 0,
       typeaheadPrefix: '',
       typeaheadText: '',
-      value: makeValue(this.placeholdersBuffer.toString(), props.syntax),
+      value: makeValue(props.initialQuery, props.syntax),
       lastExecutedValue: null,
     };
   }
@@ -118,8 +115,7 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
     if (initialQuery !== prevProps.initialQuery) {
       // and we have a version that differs
       if (initialQuery !== Plain.serialize(value)) {
-        this.placeholdersBuffer = new PlaceholdersBuffer(initialQuery || '');
-        this.setState({ value: makeValue(this.placeholdersBuffer.toString(), syntax) });
+        this.setState({ value: makeValue(initialQuery, syntax) });
       }
     }
 
@@ -136,9 +132,7 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
         .change()
         .insertText(' ')
         .deleteBackward();
-      if (this.placeholdersBuffer.hasPlaceholders()) {
-        change.move(this.placeholdersBuffer.getNextMoveOffset()).focus();
-      }
+
       this.onChange(change, true);
     }
   }
@@ -325,11 +319,7 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
 
       const insertTextOperation = nextChange.operations.find((operation: any) => operation.type === 'insert_text');
       if (insertTextOperation) {
-        const suggestionText = insertTextOperation.text;
-        this.placeholdersBuffer.setNextPlaceholderValue(suggestionText);
-        if (this.placeholdersBuffer.hasPlaceholders()) {
-          nextChange.move(this.placeholdersBuffer.getNextMoveOffset()).focus();
-        }
+        return undefined;
       }
 
       return true;
@@ -415,8 +405,6 @@ export class QueryField extends React.PureComponent<QueryFieldProps, QueryFieldS
     // If we dont wait here, menu clicks wont work because the menu
     // will be gone.
     this.resetTimer = setTimeout(this.resetTypeahead, 100);
-    // Disrupting placeholder entry wipes all remaining placeholders needing input
-    this.placeholdersBuffer.clearPlaceholders();
 
     if (previousValue !== currentValue) {
       this.executeOnChangeAndRunQueries();