Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #36887 - Add Schedule a Job to new host overview #845

Merged
merged 4 commits into from
Nov 22, 2023

Conversation

jeremylenz
Copy link
Contributor

@jeremylenz jeremylenz commented Nov 2, 2023

This change adds the "Schedule a Job" dropdown to the new hosts overview page.

  • Supports select all, which means specifying a host search query instead of a list of IDs
  • Disabled unless host(s) are selected

I was able to reuse most of the existing code from the host details page.

Requires theforeman/foreman#9878

image

@parthaa
Copy link
Contributor

parthaa commented Nov 2, 2023

Worked well in my test. ACK

@jeremylenz
Copy link
Contributor Author

@adamruzicka Updated so that the dropdown is hidden if you don't have create_job_invocations permission for at least one of the loaded results.

@jeremylenz
Copy link
Contributor Author

Unfortunately this now requires a new Foreman PR, since the other was just merged: theforeman/foreman#9895

@jeremylenz jeremylenz force-pushed the 36887-add-schedule-a-job branch 2 times, most recently from 5c0a3a2 to fb7f2eb Compare November 6, 2023 23:19
@jeremylenz
Copy link
Contributor Author

fixed rubocop, ready for review

@parthaa
Copy link
Contributor

parthaa commented Nov 18, 2023

What about this approach for your permissions ?

In foreman

  diff --git a/app/controllers/api/v2/base_controller.rb b/app/controllers/api/v2/base_controller.rb
index de48ea0ac..04192eb3f 100644
--- a/app/controllers/api/v2/base_controller.rb
+++ b/app/controllers/api/v2/base_controller.rb
@@ -47,7 +47,7 @@ module Api
       layout 'api/v2/layouts/index_layout', :only => :index
 
       helper_method :root_node_name, :metadata_total, :metadata_subtotal, :metadata_search,
-        :metadata_order, :metadata_by, :metadata_page, :metadata_per_page
+        :metadata_order, :metadata_by, :metadata_page, :metadata_per_page, :index_node_permissions
       def root_node_name
         @root_node_name ||= if Rabl.configuration.use_controller_name_as_json_root
                               controller_name.split('/').last
@@ -58,6 +58,13 @@ module Api
                             end
       end
 
+      def index_node_permissions
+        {
+          "can_create": helpers.can_create?,
+          "can_edit": helpers.can_edit?,
+        }
+      end
+
       def metadata_total
         @total ||= resource_scope.try(:size).to_i
       end
diff --git a/app/views/api/v2/layouts/index_layout.json.erb b/app/views/api/v2/layouts/index_layout.json.erb
index 3c056c97e..a9bfc85ec 100644
--- a/app/views/api/v2/layouts/index_layout.json.erb
+++ b/app/views/api/v2/layouts/index_layout.json.erb
@@ -5,8 +5,9 @@
   "per_page": <%= metadata_per_page.to_json %>,
   "search": <%= metadata_search.to_json.html_safe %>,
 <% if params.has_key?(:include_permissions) %>
-  "can_create": <%= can_create?.to_json %>,
-  "can_edit": <%= can_edit?.to_json %>,
+  <% index_node_permissions.each do |perm,value|  %>
+    <%= "#{perm}: #{value.to_json}," %>
+  <% end %>
 <% end %>
   "sort": {
     "by": <%= metadata_by.to_json.html_safe %>,

In ForemanRemoteExecution

From dfc5b1d17be2cf3c41f883eeb746a6644cada1db Mon Sep 17 00:00:00 2001
From: Partha Aji <[email protected]>
Date: Sat, 18 Nov 2023 10:41:16 -0500
Subject: [PATCH] more

---
 .../concerns/api/v2/hosts_controller_extensions.rb   | 12 ++++++++++++
 lib/foreman_remote_execution/engine.rb               |  1 +
 2 files changed, 13 insertions(+)
 create mode 100644 app/controllers/foreman_remote_execution/concerns/api/v2/hosts_controller_extensions.rb

diff --git a/app/controllers/foreman_remote_execution/concerns/api/v2/hosts_controller_extensions.rb b/app/controllers/foreman_remote_execution/concerns/api/v2/hosts_controller_extensions.rb
new file mode 100644
index 0000000..91004bd
--- /dev/null
+++ b/app/controllers/foreman_remote_execution/concerns/api/v2/hosts_controller_extensions.rb
@@ -0,0 +1,12 @@
+module ForemanRemoteExecution
+  module Concerns
+    module Api::V2::HostsControllerExtensions
+      extend ActiveSupport::Concern
+      def index_node_permissions
+        perms = super
+        perms[:can_foo] = true
+        perms
+      end
+    end
+  end
+end
diff --git a/lib/foreman_remote_execution/engine.rb b/lib/foreman_remote_execution/engine.rb
index b7db148..f29dbef 100644
--- a/lib/foreman_remote_execution/engine.rb
+++ b/lib/foreman_remote_execution/engine.rb
@@ -341,6 +341,7 @@ module ForemanRemoteExecution
       ::Api::V2::RegistrationController.prepend ::ForemanRemoteExecution::Concerns::Api::V2::RegistrationControllerExtensions
       ::Api::V2::RegistrationController.include ::ForemanRemoteExecution::Concerns::Api::V2::RegistrationControllerExtensions::ApipieExtensions
       ::Api::V2::RegistrationCommandsController.include ::ForemanRemoteExecution::Concerns::Api::V2::RegistrationCommandsControllerExtensions::ApipieExtensions
+      ::Api::V2::HostsController.include ::ForemanRemoteExecution::Concerns::Api::V2::HostsControllerExtensions
     end
 
     rake_tasks do
-- 
2.41.0

@jeremylenz jeremylenz force-pushed the 36887-add-schedule-a-job branch 2 times, most recently from b1514c2 to bb7d947 Compare November 20, 2023 19:08
Copy link
Contributor

@parthaa parthaa left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Works well

@jeremylenz
Copy link
Contributor Author

Tests should pass after the Foreman PR is merged, I'm hoping.

@jeremylenz
Copy link
Contributor Author

@adamruzicka does this look good to you?

@adamruzicka
Copy link
Contributor

With theforeman/foreman@93ad56156 , Katello/katello@878b5f4e86 and f5840a9 with these changes applied on top I'm getting this in the browser console and the button never shows up
image

What am I missing? Also the test failures are sort of odd, they don't seem to be related at first glance, but I don't recall seeing them in any other recent PRs

@jeremylenz
Copy link
Contributor Author

@adamruzicka I think you need the Foreman PR: #845 (comment)

@ianballou
Copy link

I tested with theforeman/foreman#9895 and was able to schedule a bulk rex job from the new hosts page. Not gonna ack since I'm new to REX :)

@jeremylenz
Copy link
Contributor Author

[test ruby]

@jeremylenz
Copy link
Contributor Author

Nice, looks like the Foreman merge fixed the tests :)

Copy link
Contributor

@adamruzicka adamruzicka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@adamruzicka adamruzicka merged commit f7e62ce into theforeman:master Nov 22, 2023
5 checks passed
@adamruzicka
Copy link
Contributor

Thank you @jeremylenz , @parthaa & @ianballou !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants