Skip to content

Commit e0918b7

Browse files
jeremylenzclaude
andcommitted
Add TaskProgress components and modernize inventory upload UI
This commit introduces a modernized UI for inventory upload tasks using PatternFly components and removes obsolete code from the previous implementation. **New Components:** - TaskProgress: Shows current task state with progress bar, status, and duration - TaskHistory: Displays historical task results in a clean list format - TasksController API: Provides endpoints for current and historical task data **UI Improvements:** - Replace ProgressOutput-based status tracking with ForemanTasks directly - Add accessibility features (ouiaId properties on PatternFly components) - Use Patternfly Card, Progress, and DataList components - Simplify Dashboard to use NavContainer with TaskProgress components **Backend Changes:** - Add API endpoints for task data: /api/v2/rh_cloud/inventory_upload/tasks/{current,history} - Update AccountsController to return task objects instead of status strings - Maintain backward compatibility with status strings for gradual migration **Code Cleanup:** - Remove obsolete DashboardActions, DashboardSelectors, and related Redux code - Delete unused ProgressOutput registration and instance_label logic - Remove unnecessary activeTab state management - Clean up test files for deleted functionality **Test Updates:** - Update AccountsControllerTest to match new API structure - Remove tests for deleted ProgressOutput functionality - Add new API routes to permissions test exceptions - All tests passing: 363 tests, 1203 assertions, 0 failures This change is part of a larger UI redesign effort to improve the user experience and maintainability of the inventory upload feature. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 79d98e4 commit e0918b7

File tree

70 files changed

+961
-1699
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

70 files changed

+961
-1699
lines changed

app/controllers/concerns/inventory_upload/report_actions.rb

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,14 @@ def initialize(**params)
1111
end
1212

1313
def start_report_generation(organization_id, disconnected)
14-
ForemanTasks.async_task(ForemanInventoryUpload::Async::GenerateReportJob, ForemanInventoryUpload.generated_reports_folder, organization_id, disconnected)
14+
upload = !disconnected
15+
ForemanTasks.async_task(
16+
ForemanInventoryUpload::Async::HostInventoryReportJob,
17+
ForemanInventoryUpload.generated_reports_folder,
18+
organization_id,
19+
"", # hosts_filter
20+
upload
21+
)
1522
end
1623

1724
def report_file(organization_id)

app/controllers/foreman_inventory_upload/accounts_controller.rb

Lines changed: 92 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,15 +6,20 @@ def index
66

