Fixes #28287 - Import Ansible roles and variables in one step

This PR Includes:
- The ability to sync roles and their variables from Capsule in one step
- React table to present all the roles and variables that need to be synced.
- Removing the 'import variables' view from the index variables page.

app/controllers/ansible_roles_controller.rb

@@ -4,7 +4,7 @@
 class AnsibleRolesController < ::ApplicationController
   include Foreman::Controller::AutoCompleteSearch
   include ForemanAnsible::Concerns::ImportControllerHelper
-
+  include ::ForemanAnsible::AnsibleRolesDataPreparations
   def index
     @ansible_roles = resource_base.search_for(params[:search],
                                               :order => params[:order]).
@@ -22,18 +22,19 @@ def destroy
 
   def import
     changed = @importer.import!
-    if changed.values.all?(&:empty?)
+    @rows = prepare_ansible_import_rows(changed, @variables_importer)
+    if @rows.empty?
       success no_changed_roles_message
       redirect_to ansible_roles_path
     else
-      render :locals => { :changed => changed }
+      render
     end
   end
 
   def confirm_import
     @importer.finish_import(params[:changed]&.to_unsafe_h)
-    success _('Import of roles successfully finished.')
-    redirect_to ansible_roles_path
+    @variables_importer.import_variables_roles(params[:changed]) if params[:changed]['new'] || params[:changed]['old']
+    success _('Import of roles and their variables was successfully finished.')
   end
 
   private
@@ -44,10 +45,11 @@ def default_order
 
   def create_importer
     @importer = ForemanAnsible::UiRolesImporter.new(@proxy)
+    @variables_importer = ForemanAnsible::VariablesImporter.new(@proxy)
   end
 
   def no_changed_roles_message
-    return _('No changes in roles detected.') if @proxy.blank?
-    _('No changes in roles detected on %s.') % @proxy.name
+    return _('No added or removed roles nor variables.') if @proxy.blank?
+    _('No added or removed roles nor variables detected on %s.') % @proxy.name
   end
 end

app/controllers/foreman_ansible/concerns/import_controller_helper.rb

@@ -9,8 +9,8 @@ module ImportControllerHelper
       included do
         # rubocop:disable Rails/LexicallyScopedActionFilter
         before_action :find_resource, :only => [:destroy]
-        before_action :find_proxy, :only => [:import]
-        before_action :create_importer, :only => [:import, :confirm_import]
+        before_action :find_proxy, :only => [:import, :import_variables, :confirm_import]
+        before_action :create_importer, :only => [:import, :confirm_import, :import_variables, :confirm_import]
         before_action :default_order, :only => [:index]
         # rubocop:enable Rails/LexicallyScopedActionFilter
       end

app/helpers/foreman_ansible/ansible_roles_data_preparations.rb

@@ -0,0 +1,65 @@
+# frozen_string_literal: true
+
+module ForemanAnsible
+  module AnsibleRolesDataPreparations
+    VARIABLE_ACTION_NAMES = { 'new' => N_('Add'), 'obsolete' => N_('Remove'), 'update' => N_('Update') }.freeze
+    ROLE_ACTION_NAMES = { 'new' => N_('Import Role'), 'obsolete' => N_('Remove Role'), 'old' => N_('Update Role Variables') }.freeze
+
+    def get_variable_action(kind)
+      _(VARIABLE_ACTION_NAMES[kind])
+    end
+
+    def get_role_action(kind)
+      _(ROLE_ACTION_NAMES[kind])
+    end
+
+    def get_old_roles_variables(imported_variables, role)
+      variables = { 'Add' => [], 'Remove' => [], 'Update' => [] }
+      imported_variables.each do |kind, temp_variables|
+        temp_variables.each do |temp_variable|
+          variables[get_variable_action(kind)].append(temp_variable.key) if temp_variable.ansible_role_id == role.id
+        end
+      end
+      variables
+    end
+
+    def variables_to_s(variables)
+      str = ''
+      variables.each do |action, temp_variables|
+        str += "#{action}: #{temp_variables.size}, " unless temp_variables.empty?
+      end
+      str[0..-3]
+    end
+
+    def get_roles_variables(imported_variables, variables_importer, kind, role)
+      if kind == 'new'
+        variables = { 'Add' => variables_importer.get_variables_names(role.name) }
+      elsif kind == 'obsolete'
+        variables = { 'Remove' => role.ansible_variables.map(&:key) }
+      elsif kind == 'old'
+        variables = get_old_roles_variables(imported_variables, role)
+      end
+      variables_to_s(variables)
+    end
+
+    def prepare_ansible_import_rows(changed, variables_importer)
+      rows = []
+      changed.each do |kind, roles|
+        imported_variables = variables_importer.import_variable_names(roles)
+        roles.each do |role|
+          role_action = get_role_action(kind)
+          variables = get_roles_variables(imported_variables, variables_importer, kind, role)
+          next if variables.empty? && kind['old']
+          rows.append({ cells: [
+                        role.name,
+                        role_action, variables,
+                        role_action == 'Remove Role' ? role.hosts.count : '',
+                        role_action == 'Remove Role' ? role.hostgroups.count : ''
+                      ],
+                        role: role, kind: kind, id: role.name })
+        end
+      end
+      rows
+    end
+  end
+end

app/services/foreman_ansible/roles_importer.rb

@@ -7,6 +7,7 @@ class RolesImporter
 
     def initialize(proxy = nil)
       @ansible_proxy = proxy
+      @variables_importer = ForemanAnsible::VariablesImporter.new(@ansible_proxy)
     end
 
     def import_role_names
@@ -26,8 +27,8 @@ def import_roles(roles)
 
     def detect_changes(imported)
       changes = {}.with_indifferent_access
-      old, changes[:new] = imported.partition { |role| role.id.present? }
-      changes[:obsolete] = ::AnsibleRole.where.not(:id => old.map(&:id))
+      changes[:old], changes[:new] = imported.partition { |role| role.id.present? }
+      changes[:obsolete] = ::AnsibleRole.where.not(:id => changes[:old].map(&:id))
       changes
     end
 

app/services/foreman_ansible/ui_roles_importer.rb

@@ -15,13 +15,13 @@ def finish_import(changes)
 
     def create_new_roles(changes)
       changes.each_pair do |_, new_role|
-        ::AnsibleRole.create(JSON.parse(new_role))
+        ::AnsibleRole.create(new_role)
       end
     end
 
     def delete_old_roles(changes)
       changes.each_pair do |_, old_role|
-        ::AnsibleRole.find(JSON.parse(old_role)['id']).destroy
+        ::AnsibleRole.find(old_role['id']).destroy
       end
     end
   end

app/services/foreman_ansible/variables_importer.rb

@@ -26,6 +26,10 @@ def import_variable_names(new_roles)
       import_variables(local_variables, new_roles)
     end
 
+    def get_variables_names(role_name)
+      remote_variables[role_name]
+    end
+
     def import_variables(role_variables, new_roles)
       detect_changes(
         role_variables.map do |role_name, variables|
@@ -37,6 +41,34 @@ def import_variables(role_variables, new_roles)
       )
     end
 
+    def import_variables_roles(roles)
+      if roles['new']
+        imported_roles = roles['new'].keys
+        variables_to_import = {}
+        remote_variables.each do |role, variables|
+          next unless imported_roles.include? role
+          role_obj = AnsibleRole.find_by(:name => role)
+          variables_to_import[role] = {}
+          values = initialize_variables(variables, role_obj)
+          values.each do |value|
+            variables_to_import[role][value.key] = value.attributes.to_json
+          end
+        end
+        create_new_variables(variables_to_import)
+      end
+      return if roles['old'].empty?
+      variables_to_update = { 'new' => {}, 'obsolete' => {}, 'update' => {} }
+      import_variable_names([]).each do |kind, variables|
+        variables.each do |variable|
+          next unless roles['old'].values.map { |role| role['id'] }.include?(variable.ansible_role_id)
+          role_name = variable.ansible_role.name
+          variables_to_update[kind][role_name] ||= {}
+          variables_to_update[kind][role_name][variable.key] = variable.attributes.to_json
+        end
+      end
+      finish_import(variables_to_update['new'], variables_to_update['obsolete'], variables_to_update['update'])
+    end
+
     def import_new_role(role_name, new_roles)
       role = AnsibleRole.find_by(:name => role_name)
       if role.blank? && new_roles.map(&:name).include?(role_name)
@@ -92,8 +124,6 @@ def create_new_variables(variables)
     def update_variables(variables)
       iterate_over_variables(variables) do |_role, memo, attrs|
         attributes = JSON.parse(attrs)
