From ae62daf588773c09a206fc0493397cd92cebf52a Mon Sep 17 00:00:00 2001
From: Erik Clarizio <eclarizi@redhat.com>
Date: Fri, 2 Dec 2016 07:42:23 -0800
Subject: [PATCH 1/5] Add new retrieve git datastore click handler in GitImport
 javascript

https://bugzilla.redhat.com/show_bug.cgi?id=1393982
---
 app/assets/javascripts/git_import.js |  48 +++++++++
 spec/javascripts/git_import_spec.js  | 148 +++++++++++++++++++++++++++
 2 files changed, 196 insertions(+)
 create mode 100644 app/assets/javascripts/git_import.js
 create mode 100644 spec/javascripts/git_import_spec.js

diff --git a/app/assets/javascripts/git_import.js b/app/assets/javascripts/git_import.js
new file mode 100644
index 00000000000..3012b0422eb
--- /dev/null
+++ b/app/assets/javascripts/git_import.js
@@ -0,0 +1,48 @@
+/* global miqSparkleOn miqSparkleOff showErrorMessage clearMessages */
+
+var GitImport = {
+  retrieveDatastoreClickHandler: function() {
+    $('.git-retrieve-datastore').click(function(event) {
+      event.preventDefault();
+      miqSparkleOn();
+      clearMessages();
+
+      $.post('retrieve_git_datastore', $('#retrieve-git-datastore-form').serialize(), function(data) {
+        var parsedData = JSON.parse(data);
+        var messages = parsedData.message;
+        if (messages && messages.level === "error") {
+          showErrorMessage(messages.message);
+          miqSparkleOff();
+        } else {
+          GitImport.pollForGitTaskCompletion(parsedData);
+        }
+      });
+    });
+  },
+
+  pollForGitTaskCompletion: function(gitData) {
+    $.get('check_git_task', gitData, function(data) {
+      var parsedData = JSON.parse(data);
+      if (parsedData.state) {
+        setTimeout(GitImport.pollForGitTaskCompletion, 1500, gitData);
+      } else {
+        GitImport.gitTaskCompleted(parsedData);
+      }
+    });
+  },
+
+  gitTaskCompleted: function(data) {
+    if (data.success) {
+      var postMessageData = {
+        git_repo_id: data.git_repo_id,
+        git_branches: data.git_branches,
+        git_tags: data.git_tags,
+        message: data.message
+      };
+
+      parent.postMessage(postMessageData, '*');
+    } else {
+      parent.postMessage({message: data.message}, '*');
+    }
+  },
+};
diff --git a/spec/javascripts/git_import_spec.js b/spec/javascripts/git_import_spec.js
new file mode 100644
index 00000000000..af32a996fd2
--- /dev/null
+++ b/spec/javascripts/git_import_spec.js
@@ -0,0 +1,148 @@
+describe('GitImport', function() {
+  describe('#retrieveDatastoreClickHandler', function() {
+    var retrieveGitDatastoreCallback;
+
+    beforeEach(function() {
+      var html = '';
+      html += '<form id="retrieve-git-datastore-form">';
+      html += '  <input type="text" name="test" value="test" />';
+      html += '  <button class="git-retrieve-datastore" />';
+      html += '</form>';
+
+      spyOn(window, 'miqSparkleOn');
+      spyOn(window, 'clearMessages');
+
+      spyOn($, 'post').and.callFake(function(url, data, callback) {
+        retrieveGitDatastoreCallback = callback;
+      });
+
+      setFixtures(html);
+      GitImport.retrieveDatastoreClickHandler();
+    });
+
+    it('turns on the spinner', function() {
+      $('.git-retrieve-datastore').trigger('click');
+      expect(window.miqSparkleOn).toHaveBeenCalled();
+    });
+
+    it('clears the messages', function() {
+      $('.git-retrieve-datastore').trigger('click');
+      expect(window.clearMessages).toHaveBeenCalled();
+    });
+
+    it('makes a post to retrieve the git datastore', function() {
+      $('.git-retrieve-datastore').trigger('click');
+      expect($.post).toHaveBeenCalledWith('retrieve_git_datastore', 'test=test', retrieveGitDatastoreCallback);
+
+    });
+
+    describe('#retrieveDatastoreClickHandler retrieveGitDatastoreCallback', function() {
+      describe('when there are messages and the level is "error"', function() {
+        beforeEach(function() {
+          spyOn(window, 'showErrorMessage');
+          spyOn(window, 'miqSparkleOff');
+
+          retrieveGitDatastoreCallback('{"message": {"message": "the message", "level": "error"}}');
+        });
+
+        it('shows error messages', function() {
+          expect(window.showErrorMessage).toHaveBeenCalledWith('the message');
+        });
+
+        it('turns off the sparkle', function() {
+          expect(window.miqSparkleOff).toHaveBeenCalled();
+        });
+      });
+
+      describe('when there are are no "error" level messages', function() {
+        beforeEach(function() {
+          spyOn(GitImport, 'pollForGitTaskCompletion');
+          retrieveGitDatastoreCallback('{"message": {"message": "the message"}}');
+        });
+
+        it('polls for task completion', function() {
+          expect(GitImport.pollForGitTaskCompletion).toHaveBeenCalledWith({message: {message: 'the message'}});
+        });
+      });
+    });
+  });
+
+  describe('#pollForGitTaskCompletion', function() {
+    var checkGitTaskCallback;
+
+    beforeEach(function() {
+      spyOn($, 'get').and.callFake(function(url, data, callback) {
+        checkGitTaskCallback = callback;
+      });
+    });
+
+    it('makes a get request to check_git_task', function() {
+      GitImport.pollForGitTaskCompletion('the data');
+      expect($.get).toHaveBeenCalledWith('check_git_task', 'the data', checkGitTaskCallback);
+    });
+
+    describe('#pollForGitTaskCompletion checkGitTaskCallback', function() {
+      context('when data has a state', function() {
+        beforeEach(function() {
+          spyOn(window, 'setTimeout');
+        });
+
+        it('sets a timeout to call itself', function() {
+          checkGitTaskCallback('{"state": "still doing stuff"}');
+          expect(window.setTimeout).toHaveBeenCalledWith(GitImport.pollForGitTaskCompletion, 1500, 'the data');
+        });
+      });
+
+      context('when the data does not have a state', function() {
+        beforeEach(function() {
+          spyOn(GitImport, 'gitTaskCompleted');
+        });
+
+        it('calls gitTaskCompleted with the parsed data', function() {
+          checkGitTaskCallback('{"parsed": "data"}');
+          expect(GitImport.gitTaskCompleted).toHaveBeenCalledWith({'parsed': 'data'});
+        });
+      });
+    });
+  });
+
+  describe('#gitTaskCompleted', function() {
+    var data = {
+      git_repo_id: 123,
+      git_branches: 'gitbranches',
+      git_tags: 'gittags',
+      message: 'the message'
+    };
+
+    beforeEach(function() {
+      spyOn(parent, 'postMessage');
+    });
+
+    context('when data.success is true', function() {
+      beforeEach(function() {
+        data.success = true;
+      });
+
+      it('posts a message to the parent with all the git data', function() {
+        GitImport.gitTaskCompleted(data);
+        expect(parent.postMessage).toHaveBeenCalledWith({
+          git_repo_id: 123,
+          git_branches: 'gitbranches',
+          git_tags: 'gittags',
+          message: 'the message'
+        }, '*');
+      });
+    });
+
+    context('when data.success is false', function() {
+      beforeEach(function() {
+        data.success = false;
+      });
+
+      it('posts a message to the parent with only the message data', function() {
+        GitImport.gitTaskCompleted(data);
+        expect(parent.postMessage).toHaveBeenCalledWith({message: 'the message'}, '*');
+      });
+    });
+  });
+});