77
accounts = Hash[
88
labels.map do |id, label|
9-
generate_report_status = status_for(id, ForemanInventoryUpload::Async::GenerateReportJob)
10-
upload_report_status = status_for(id, ForemanInventoryUpload::Async::UploadReportDirectJob)
9+
generate_task = latest_task_for(id, ForemanInventoryUpload::Async::HostInventoryReportJob)
10+
11+
# Check sub-action completion status
12+
generated_status = sub_action_status(generate_task, 'GenerateHostReport')
13+
uploaded_status = sub_action_status(generate_task, 'UploadReportDirectJob')
14+
1115
report_file_paths = ForemanInventoryUpload.report_file_paths(id)
1216

1317
[
1418
label,
1519
{
16-
generate_report_status: generate_report_status,
17-
upload_report_status: upload_report_status,
20+
generated_status: generated_status,
21+
uploaded_status: uploaded_status,
22+
generate_task: task_json(generate_task),
1823
report_file_paths: report_file_paths,
1924
id: id,
2025
},
@@ -30,9 +35,89 @@ def index
3035

3136
private
3237

33-
def status_for(label, job_class)
34-
label = job_class.output_label(label)
35-
ForemanInventoryUpload::Async::ProgressOutput.get(label)&.status
38+
def latest_task_for(org_id, job_class)
39+
ForemanTasks::Task
40+
.for_action_types([job_class.name])
41+
.joins(:links)
42+
.where(foreman_tasks_links: {
43+
resource_type: 'Organization',
44+
resource_id: org_id,
45+
})
46+
.with_duration
47+
.order('started_at DESC')
48+
.first
49+
end
50+
51+
def task_status_string(task)
52+
return nil unless task
53+
54+
if task.state == 'stopped'
55+
# Mimic old ProgressOutput format: "pid 12345 exit 0"
56+
exit_code = task.result == 'success' ? 0 : 1
57+
"pid #{Process.pid} exit #{exit_code}"
58+
else
59+
task.state
60+
end
61+
end
62+
63+
def sub_action_status(task, action_class_name)
64+
return nil unless task
65+
66+
# If task is still running, return the state
67+
return task.state unless task.state == 'stopped'
68+
69+
# For GenerateHostReport: always show status if task completed (generation always runs)
70+
if action_class_name == 'GenerateHostReport'
71+
exit_code = task.result == 'success' ? 0 : 1
72+
return "pid #{Process.pid} exit #{exit_code}"
73+
end
74+
75+
# For UploadReportDirectJob: only show status if task had upload enabled
76+
if action_class_name == 'UploadReportDirectJob'
77+
main_action = task.main_action
78+
return nil unless main_action.respond_to?(:input)
79+
80+
# Check if upload was enabled for this task
81+
return nil unless main_action.input[:upload]
82+
83+
# Show same status as overall task (upload runs as part of the task)
84+
exit_code = task.result == 'success' ? 0 : 1
85+
return "pid #{Process.pid} exit #{exit_code}"
86+
end
87+
88+
nil
89+
rescue StandardError => e
90+
Rails.logger.warn("Failed to get sub-action status for #{action_class_name} in task #{task.id}: #{e.message}")
91+
nil
92+
end
93+
94+
def task_json(task)
95+
return nil unless task
96+
97+
{
98+
id: task.id,
99+
label: task.label,
100+
state: task.state,
101+
result: task.result,
102+
progress: task.progress,
103+
started_at: task.started_at,
104+
ended_at: task.ended_at,
105+
duration: task.try(:duration)&.to_f,
106+
report_file_path: task_report_file_path(task),
107+
}
108+
end
109+
110+
def task_report_file_path(task)
111+
return nil unless task&.state == 'stopped'
112+
113+
# Get the main action from the task
114+
main_action = task.main_action
115+
return nil unless main_action.respond_to?(:report_file_path)
116+
117+
main_action.report_file_path
118+
rescue StandardError => e
119+
Rails.logger.warn("Failed to get report file path for task #{task.id}: #{e.message}")
120+
nil
36121
end
37122
end
38123
end
Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
module ForemanInventoryUpload
2+
module Api
3+
class TasksController < ::Api::V2::BaseController
4+
# GET /foreman_inventory_upload/api/tasks/current
5+
# Returns current running/active tasks for inventory operations
6+
def current
7+
organization_id = params[:organization_id]
8+
action_types = [
9+
'ForemanInventoryUpload::Async::GenerateReportJob',
10+
'ForemanInventoryUpload::Async::UploadReportDirectJob',
11+
]
12+
13+
tasks = ForemanTasks::Task
14+
.active
15+
.for_action_types(action_types)
16+
.with_duration
17+
18+
if organization_id.present?
19+
tasks = tasks.joins(:links)
20+
.where(foreman_tasks_links: {
21+
resource_type: 'Organization',
22+
resource_id: organization_id,
23+
})
24+
end
25+
26+
render json: {
27+
tasks: tasks.map { |task| task_json(task) },
28+
}
29+
end
30+
31+
# GET /foreman_inventory_upload/api/tasks/history
32+
# Returns recent task history for inventory operations
33+
def history
34+
organization_id = params[:organization_id]
35+
limit = params[:limit]&.to_i || 10
36+
action_types = [
37+
'ForemanInventoryUpload::Async::GenerateReportJob',
38+
'ForemanInventoryUpload::Async::UploadReportDirectJob',
39+
]
40+
41+
tasks = ForemanTasks::Task
42+
.for_action_types(action_types)
43+
.with_duration
44+
.order('started_at DESC')
45+
.limit(limit)
46+
47+
if organization_id.present?
48+
tasks = tasks.joins(:links)
49+
.where(foreman_tasks_links: {
50+
resource_type: 'Organization',
51+
resource_id: organization_id,
52+
})
53+
end
54+
55+
render json: {
56+
tasks: tasks.map { |task| task_json(task) },
57+
}
58+
end
59+
60+
private
61+
62+
def task_json(task)
63+
{
64+
id: task.id,
65+
label: task.label,
66+
action: task.action,
67+
state: task.state,
68+
result: task.result,
69+
progress: task.progress,
70+
started_at: task.started_at,
71+
ended_at: task.ended_at,
72+
duration: task.try(:duration),
73+
humanized: task.humanized,
74+
}
75+
end
76+
end
77+
end
78+
end

app/controllers/foreman_inventory_upload/reports_controller.rb

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,13 @@ def generate
2323
organization_id = params[:organization_id]
2424
disconnected = params[:disconnected]
2525

26-
start_report_generation(organization_id, disconnected)
26+
task = start_report_generation(organization_id, disconnected)
2727

2828
render json: {
29-
action_status: 'success',
29+
id: task.id,
30+
humanized: {
31+
action: task.action,
32+
},
3033
}, status: :ok
3134
end
3235
end

config/routes.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,10 @@
7676
post 'enable_connector', to: 'inventory#enable_cloud_connector'
7777
post 'cloud_request', to: 'cloud_request#update'
7878
get 'advisor_engine_config', to: 'advisor_engine_config#show'
79+
80+
# Inventory upload task endpoints
81+
get 'inventory_upload/tasks/current', to: 'foreman_inventory_upload/api/tasks#current'
82+
get 'inventory_upload/tasks/history', to: 'foreman_inventory_upload/api/tasks#history'
7983
end
8084

8185
namespace 'advisor_engine' do

lib/foreman_inventory_upload/async/host_inventory_report_job.rb

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,21 @@
11
module ForemanInventoryUpload
22
module Async
33
class HostInventoryReportJob < ::Actions::EntryAction
4+
def resource_locks
5+
:link
6+
end
7+
48
def plan(base_folder, organization_id, hosts_filter = "", upload = true)
9+
organization = Organization.find(organization_id)
10+
action_subject(organization)
11+
12+
plan_self(
13+
base_folder: base_folder,
14+
organization_id: organization_id,
15+
hosts_filter: hosts_filter,
16+
upload: upload
17+
)
18+
519
sequence do
620
plan_action(
721
GenerateHostReport,
@@ -31,9 +45,34 @@ def humanized_name
3145
_("Host inventory report job")
3246
end
3347

48+
def organization
49+
Organization.find(input[:organization_id])
50+
end
51+
3452
def organization_id
3553
input[:organization_id]
3654
end
55+
56+
def report_file_path
57+
filename = ForemanInventoryUpload.facts_archive_name(input[:organization_id], input[:hosts_filter])
58+
59+
if input[:upload]
60+
# For upload tasks, check: done folder (uploaded), uploads folder (queued), generated_reports folder (failed upload)
61+
[
62+
ForemanInventoryUpload.done_file_path(filename),
63+
ForemanInventoryUpload.uploads_file_path(filename),
64+
File.join(input[:base_folder], filename),
65+
].find { |path| File.exist?(path) }
66+
else
67+
# For generate-only tasks, only check generated_reports folder
68+
generated_path = File.join(input[:base_folder], filename)
69+
File.exist?(generated_path) ? generated_path : nil
70+
end
71+
end
72+
73+
def rescue_strategy_for_self
74+
Dynflow::Action::Rescue::Fail
75+
end
3776
end
3877
end
3978
end

lib/foreman_inventory_upload/async/upload_report_direct_job.rb

Lines changed: 3 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -43,69 +43,34 @@ def self.output_label(label)
4343
end
4444

4545
def plan(filename, organization_id)
46-
# NOTE: This implementation assumes a single organization will not trigger multiple
47-
# concurrent uploads. The instance_label is derived from organization_id alone, which
48-
# means concurrent uploads for the same org would share ProgressOutput storage.
49-
# This matches the pattern in GenerateReportJob. A full fix for thread-safety
50-
# requires UI changes to display multiple concurrent tasks per org (tracked for PR #2).
51-
label = UploadReportDirectJob.output_label(organization_id)
52-
clear_task_output(label)
5346
plan_self(
54-
instance_label: label,
5547
filename: filename,
5648
organization_id: organization_id
5749
)
5850
end
5951

6052
def try_execute
6153
if content_disconnected?
62-
progress_output do |progress_output|
63-
progress_output.write_line("Report was not moved and upload was canceled because connection to Insights is not enabled. Report location: #{filename}.")
64-
progress_output.status = "Task aborted, exit 1"
65-
done!
66-
end
54+
logger.info("Upload canceled: connection to Insights is not enabled. Report location: #{filename}")
6755
return
6856
end
6957

7058
unless organization.owner_details&.dig('upstreamConsumer', 'idCert')
7159
logger.info("Skipping organization '#{organization}', no candlepin certificate defined.")
72-
progress_output do |progress_output|
73-
progress_output.write_line("Skipping organization #{organization}, no candlepin certificate defined.")
74-
progress_output.status = "Task aborted, exit 1"
75-
done!
76-
end
7760
return
7861
end
7962

8063
Tempfile.create([organization.name, '.pem']) do |cer_file|
8164
cer_file.write(certificate[:cert])
8265
cer_file.write(certificate[:key])
8366
cer_file.flush
84-
upload_report(cer_file.path)
67+
upload_file(cer_file.path)
8568
end
8669

70+
move_to_done_folder
8771
done!
8872
end
8973

90-
def upload_report(cer_path)
91-
progress_output do |progress_output|
92-
progress_output.write_line("Uploading report for organization #{organization.label}...")
93-
progress_output.status = "Running upload"
94-
95-
begin
96-
upload_file(cer_path)
97-
progress_output.write_line("Upload completed successfully")
98-
move_to_done_folder
99-
progress_output.write_line("Uploaded file moved to done/ folder")
100-
progress_output.status = "pid #{Process.pid} exit 0"
101-
rescue StandardError => e
102-
progress_output.write_line("Upload failed: #{e.message}")
103-
progress_output.status = "pid #{Process.pid} exit 1"
104-
raise
105-
end
106-
end
107-
end
108-
10974
def upload_file(cer_path)
11075
cert_content = File.read(cer_path)
11176

@@ -172,29 +137,13 @@ def content_disconnected?
172137
!Setting[:subscription_connection_enabled]
173138
end
174139

175-
def progress_output
176-
progress_output = ProgressOutput.register(instance_label)
177-
yield(progress_output)
178-
ensure
179-
progress_output.close
180-
end
181-
182-
def instance_label
183-
input[:instance_label]
184-
end
185-
186140
def logger
187141
Foreman::Logging.logger('background')
188142
end
189143

190144
def rescue_strategy_for_self
191145
Dynflow::Action::Rescue::Fail
192146
end
193-
194-
def clear_task_output(label)
195-
TaskOutputLine.where(label: label).delete_all
196-
TaskOutputStatus.where(label: label).delete_all
197-
end
198147
end
199148
end
200149
end

0 commit comments

Comments
 (0)