Improve tests and refactored JS21143-customize-branch-name-when-using-create-branch-in-an-issue

2 files changed, 108 insertions, 32 deletions

diff --git a/app/assets/javascripts/create_merge_request_dropdown.js b/app/assets/javascripts/create_merge_request_dropdown.js
index 173d864de8c..62e17659e57 100644
--- a/app/assets/javascripts/create_merge_request_dropdown.js
+++ b/app/assets/javascripts/create_merge_request_dropdown.js
@@ -27,7 +27,7 @@ export default class CreateMergeRequestDropdown {
     this.unavailableButtonText = this.unavailableButton.querySelector('.text');
 
     this.branchCreated = false;
-    this.branchTaken = true;
+    this.branchIsValid = true;
     this.canCreatePath = this.wrapperEl.dataset.canCreatePath;
     this.createBranchPath = this.wrapperEl.dataset.createBranchPath;
     this.createMrPath = this.wrapperEl.dataset.createMrPath;
@@ -35,9 +35,9 @@ export default class CreateMergeRequestDropdown {
     this.isCreatingBranch = false;
     this.isCreatingMergeRequest = false;
     this.isGettingRef = false;
-    this.inputsAreValid = true;
     this.mergeRequestCreated = false;
     this.refDebounce = _.debounce((value, target) => this.getRef(value, target), 500);
+    this.refIsValid = true;
     this.refsPath = this.wrapperEl.dataset.refsPath;
 
     // These regexps are used to replace
@@ -66,6 +66,7 @@ export default class CreateMergeRequestDropdown {
     this.createMergeRequestButton.addEventListener('click', this.onClickCreateMergeRequestButton.bind(this));
     this.createTargetButton.addEventListener('click', this.onClickCreateMergeRequestButton.bind(this));
     this.branchInput.addEventListener('keyup', this.onChangeInput.bind(this));
+    this.dropdownToggle.addEventListener('click', this.onClickSetFocusOnBranchNameInput.bind(this));
     this.refInput.addEventListener('keyup', this.onChangeInput.bind(this));
     this.refInput.addEventListener('keydown', CreateMergeRequestDropdown.processTab.bind(this));
     this.refInput.addEventListener('blur', this.clearRefHint.bind(this));
@@ -248,7 +249,16 @@ export default class CreateMergeRequestDropdown {
   initDroplab() {
     this.droplab = new DropLab();
 
-    this.droplab.init(this.dropdownToggle, this.dropdownList, [InputSetter], this.getDroplabConfig());
+    this.droplab.init(
+      this.dropdownToggle,
+      this.dropdownList,
+      [InputSetter],
+      this.getDroplabConfig(),
+    );
+  }
+
+  inputsAreValid() {
+    return this.branchIsValid && this.refIsValid;
   }
 
   isBusy() {
@@ -277,7 +287,7 @@ export default class CreateMergeRequestDropdown {
     if (this.isGettingRef) return false;
 
     // `ENTER` key submits the data.
-    if (event.keyCode === 13 && this.inputsAreValid) {
+    if (event.keyCode === 13 && this.inputsAreValid()) {
       event.preventDefault();
       return this.createMergeRequestButton.click();
     }
@@ -286,7 +296,13 @@ export default class CreateMergeRequestDropdown {
     if (!value) {
       this.createBranchPath = this.wrapperEl.dataset.createBranchPath;
       this.createMrPath = this.wrapperEl.dataset.createMrPath;
-      this.inputsAreValid = true;
+
+      if (target === 'branch') {
+        this.branchIsValid = true;
+      } else {
+        this.refIsValid = true;
+      }
+
       this.enable();
       this.showAvailableMessage(target);
       return true;
@@ -322,6 +338,10 @@ export default class CreateMergeRequestDropdown {
     this.disable();
   }
 
+  onClickSetFocusOnBranchNameInput() {
+    this.branchInput.focus();
+  }
+
   // `TAB` autocompletes the source.
   static processTab(event) {
     if (event.keyCode !== 9) return;
@@ -396,24 +416,21 @@ export default class CreateMergeRequestDropdown {
     // that means a new branch cannot be created as it is already exists.
     if (ref === result) {
       if (target === 'branch') {
-        this.inputsAreValid = false;
-        this.disableCreateAction();
+        this.branchIsValid = false;
         this.showNotAvailableMessage('branch');
       } else {
-        this.inputsAreValid = true;
-        this.enable();
+        this.refIsValid = true;
         this.showAvailableMessage('ref');
         this.createBranchPath = this.createBranchPath.replace(this.regexps.ref.createBranchPath, `$1${ref}`);
         this.createMrPath = this.createMrPath.replace(this.regexps.ref.createMrPath, `$1${ref}`);
       }
     } else if (target === 'branch') {
-      this.inputsAreValid = true;
-      this.enable();
+      this.branchIsValid = true;
       this.showAvailableMessage('branch');
       this.createBranchPath = this.createBranchPath.replace(this.regexps.branch.createBranchPath, `$1${ref}`);
       this.createMrPath = this.createMrPath.replace(this.regexps.branch.createMrPath, `$1${ref}`);
     } else {
-      this.inputsAreValid = false;
+      this.refIsValid = false;
       this.disableCreateAction();
       this.showNotAvailableMessage('ref');
 
@@ -423,5 +440,11 @@ export default class CreateMergeRequestDropdown {
         this.refInput.setSelectionRange(ref.length, result.length);
       }
     }
+
+    if (this.inputsAreValid()) {
+      this.enable();
+    } else {
+      this.disableCreateAction();
+    }
   }
 }
diff --git a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb
index a4d8612cd76..0fe3c799d26 100644
--- a/spec/features/issues/user_creates_branch_and_merge_request_spec.rb
+++ b/spec/features/issues/user_creates_branch_and_merge_request_spec.rb
@@ -27,14 +27,24 @@ describe 'User creates branch and merge request on issue page', :js do
         visit project_issue_path(project, issue)
       end
 
+      # In order to improve tests performance, all UI checks are placed in this test.
       it 'shows elements' do
         button_create_merge_request = find('.js-create-merge-request')
+        button_toggle_dropdown = find('.create-mr-dropdown-wrap .dropdown-toggle')
 
-        find('.create-mr-dropdown-wrap .dropdown-toggle').click
+        button_toggle_dropdown.click
 
-        page.within('.create-merge-request-dropdown-menu') do
+        dropdown = find('.create-merge-request-dropdown-menu')
+
+        # button_toggle_dropdown.native.send_keys(:escape)
+
+        # The dropdown should not be visible if an user pressed the `ESCAPE` key.
+        # expect(page).to have_css('.create-merge-request-dropdown-menu', visible: false)
+
+        page.within(dropdown) do
           button_create_target = find('.js-create-target')
           input_branch_name = find('.js-branch-name')
+          input_source = find('.js-ref')
           li_create_branch = find("li[data-value='create-branch']")
           li_create_merge_request = find("li[data-value='create-mr']")
 
@@ -44,6 +54,7 @@ describe 'User creates branch and merge request on issue page', :js do
           expect(page).to have_content('Branch name')
           expect(page).to have_content('Source (branch or tag)')
           expect(page).to have_button('Create merge request')
+          expect(page).to have_selector('.js-branch-name:focus')
 
           # Test selection mark
           page.within(li_create_merge_request) do
@@ -60,26 +71,15 @@ describe 'User creates branch and merge request on issue page', :js do
             expect(button_create_merge_request).to have_text('Create branch')
           end
 
-          # Test branch name checking
-          expect(input_branch_name.value).to eq(issue.to_branch_name)
-
-          input_branch_name.set('new-branch-name')
-          branch_name_message = find('.js-branch-message')
-
-          expect(branch_name_message).to have_text('Checking branch name availability…')
-
-          wait_for_requests
-
-          expect(branch_name_message).to have_text('branch name is available')
-
-          input_branch_name.set(project.default_branch)
+          test_branch_name_checking(input_branch_name)
+          test_source_checking(input_source)
 
-          expect(branch_name_message).to have_text('Checking branch name availability…')
-
-          wait_for_requests
-
-          expect(branch_name_message).to have_text('Branch is already taken')
+          # The button inside dropdown should be disabled if any errors occured.
+          expect(page).to have_button('Create branch', disabled: true)
         end
+
+        # The top level button should be disabled if any errors occured.
+        expect(page).to have_button('Create branch', disabled: true)
       end
 
       it 'creates a merge request' do
@@ -156,9 +156,62 @@ describe 'User creates branch and merge request on issue page', :js do
     end
   end
 
+  private
+
   def select_dropdown_option(option)
     find('.create-mr-dropdown-wrap .dropdown-toggle').click
     find("li[data-value='#{option}']").click
     find('.js-create-merge-request').click
   end
+
+  def test_branch_name_checking(input_branch_name)
+    expect(input_branch_name.value).to eq(issue.to_branch_name)
+
+    input_branch_name.set('new-branch-name')
+    branch_name_message = find('.js-branch-message')
+
+    expect(branch_name_message).to have_text('Checking branch name availability…')
+
+    wait_for_requests
+
+    expect(branch_name_message).to have_text('branch name is available')
+
+    input_branch_name.set(project.default_branch)
+
+    expect(branch_name_message).to have_text('Checking branch name availability…')
+
+    wait_for_requests
+
+    expect(branch_name_message).to have_text('Branch is already taken')
+  end
+
+  def test_source_checking(input_source)
+    expect(input_source.value).to eq(project.default_branch)
+
+    input_source.set('mas') # Intentionally entered first 3 letters of `master` to check autocomplete feature later.
+    source_message = find('.js-ref-message')
+
+    expect(source_message).to have_text('Checking source availability…')
+
+    wait_for_requests
+
+    expect(source_message).to have_text('Source is not available')
+
+    # JavaScript gets refs started with `mas` (entered above) and places the first match.
+    # User sees `mas` in black color (the part he entered) and the `ter` in gray color (a hint).
+    # Since hinting is implemented via text selection and rspec/capybara doesn't support matchers for it,
+    # we just checking the whole source name.
+    expect(input_source.value).to eq(project.default_branch)
+
+    # `TAB` should apply the hint (autocomplete).`
+    input_source.native.send_keys(:tab)
+
+    expect(source_message).to have_text('Checking source availability…')
+
+    wait_for_requests
+
+    # The source name should be the same. Nothing should be changed/deleted.
+    expect(input_source.value).to eq(project.default_branch)
+    expect(source_message).to have_text('source is available')
+  end
 end