-        # Make sure to let the flag if a variable is hidden untouched
-        attributes.delete('hidden_value')
         var = AnsibleVariable.find attributes['id']
         var.update(attributes)
         memo << var

app/views/ansible_roles/import.html.erb

@@ -1,51 +1,11 @@
 <% title _("Changed Ansible roles") %>
-<%= form_tag confirm_import_ansible_roles_path do %>
-  <h4><%= _("Select the changes you want to realize in Foreman") %></h4>
-  <h6>
-  <%= _("Toggle") %>:
-  <%= link_to_function(icon_text("check", _("New")),
-                       "toggleCheckboxesBySelector('.role_select_boxes_new')",
-                       :title => _("Check/Uncheck new")) %> |
-  <%= link_to_function(icon_text("check", _("Obsolete")),
-                       "toggleCheckboxesBySelector('.role_select_boxes_obsolete')",
-                       :title => _("Check/Uncheck obsolete")) %>
-  </h6>
-  <table class="<%= table_css_classes %>">
-    <thead>
-      <tr>
-        <th class="ca">
-          <%= link_to_function(icon_text("check"),
-                               "toggleCheckboxesBySelector('.role_select_boxes')",
-                               :title => _("Check/Uncheck all")) %>
-        </th>
-        <th><%= _("Name") %></th>
-        <th class="col-md-2"><%= _("Hosts count") %></th>
-        <th class="col-md-2"><%= _("Hostgroups count") %></th>
-        <th><%= _("Operation") %></th>
-      </tr>
-    </thead>
-    <tbody>
-      <% changed.each do |kind, roles| %>
-        <% roles.each do |role| %>
-          <tr>
-            <td>
-              <%= check_box_tag "changed[#{kind}][#{role}]", role.to_json, false, :class => "role_select_boxes role_select_boxes_#{kind} role_select_boxes_role_#{role}" %>
-            </td>
-            <td>
-              <%= link_to_function("#{role}", "toggleCheckboxesBySelector('.role_select_boxes_role_#{role}')", :title => _("Check/Uncheck all %s changes") % role) %>
-            </td>
-            <td><%= role.hosts.count %></td>
-            <td><%= role.hostgroups.count %></td>
-            <td>
-              <%= { "new" => _("Add"), "obsolete" => _("Remove") }[kind] %>
-            </td>
-          </tr>
-        <% end %>
-      <% end %>
-    </tbody>
-  </table>
-  <div>
-    <%= link_to _("Cancel"), ansible_roles_path, :class => "btn btn-default" %>
-    <%= submit_tag _("Update"), :class => "btn btn-primary" %>
-  </div>
-<% end %>
+<%= webpacked_plugins_js_for :foreman_ansible %>
+<%= webpacked_plugins_css_for :foreman_ansible %>
+
+
+<%= react_component(
+        'WrappedImportRolesAndVariables',
+        :rowsData => @rows,
+        :proxy => @proxy.id
+    ) %>
+

app/views/ansible_variables/index.html.erb

@@ -1,8 +1,7 @@
 <% title _("Ansible Variables") %>
 <%= stylesheet 'foreman_ansible/foreman-ansible' %>
 
-<%= title_actions ansible_proxy_import(hash_for_import_ansible_variables_path),
-      display_link_if_authorized(_('New Ansible Variable'), hash_for_new_ansible_variable_path, :class => "btn btn-default no-float"),
+<%= title_actions display_link_if_authorized(_('New Ansible Variable'), hash_for_new_ansible_variable_path, :class => "btn btn-default no-float"),
       documentation_button('#4.3Variables', :root_url => ansible_doc_url)
        %>
 

test/functional/ansible_variables_controller_test.rb

@@ -34,21 +34,6 @@ class AnsibleVariablesControllerTest < ActionController::TestCase
     assert_redirected_to ansible_variables_url
   end
 
-  test 'should show import page' do
-    ForemanAnsible::UiRolesImporter.any_instance.
-      expects(:import_role_names).returns({})
-
-    ForemanAnsible::VariablesImporter.any_instance.
-      expects(:import_variable_names).returns({
-        :obsolete => [@model]
-      })
-
-    get :import,
-        :params => { :proxy => @proxy.id },
-        :session => set_session_user
-    assert_response :success
-  end
-
   test 'should create ansible variable' do
     params = { :ansible_variable => { :key => 'great name', :ansible_role_id => FactoryBot.create(:ansible_role).id } }
     assert_difference('AnsibleVariable.count', 1) do