From 436969b205dc85db451136c2d19c302714c2820d Mon Sep 17 00:00:00 2001
From: Erik Clarizio <eclarizi@redhat.com>
Date: Fri, 2 Dec 2016 07:43:03 -0800
Subject: [PATCH 2/5] Add GitRepositoryService in preparation for extracting
 logic

https://bugzilla.redhat.com/show_bug.cgi?id=1393982
---
 app/services/git_repository_service.rb       |  15 +++
 spec/services/git_repository_service_spec.rb | 109 +++++++++++++++++++
 2 files changed, 124 insertions(+)
 create mode 100644 app/services/git_repository_service.rb
 create mode 100644 spec/services/git_repository_service_spec.rb

diff --git a/app/services/git_repository_service.rb b/app/services/git_repository_service.rb
new file mode 100644
index 00000000000..17347e813df
--- /dev/null
+++ b/app/services/git_repository_service.rb
@@ -0,0 +1,15 @@
+class GitRepositoryService
+  def setup(git_url, git_username, git_password, verify_ssl)
+    new_git_repo = false
+    git_repo = GitRepository.find_or_create_by!(:url => git_url) { new_git_repo = true }
+    git_repo.update_attributes(
+      :verify_ssl => verify_ssl == "true" ? OpenSSL::SSL::VERIFY_PEER : OpenSSL::SSL::VERIFY_NONE
+    )
+
+    if git_username && git_password
+      git_repo.update_authentication(:values => {:userid => git_username, :password => git_password})
+    end
+
+    return {:git_repo_id => git_repo.id, :new_git_repo? => new_git_repo}
+  end
+end
diff --git a/spec/services/git_repository_service_spec.rb b/spec/services/git_repository_service_spec.rb
new file mode 100644
index 00000000000..25e7f214558
--- /dev/null
+++ b/spec/services/git_repository_service_spec.rb
@@ -0,0 +1,109 @@
+describe GitRepositoryService do
+  let(:git_repository_service) { described_class.new }
+  describe "#setup" do
+    let(:git_url) { "http://example.com/" }
+    let(:git_username) { nil }
+    let(:git_password) { nil }
+    let(:new_record) { false }
+
+    context "when the git repository already exists" do
+      let!(:git_repo) { GitRepository.create!(:url => "http://example.com/") }
+
+      shared_examples_for "GitRepositoryService#setup when verify_ssl is 'true'" do
+        it "sets the verify_ssl setting to verify_peer" do
+          git_repository_service.setup(git_url, git_username, git_password, verify_ssl)
+          expect(git_repo.reload.verify_ssl).to eq(OpenSSL::SSL::VERIFY_PEER)
+        end
+      end
+
+      shared_examples_for "GitRepositoryService#setup with an existing repo" do
+        it "returns the id and the fact that it is not a new repo" do
+          expect(git_repository_service.setup(git_url, git_username, git_password, verify_ssl)).to eq(
+            :git_repo_id => git_repo.id, :new_git_repo? => false
+          )
+        end
+      end
+
+      context "when verify_ssl is 'true'" do
+        let(:verify_ssl) { "true" }
+
+        context "when git username is present" do
+          let(:git_username) { "username" }
+
+          context "when git password is present" do
+            let(:git_password) { "password" }
+
+            before do
+              allow(MiqQueue).to receive(:put_unless_exists)
+            end
+
+            it "updates the authentication" do
+              git_repository_service.setup(git_url, git_username, git_password, verify_ssl)
+              authentication = git_repo.authentications.first
+              expect(authentication.userid).to eq("username")
+              expect(authentication.password).to eq("password")
+            end
+
+            it_behaves_like "GitRepositoryService#setup when verify_ssl is 'true'"
+            it_behaves_like "GitRepositoryService#setup with an existing repo"
+          end
+
+          context "when git password is not present" do
+            it_behaves_like "GitRepositoryService#setup when verify_ssl is 'true'"
+            it_behaves_like "GitRepositoryService#setup with an existing repo"
+          end
+        end
+
+        context "when git username is not present" do
+          it_behaves_like "GitRepositoryService#setup when verify_ssl is 'true'"
+          it_behaves_like "GitRepositoryService#setup with an existing repo"
+        end
+      end
+
+      context "when verify_ssl is not 'true'" do
+        let(:verify_ssl) { "not true" }
+
+        it "sets the verify_ssl setting to verify_none" do
+          git_repository_service.setup(git_url, git_username, git_password, verify_ssl)
+          expect(git_repo.reload.verify_ssl).to eq(OpenSSL::SSL::VERIFY_NONE)
+        end
+
+        it_behaves_like "GitRepositoryService#setup with an existing repo"
+      end
+    end
+
+    context "when the git repository does not already exist" do
+      shared_examples_for "GitRepositoryService#setup when a git repo does not exist" do
+        it "returns the id and the fact that it is a new repo" do
+          expect(git_repository_service.setup(git_url, git_username, git_password, verify_ssl)).to eq(
+            :git_repo_id => GitRepository.first.id, :new_git_repo? => true
+          )
+        end
+      end
+
+      context "when verify_ssl is 'true'" do
+        let(:verify_ssl) { "true" }
+
+        it "creates a git repository with verify_peer" do
+          git_repository_service.setup(git_url, git_username, git_password, verify_ssl)
+          git_repo = GitRepository.first
+          expect(git_repo.verify_ssl).to eq(OpenSSL::SSL::VERIFY_PEER)
+        end
+
+        it_behaves_like "GitRepositoryService#setup when a git repo does not exist"
+      end
+
+      context "when verify_ssl is not 'true'" do
+        let(:verify_ssl) { "not true" }
+
+        it "creates a git repository with verify_none" do
+          git_repository_service.setup(git_url, git_username, git_password, verify_ssl)
+          git_repo = GitRepository.first
+          expect(git_repo.verify_ssl).to eq(OpenSSL::SSL::VERIFY_NONE)
+        end
+
+        it_behaves_like "GitRepositoryService#setup when a git repo does not exist"
+      end
+    end
+  end
+end

