From 6b1b52b704eeb58f23a10b4c512a6aebfe5b99b4 Mon Sep 17 00:00:00 2001
From: Lucas Raschek <17753678+LucasRaschek@users.noreply.github.com>
Date: Thu, 13 Aug 2020 06:52:32 +0200
Subject: [PATCH] Templating: Fixes renaming a variable using special
 characters or same name (#26866)

* Fix variable editor name-input bug

You couldn't delete an invalid character after typing it into the name-input field.
While investigating the issue turned out to be bigger, as there was a problem with
valid characters too. (See test scenarios below)

The fix seems to be, to remove an unnecessary check in the `changeVariableName`
action. There is theoretically now the possibility, that the `changeVariableName`
action is called with the same name, as the variable is already, but practically
there seems no possibility, that this could happen. A test, which checks that, had
to be removed too.

Test scenarios:
* 1st Scenario
    1. Type "@"
    2. Try deleting it
* 2nd Scenario
    1. Type "w"
    2. delete "w"
    3. Try typing "w" again

Fixes #26562

* Fix bug when updating existing variable
---
 public/app/features/variables/editor/actions.ts     | 9 ++++-----
 public/app/features/variables/state/actions.test.ts | 7 ++++---
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/public/app/features/variables/editor/actions.ts b/public/app/features/variables/editor/actions.ts
index 1a7164a1881..150d1e83f54 100644
--- a/public/app/features/variables/editor/actions.ts
+++ b/public/app/features/variables/editor/actions.ts
@@ -57,11 +57,6 @@ export const onEditorAdd = (identifier: VariableIdentifier): ThunkResult<void> =
 
 export const changeVariableName = (identifier: VariableIdentifier, newName: string): ThunkResult<void> => {
   return (dispatch, getState) => {
-    const variableInState = getVariable(identifier.id, getState());
-    if (newName === variableInState.name) {
-      return;
-    }
-
     let errorText = null;
     if (!newName.match(/^(?!__).*$/)) {
       errorText = "Template names cannot begin with '__', that's reserved for Grafana's global variables";
@@ -100,6 +95,10 @@ export const completeChangeVariableName = (identifier: VariableIdentifier, newNa
   getState
 ) => {
   const originalVariable = getVariable(identifier.id, getState());
+  if (originalVariable.name === newName) {
+    dispatch(changeVariableNameSucceeded(toVariablePayload(identifier, { newName })));
+    return;
+  }
   const model = { ...cloneDeep(originalVariable), name: newName, id: newName };
   const global = originalVariable.global;
   const index = originalVariable.index;
diff --git a/public/app/features/variables/state/actions.test.ts b/public/app/features/variables/state/actions.test.ts
index 17088532ba3..5ca68d71dd4 100644
--- a/public/app/features/variables/state/actions.test.ts
+++ b/public/app/features/variables/state/actions.test.ts
@@ -316,7 +316,7 @@ describe('shared actions', () => {
 
   describe('changeVariableName', () => {
     describe('when changeVariableName is dispatched with the same name', () => {
-      it('then no actions are dispatched', () => {
+      it('then the correct actions are dispatched', () => {
         const textbox = textboxBuilder()
           .withId('textbox')
           .withName('textbox')
@@ -333,10 +333,11 @@ describe('shared actions', () => {
             addVariable(toVariablePayload(constant, { global: false, index: 1, model: constant }))
           )
           .whenActionIsDispatched(changeVariableName(toVariableIdentifier(constant), constant.name), true)
-          .thenNoActionsWhereDispatched();
+          .thenDispatchedActionsShouldEqual(
+            changeVariableNameSucceeded({ type: 'constant', id: 'constant', data: { newName: 'constant' } })
+          );
       });
     });
-
     describe('when changeVariableName is dispatched with an unique name', () => {
       it('then the correct actions are dispatched', () => {
         const textbox = textboxBuilder()