From 7462b29ffa99d23f097dc3850eab4092330e0664 Mon Sep 17 00:00:00 2001
From: Erik Clarizio <eclarizi@redhat.com>
Date: Fri, 2 Dec 2016 07:44:33 -0800
Subject: [PATCH 3/5] Replace retrieve_git_datastore call with an async version

https://bugzilla.redhat.com/show_bug.cgi?id=1393982

retrieve_git_datastore now returns a task id that the javascript then
uses to poll to see if the task is done. When it is, it runs through the
rendering of the git import stuff.
---
 app/assets/javascripts/application.js         |   1 +
 .../javascripts/automate_import_export.js     |  18 +-
 app/assets/javascripts/import.js              |   3 +-
 app/controllers/miq_ae_tools_controller.rb    |  79 ++++--
 .../miq_ae_tools/_import_export.html.haml     |   5 +-
 config/routes.rb                              |   1 +
 .../miq_ae_tools_controller_spec.rb           | 261 ++++++++++--------
 .../automate_import_export_spec.js            |  90 ++++++
 spec/javascripts/import_spec.js               |   2 +-
 9 files changed, 304 insertions(+), 156 deletions(-)

diff --git a/app/assets/javascripts/application.js b/app/assets/javascripts/application.js
index 02763c69b66..6692a67bc78 100644
--- a/app/assets/javascripts/application.js
+++ b/app/assets/javascripts/application.js
@@ -34,6 +34,7 @@
 //= require miq_application
 //= require miq_change_stored_password
 //= require miq_qe
+//= require git_import
 //= require automate_import_export
 //= require dialog_field_refresh
 //= require excanvas
diff --git a/app/assets/javascripts/automate_import_export.js b/app/assets/javascripts/automate_import_export.js
index 81cb4abcef3..b1d71b4828c 100644
--- a/app/assets/javascripts/automate_import_export.js
+++ b/app/assets/javascripts/automate_import_export.js
@@ -33,8 +33,9 @@ var Automate = {
 
   renderGitImport: function(branches, tags, gitRepoId, messages) {
     clearMessages();
-    var message = JSON.parse(messages).message;
-    var messageLevel = JSON.parse(messages).level;
+
+    var message = messages.message;
+    var messageLevel = messages.level;
 
     if (messageLevel === 'error') {
       showErrorMessage(message);
@@ -50,17 +51,14 @@ var Automate = {
 
       var addToDropDown = function(identifier, child) {
         $('select.git-' + identifier).append(
-          $('<option>', {
-            value: child,
-            text: child,
-          })
+          $('<option>', {value: child, text: child})
         );
       };
 
-      $.each(JSON.parse(branches), function(_index, child) {
+      $.each(branches, function(_index, child) {
         addToDropDown('branches', child);
       });
-      $.each(JSON.parse(tags), function(_index, child) {
+      $.each(tags, function(_index, child) {
         addToDropDown('tags', child);
       });
 
@@ -169,9 +167,7 @@ var Automate = {
 
     Automate.setUpGitRefreshClickHandlers();
 
-    $('.git-retreive-datastore').click(function() {
-      miqSparkleOn();
-    });
+    GitImport.retrieveDatastoreClickHandler();
 
     $('.git-import-submit').click(function(event) {
       event.preventDefault();
diff --git a/app/assets/javascripts/import.js b/app/assets/javascripts/import.js
index fc50be2c3ec..973e5363390 100644
--- a/app/assets/javascripts/import.js
+++ b/app/assets/javascripts/import.js
@@ -19,8 +19,7 @@ var ImportSetup = {
 
   listenForGitPostMessages: function() {
     window.addEventListener('message', function(event) {
-      var unencodedMessage = event.data.message.replace(/&quot;/g, '"');
-      var messageData = JSON.parse(unencodedMessage);
+      var messageData = event.data.message;
 
       if (messageData.level === 'error') {
         showErrorMessage(messageData.message);
diff --git a/app/controllers/miq_ae_tools_controller.rb b/app/controllers/miq_ae_tools_controller.rb
index a634981734b..6e941924fef 100644
--- a/app/controllers/miq_ae_tools_controller.rb
+++ b/app/controllers/miq_ae_tools_controller.rb
@@ -196,23 +196,24 @@ def review_import
   end
 
   def retrieve_git_datastore
-    redirect_options = {:action => :review_git_import}
     git_url = params[:git_url]
-    verify_ssl = params[:git_verify_ssl] == "true" ? OpenSSL::SSL::VERIFY_PEER : OpenSSL::SSL::VERIFY_NONE
-    new_git_repo = false
 
     if git_url.blank?
       add_flash(_("Please provide a valid git URL"), :error)
+      response_json = {:message => @flash_array.first}
     elsif !GitBasedDomainImportService.available?
       add_flash(_("Please enable the git owner role in order to import git repositories"), :error)
+      response_json = {:message => @flash_array.first}
     else
       begin
-        git_repo = GitRepository.find_or_create_by!(:url => git_url) { new_git_repo = true }
-        git_repo.update_attributes(:verify_ssl => verify_ssl)
-        if params[:git_username] && params[:git_password]
-          git_repo.update_authentication(:values => {:userid   => params[:git_username],
-                                                     :password => params[:git_password]})
-        end
+        setup_results = git_repository_service.setup(
+          git_url,
+          params[:git_username],
+          params[:git_password],
+          params[:git_verify_ssl]
+        )
+        git_repo_id = setup_results[:git_repo_id]
+        new_git_repo = setup_results[:new_git_repo?]
 
         task_options = {
           :action => "Retrieve git repository",
@@ -221,32 +222,56 @@ def retrieve_git_datastore
         queue_options = {
           :class_name  => "GitRepository",
           :method_name => "refresh",
-          :instance_id => git_repo.id,
+          :instance_id => git_repo_id,
           :role        => "git_owner",
           :args        => []
         }
 
         task_id = MiqTask.generic_action_with_callback(task_options, queue_options)
-        task = MiqTask.wait_for_taskid(task_id)
-
-        raise task.message unless task.status == "Ok"
-
-        branch_names = git_repo.git_branches.collect(&:name)
-        tag_names = git_repo.git_tags.collect(&:name)
-        redirect_options[:git_branches] = branch_names.to_json
-        redirect_options[:git_tags] = tag_names.to_json
-        redirect_options[:git_repo_id] = git_repo.id
-        flash_message = "Successfully found git repository, please choose a branch or tag"
-        add_flash(_(flash_message), :success)
+        response_json = {:task_id => task_id, :git_repo_id => git_repo_id, :new_git_repo => new_git_repo}
       rescue => err
-        git_repo.destroy if git_repo && new_git_repo
-        add_flash(_("Error during repository fetch: %{error_message}") % {:error_message => err.message}, :error)
+        add_flash(_("Error during repository setup: %{error_message}") % {:error_message => err.message}, :error)
+        response_json = {:message => @flash_array.first}
       end
     end
 
-    redirect_options[:message] = @flash_array.first.to_json
+    respond_to do |format|
+      format.js { render :json => response_json.to_json, :status => 200 }
+    end
+  end
 
-    redirect_to redirect_options
+  def check_git_task
+    task = MiqTask.find(params[:task_id])
+    json = if task.state != MiqTask::STATE_FINISHED
+             {:state => task.state}
+           else
+             git_repo = GitRepository.find(params[:git_repo_id])
+
+             if task.status == "Ok"
+               branch_names = git_repo.git_branches.collect(&:name)
+               tag_names = git_repo.git_tags.collect(&:name)
+               flash_message = "Successfully found git repository, please choose a branch or tag"
+               add_flash(_(flash_message), :success)
+               {
+                 :git_branches => branch_names,
+                 :git_tags     => tag_names,
+                 :git_repo_id  => git_repo.id,
+                 :success      => true,
+                 :message      => @flash_array.first
+               }
+             else
+               git_repo.destroy if git_repo && params[:new_git_repo] != "false"
+               add_flash(_("Error during repository fetch: #{task.message}"), :error)
+               {
+                 :success => false,
+                 :message => @flash_array.first
+               }
+             end
+           end
+
+    respond_to do |format|
+      format.js { render :json => json.to_json, :status => 200 }
+    end
   end
 
   def review_git_import
@@ -320,6 +345,10 @@ def git_based_domain_import_service
     @git_based_domain_import_service ||= GitBasedDomainImportService.new
   end
 
+  def git_repository_service
+    @git_repository_service ||= GitRepositoryService.new
+  end
+
   def add_stats(stats_hash)
     stats_hash.inject(0) do |result, key_value|
       result + key_value[1]
diff --git a/app/views/miq_ae_tools/_import_export.html.haml b/app/views/miq_ae_tools/_import_export.html.haml
index 763a25cc556..0ad09c48132 100644
--- a/app/views/miq_ae_tools/_import_export.html.haml
+++ b/app/views/miq_ae_tools/_import_export.html.haml
@@ -27,7 +27,8 @@
     = _('Import Datastore via git')
   = form_tag({:action => "retrieve_git_datastore"},
               :target => "upload_target",
-              :method => :post) do
+              :method => :post,
+              :id     => "retrieve-git-datastore-form") do
     .form-horizontal
       .form-group
         %label.col-sm-2.control-label
@@ -54,7 +55,7 @@
         .col-md-8
           = submit_tag(_("Submit"),
             :id       => "git-url-import",
-            :class    => "git-retreive-datastore btn btn-default",
+            :class    => "git-retrieve-datastore btn btn-default",
             :disabled => !git_import_button_enabled?)
           = git_import_submit_help
 
diff --git a/config/routes.rb b/config/routes.rb
index d0dcbf6627c..d85e3ce3416 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -1910,6 +1910,7 @@
     :miq_ae_tools             => {
       :get  => %w(
         automate_json
+        check_git_task
         export_datastore
         fetch_log
         import_export
diff --git a/spec/controllers/miq_ae_tools_controller_spec.rb b/spec/controllers/miq_ae_tools_controller_spec.rb
index cef8a56f9e5..e8ed54c456b 100644
--- a/spec/controllers/miq_ae_tools_controller_spec.rb
+++ b/spec/controllers/miq_ae_tools_controller_spec.rb
@@ -259,168 +259,199 @@
     include_context "valid session"
 
     let(:params) do
-      {:git_url => git_url, :git_username => nil, :git_password => nil, :git_verify_ssl => git_verify_ssl}
+      {
+        :git_url        => git_url,
+        :git_username   => "gitusername",
+        :git_password   => "gitpassword",
+        :git_verify_ssl => "gitverifyssl"
+      }
     end
 
     context "when the git url is blank" do
       let(:git_url) { "" }
-      let(:git_verify_ssl) { "true" }
 
-      it "redirects with a flash error" do
-        post :retrieve_git_datastore, :params => params
-        expect(response).to redirect_to(
-          :action  => :review_git_import,
+      it "responds with an error message" do
+        post :retrieve_git_datastore, :params => params, :xhr => true
+        expect(response.body).to eq({
           :message => {
             :message => "Please provide a valid git URL",
             :level   => :error
-          }.to_json
-        )
+          }
+        }.to_json)
       end
     end
 
     context "when the git url is not blank" do
-      let(:git_url) { "http://www.example.com" }
-      let(:git_verify_ssl) { "true" }
-      let(:my_region) { double("MiqRegion") }
+      let(:git_url) { "http://www.example.com/" }
 
-      before do
-        allow(MiqRegion).to receive(:my_region).and_return(my_region)
-        allow(my_region).to receive(:role_active?).with("git_owner").and_return(git_owner_active)
-      end
-
-      context "when the MiqRegion does not have an active git_owner role" do
-        let(:git_owner_active) { false }
+      context "when the import service is not available" do
+        before do
+          allow(GitBasedDomainImportService).to receive(:available?).and_return(false)
+        end
 
-        it "redirects with a flash error" do
-          post :retrieve_git_datastore, :params => params
-          expect(response).to redirect_to(
-            :action  => :review_git_import,
+        it "responds with an error message" do
+          post :retrieve_git_datastore, :params => params, :xhr => true
+          expect(response.body).to eq({
             :message => {
               :message => "Please enable the git owner role in order to import git repositories",
               :level   => :error
-            }.to_json
-          )
+            }
+          }.to_json)
         end
       end
 
-      context "when the MiqRegion has an active git_owner role" do
-        let(:verify_ssl) { OpenSSL::SSL::VERIFY_PEER }
-        let(:task_message) { "Success" }
-        let(:task_status) { "Ok" }
-        let(:git_owner_active) { true }
-        let(:git_repo) { double("GitRepository", :id => 321) }
-        let(:miq_task) do
-          double("MiqTask", :id => 3211, :status => task_status, :message => task_message)
-        end
-        let(:git_branches) { [double("GitBranch", :name => "git_branch1")] }
-        let(:git_tags) { [double("GitTag", :name => "git_tag1")] }
-        let(:task_options) { {:action => "Retrieve git repository", :userid => controller.current_user.userid} }
-        let(:queue_options) do
-          {
-            :class_name  => "GitRepository",
-            :method_name => "refresh",
-            :instance_id => git_repo.id,
-            :role        => "git_owner",
-            :args        => []
-          }
-        end
+      context "when the import service is available" do
+        let(:git_repository_service) { double("GitRepositoryService") }
 
         before do
-          allow(GitRepository).to receive(:create!).with(:url => git_url).and_return(git_repo)
-          allow(git_repo).to receive(:update_authentication).with(:values => {:userid => "", :password => ""})
-          allow(MiqTask).to receive(:generic_action_with_callback)
-            .with(task_options, queue_options).and_return(miq_task.id)
-          allow(MiqTask).to receive(:wait_for_taskid).with(miq_task.id).and_return(miq_task)
-          allow(git_repo).to receive(:git_branches).and_return(git_branches)
-          allow(git_repo).to receive(:git_tags).and_return(git_tags)
-          allow(git_repo).to receive(:update_attributes).and_return(nil)
+          allow(GitBasedDomainImportService).to receive(:available?).and_return(true)
+          allow(GitRepositoryService).to receive(:new).and_return(git_repository_service)
         end
 
-        shared_examples_for "task failure" do
-          it "check error message" do
-            post :retrieve_git_datastore, :params => params
-            expect(response).to redirect_to(
-              :action       => :review_git_import,
-              :message      => {
-                :message => "Error during repository fetch: #{task_message}",
-                :level   => :error
-              }.to_json
-            )
+        context "when something goes wrong in the git repository service" do
+          before do
+            allow(git_repository_service).to receive(:setup).and_raise("Oopsies")
           end
-        end
 
-        context "when the git repository exists with the given url" do
-          before do
-            allow(GitRepository).to receive(:find_or_create_by!).with(:url => git_url).and_return(git_repo)
+          it "responds with an error message" do
+            post :retrieve_git_datastore, :params => params, :xhr => true
+            expect(response.body).to eq({
+              :message => {
+                :message => "Error during repository setup: Oopsies",
+                :level   => :error
+              }
+            }.to_json)
           end
+        end
 
-          it "queues the refresh action" do
-            expect(MiqTask).to receive(:generic_action_with_callback).with(task_options, queue_options)
-            post :retrieve_git_datastore, :params => params
+        context "when everything works fine" do
+          let(:git_repo) { double("GitRepository", :id => 123) }
+          let(:task_options) { {:action => "Retrieve git repository", :userid => controller.current_user.userid} }
+          let(:queue_options) do
+            {
+              :class_name  => "GitRepository",
+              :method_name => "refresh",
+              :instance_id => 123,
+              :role        => "git_owner",
+              :args        => []
+            }
           end
 
-          it "waits for the refresh action" do
-            expect(MiqTask).to receive(:wait_for_taskid).with(miq_task.id)
-            post :retrieve_git_datastore, :params => params
+          before do
+            allow(git_repository_service).to receive(:setup).with(
+              git_url,
+              "gitusername",
+              "gitpassword",
+              "gitverifyssl"
+            ).and_return({:git_repo_id => git_repo.id, :new_git_repo? => false})
+            allow(MiqTask).to receive(:generic_action_with_callback).with(task_options, queue_options).and_return(321)
           end
 
-          context "task failure" do
-            let(:task_message) { "Disaster happened" }
-            let(:task_status) { "Failed" }
-            it_behaves_like "task failure"
+          it "responds with task information" do
+            post :retrieve_git_datastore, :params => params, :xhr => true
+            expect(response.body).to eq({
+              :task_id      => 321,
+              :git_repo_id  => 123,
+              :new_git_repo => false
+            }.to_json)
           end
         end
+      end
+    end
+  end
 
-        context "when the repository is using self signed certificates" do
-          let (:verify_ssl) { OpenSSL::SSL::VERIFY_NONE }
-          let (:git_verify_ssl) { "false" }
+  describe "#check_git_task" do
+    include_context "valid session"
 
-          before do
-            allow(GitRepository).to receive(:exists?).with(:url => git_url).and_return(false)
-          end
-          it "queues the refresh action" do
-            expect(MiqTask).to receive(:generic_action_with_callback).with(task_options, queue_options)
-            post :retrieve_git_datastore, :params => params
-          end
+    let(:params) do
+      {:task_id => 123, :git_repo_id => 321, :new_git_repo => new_git_repo}
+    end
+    let(:new_git_repo) { false }
+
+    let(:miq_task) { double("MiqTask", :state => state, :status => status, :message => "o noes") }
+    let(:status) { "not ok" }
+
+    before do
+      allow(MiqTask).to receive(:find).with("123").and_return(miq_task)
+    end
+
+    context "when the task's state is not finished" do
+      let(:state) { "potato" }
+
+      it "renders the state as json" do
+        get :check_git_task, :params => params, :xhr => true
+        expect(response.body).to eq({:state => "potato"}.to_json)
+      end
+    end
+
+    context "when the task's state is finished" do
+      let(:state) { MiqTask::STATE_FINISHED }
+      let(:git_repo) { double("GitRepository", :id => 321, :git_branches => git_branches, :git_tags => git_tags) }
+      let(:git_branches) { [double("GitBranch", :name => "branches")] }
+      let(:git_tags) { [double("GitTag", :name => "tags")] }
+
+      before do
+        allow(GitRepository).to receive(:find).with("321").and_return(git_repo)
+      end
+
+      context "when the status is 'Ok'" do
+        let(:status) { "Ok" }
+
+        it "responds with the git information" do
+          get :check_git_task, :params => params, :xhr => true
+          expect(response.body).to eq({
+            :git_branches => ["branches"],
+            :git_tags     => ["tags"],
+            :git_repo_id  => 321,
+            :success      => true,
+            :message      => {
+              :message => "Successfully found git repository, please choose a branch or tag",
+              :level   => :success
+            }
+          }.to_json)
         end
+      end
+
+      context "when the status is not 'Ok'" do
+        context "when the repository is new" do
+          let(:new_git_repo) { true }
 
-        context "when the git repository does not exist with the given url" do
           before do
-            allow(GitRepository).to receive(:find_or_create_by!).with(:url => git_url).and_return(git_repo)
+            allow(git_repo).to receive(:destroy)
           end
 
-          it "queues the refresh action" do
-            expect(MiqTask).to receive(:generic_action_with_callback).with(task_options, queue_options)
-            post :retrieve_git_datastore, :params => params
+          it "destroys the git repository" do
+            expect(git_repo).to receive(:destroy)
+            get :check_git_task, :params => params, :xhr => true
           end
 
-          it "waits for the refresh action" do
-            expect(MiqTask).to receive(:wait_for_taskid).with(miq_task.id).and_return(miq_task)
-            post :retrieve_git_datastore, :params => params
+          it "responds with the error" do
+            get :check_git_task, :params => params, :xhr => true
+            expect(response.body).to eq({
+              :success => false,
+              :message => {
+                :message => "Error during repository fetch: o noes",
+                :level   => :error
+              }
+            }.to_json)
           end
+        end
 
-          context "task failure" do
-            before do
-              allow(git_repo).to receive(:destroy).with(no_args)
-            end
-
-            let(:task_message) { "Disaster happened" }
-            let(:task_status) { "Failed" }
-            it_behaves_like "task failure"
+        context "when the repository is not new" do
+          it "does not destroy the git repository" do
+            expect(git_repo).to_not receive(:destroy)
+            get :check_git_task, :params => params, :xhr => true
           end
 
-          it "adds a success flash message with the other redirect options" do
-            post :retrieve_git_datastore, :params => params
-            expect(response).to redirect_to(
-              :action       => :review_git_import,
-              :git_branches => ["git_branch1"].to_json,
-              :git_tags     => ["git_tag1"].to_json,
-              :git_repo_id  => 321,
-              :message      => {
-                :message => "Successfully found git repository, please choose a branch or tag",
-                :level   => :success
-              }.to_json
-            )
+          it "responds with the error" do
+            get :check_git_task, :params => params, :xhr => true
+            expect(response.body).to eq({
+              :success => false,
+              :message => {
+                :message => "Error during repository fetch: o noes",
+                :level   => :error
+              }
+            }.to_json)
           end
         end
       end
diff --git a/spec/javascripts/automate_import_export_spec.js b/spec/javascripts/automate_import_export_spec.js
index e36b626d950..7b7c8bd4e8b 100644
--- a/spec/javascripts/automate_import_export_spec.js
+++ b/spec/javascripts/automate_import_export_spec.js
@@ -185,4 +185,94 @@ describe('Automate', function() {
       });
     });
   });
+
+  describe('#renderGitImport', function() {
+    beforeEach(function() {
+      var html = '';
+      html += '<input type="hidden" class="hidden-git-repo-id" />';
+      html += '<div class="git-import-data" style="display: none;" />';
+      html += '<div class="import-or-export" />';
+      html += '<select class="git-branches"></select>';
+      html += '<select class="git-tags"></select>';
+
+      spyOn(window, 'clearMessages');
+
+      setFixtures(html);
+    });
+
+    context('when the message level is an error', function() {
+      beforeEach(function() {
+        spyOn(window, 'showErrorMessage');
+      });
+
+      it('clears messages', function() {
+        Automate.renderGitImport('branches', 'tags', 'gitrepoid', {message: 'the message', level: 'error'});
+        expect(window.clearMessages).toHaveBeenCalled();
+      });
+
+      it('calls showErrorMessage with the message', function() {
+        Automate.renderGitImport('branches', 'tags', 'gitrepoid', {message: 'the message', level: 'error'});
+        expect(window.showErrorMessage).toHaveBeenCalledWith('the message');
+      });
+    });
+
+    context('when the message level is not an error', function() {
+      beforeEach(function() {
+        spyOn(Automate, 'selectDefaultBranch');
+        spyOn($.fn, 'selectpicker');
+      });
+
+      context('when the message level is a warning', function() {
+        beforeEach(function() {
+          spyOn(window, 'showWarningMessage');
+        });
+
+        it('assigns the repo id into the hidden input', function() {
+          Automate.renderGitImport(['branches'], ['tags'], '123', {message: 'the message', level: 'warning'});
+          expect($('.hidden-git-repo-id').val()).toEqual("123");
+        });
+
+        it('shows the git import data div', function() {
+          Automate.renderGitImport(['branches'], ['tags'], '123', {message: 'the message', level: 'warning'});
+          expect($('.git-import-data')).toBeVisible();
+        });
+
+        it('hides the import or export div', function() {
+          Automate.renderGitImport(['branches'], ['tags'], '123', {message: 'the message', level: 'warning'});
+          expect($('.import-or-export')).not.toBeVisible();
+        });
+
+        it('calls showWarningMessage with the message', function() {
+          Automate.renderGitImport(['branches'], ['tags'], '123', {message: 'the message', level: 'warning'});
+          expect(window.showWarningMessage).toHaveBeenCalledWith('the message');
+        });
+
+        it('adds the options to the dropdowns', function() {
+          expect($('select.git-branches')[0].options.length).toEqual(0);
+          expect($('select.git-tags')[0].options.length).toEqual(0);
+          Automate.renderGitImport(['branches'], ['tags'], '123', {message: 'the message', level: 'warning'});
+          expect($('select.git-branches')[0].options.length).toEqual(1);
+          expect($('select.git-tags')[0].options.length).toEqual(1);
+        });
+
+        it('calls Automate.selectDefaultBranch', function() {
+          Automate.renderGitImport(['branches'], ['tags'], '123', {message: 'the message', level: 'warning'});
+          expect(Automate.selectDefaultBranch).toHaveBeenCalled();
+        });
+
+        it('refreshes the selectpicker for git-branches and git-tags', function() {
+          Automate.renderGitImport(['branches'], ['tags'], '123', {message: 'the message', level: 'warning'});
+          expect($.fn.selectpicker.calls.allArgs()).toEqual([['refresh'], ['refresh']]);
+          expect($.fn.selectpicker.calls.first().object.selector).toEqual('select.git-branches');
+          expect($.fn.selectpicker.calls.mostRecent().object.selector).toEqual('select.git-tags');
+        });
+      });
+
+      context('when the message level is not a warning', function() {
+        beforeEach(function() {
+          spyOn(window, 'showSuccessMessage');
+        });
+      });
+    });
+  });
 });
diff --git a/spec/javascripts/import_spec.js b/spec/javascripts/import_spec.js
index 3e988cec777..6d3984221f7 100644
--- a/spec/javascripts/import_spec.js
+++ b/spec/javascripts/import_spec.js
@@ -109,7 +109,7 @@ describe('import.js', function() {
             spyOn(window, 'showErrorMessage');
             spyOn($.fn, 'prop');
             event.data = {
-              message: '{&quot;level&quot;: &quot;error&quot;, &quot;message&quot;: &quot;test&quot;}'
+              message: {level: 'error', message: 'test'}
             };
             gitPostMessageCallback(event);
           });

From d5ee1dd924c7360859b14bb4223b1eba1f8c701f Mon Sep 17 00:00:00 2001
From: Erik Clarizio <eclarizi@redhat.com>
Date: Fri, 2 Dec 2016 11:14:19 -0800
Subject: [PATCH 4/5] Utilize queue_refresh call within
 GitBasedDomainImportService

https://bugzilla.redhat.com/show_bug.cgi?id=1393982
---
 app/controllers/miq_ae_tools_controller.rb       | 14 +-------------
 spec/controllers/miq_ae_tools_controller_spec.rb | 14 +++-----------
 2 files changed, 4 insertions(+), 24 deletions(-)

diff --git a/app/controllers/miq_ae_tools_controller.rb b/app/controllers/miq_ae_tools_controller.rb
index 6e941924fef..311d6746445 100644
--- a/app/controllers/miq_ae_tools_controller.rb
+++ b/app/controllers/miq_ae_tools_controller.rb
@@ -215,19 +215,7 @@ def retrieve_git_datastore
         git_repo_id = setup_results[:git_repo_id]
         new_git_repo = setup_results[:new_git_repo?]
 
-        task_options = {
-          :action => "Retrieve git repository",
-          :userid => current_user.userid
-        }
-        queue_options = {
-          :class_name  => "GitRepository",
-          :method_name => "refresh",
-          :instance_id => git_repo_id,
-          :role        => "git_owner",
-          :args        => []
-        }
-
-        task_id = MiqTask.generic_action_with_callback(task_options, queue_options)
+        task_id = git_based_domain_import_service.queue_refresh(git_repo_id)
         response_json = {:task_id => task_id, :git_repo_id => git_repo_id, :new_git_repo => new_git_repo}
       rescue => err
         add_flash(_("Error during repository setup: %{error_message}") % {:error_message => err.message}, :error)
diff --git a/spec/controllers/miq_ae_tools_controller_spec.rb b/spec/controllers/miq_ae_tools_controller_spec.rb
index e8ed54c456b..ae8fd6742b0 100644
--- a/spec/controllers/miq_ae_tools_controller_spec.rb
+++ b/spec/controllers/miq_ae_tools_controller_spec.rb
@@ -326,16 +326,7 @@
 
         context "when everything works fine" do
           let(:git_repo) { double("GitRepository", :id => 123) }
-          let(:task_options) { {:action => "Retrieve git repository", :userid => controller.current_user.userid} }
-          let(:queue_options) do
-            {
-              :class_name  => "GitRepository",
-              :method_name => "refresh",
-              :instance_id => 123,
-              :role        => "git_owner",
-              :args        => []
-            }
-          end
+          let(:git_based_domain_import_service) { double("GitBasedDomainImportService") }
 
           before do
             allow(git_repository_service).to receive(:setup).with(
@@ -344,7 +335,8 @@
               "gitpassword",
               "gitverifyssl"
             ).and_return({:git_repo_id => git_repo.id, :new_git_repo? => false})
-            allow(MiqTask).to receive(:generic_action_with_callback).with(task_options, queue_options).and_return(321)
+            allow(GitBasedDomainImportService).to receive(:new).and_return(git_based_domain_import_service)
+            allow(git_based_domain_import_service).to receive(:queue_refresh).with(123).and_return(321)
           end
 
           it "responds with task information" do

From dbff1b629627610e45ed00d9f7bc9728ba6205e4 Mon Sep 17 00:00:00 2001
From: Erik Clarizio <eclarizi@redhat.com>
Date: Fri, 2 Dec 2016 14:15:10 -0800
Subject: [PATCH 5/5] Add constant for the poll timeout value and fix quotes

https://bugzilla.redhat.com/show_bug.cgi?id=1393982
---
 app/assets/javascripts/git_import.js | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/app/assets/javascripts/git_import.js b/app/assets/javascripts/git_import.js
index 3012b0422eb..85ae276d8a0 100644
--- a/app/assets/javascripts/git_import.js
+++ b/app/assets/javascripts/git_import.js
@@ -1,6 +1,8 @@
 /* global miqSparkleOn miqSparkleOff showErrorMessage clearMessages */
 
 var GitImport = {
+  TASK_POLL_TIMEOUT: 1500,
+
   retrieveDatastoreClickHandler: function() {
     $('.git-retrieve-datastore').click(function(event) {
       event.preventDefault();
@@ -10,7 +12,7 @@ var GitImport = {
       $.post('retrieve_git_datastore', $('#retrieve-git-datastore-form').serialize(), function(data) {
         var parsedData = JSON.parse(data);
         var messages = parsedData.message;
-        if (messages && messages.level === "error") {
+        if (messages && messages.level === 'error') {
           showErrorMessage(messages.message);
           miqSparkleOff();
         } else {
@@ -24,7 +26,7 @@ var GitImport = {
     $.get('check_git_task', gitData, function(data) {
       var parsedData = JSON.parse(data);
       if (parsedData.state) {
-        setTimeout(GitImport.pollForGitTaskCompletion, 1500, gitData);
+        setTimeout(GitImport.pollForGitTaskCompletion, GitImport.TASK_POLL_TIMEOUT, gitData);
       } else {
         GitImport.gitTaskCompleted(parsedData);
       }