Add TaskProgress components and remove obsolete UI

[jeremylenz]

Inventory Upload Page Redesign - PR 2 of 3

User Impact:
This PR removes the old terminal-style progress output UI and replaces it with modern PatternFly components that show task progress, status, and history in a clear, user-friendly format.

What Changed:

• New TaskProgress component displays current task state with progress bars and status
• New TaskHistory component shows recent task results in a clean list
• Dashboard simplified to use these new components
• Backend now queries ForemanTasks directly instead of using ProgressOutput intermediary
• All obsolete UI code, Redux state, and test files removed

Technical Details:

• Added API endpoints: /api/v2/rh_cloud/inventory_upload/tasks/{current,history}
• AccountsController returns task objects with backward-compatible status strings
• All 363 tests passing

🤖 Generated with Claude Code

Co-Authored-By: Claude [email protected]

[sourcery-ai]

Reviewer's Guide

This PR modernizes the inventory upload UI by replacing the legacy ProgressOutput-based Dashboard with new PatternFly TaskProgress and TaskHistory components, integrates with ForemanTasks API via new controller endpoints, refactors the asynchronous upload job to remove obsolete ProgressOutput logic, and cleans up associated Redux, controller, and test code.

Sequence diagram for fetching current and historical task data

sequenceDiagram
  actor User
  participant UI as Dashboard UI
  participant API as TasksController API
  participant DB as ForemanTasks::Task
  User->>UI: Open inventory upload dashboard
  UI->>API: GET /api/v2/rh_cloud/inventory_upload/tasks/current
  API->>DB: Query active ForemanTasks
  DB-->>API: Return active tasks
  API-->>UI: Return current tasks JSON
  UI->>API: GET /api/v2/rh_cloud/inventory_upload/tasks/history
  API->>DB: Query recent ForemanTasks
  DB-->>API: Return historical tasks
  API-->>UI: Return historical tasks JSON
  UI-->>User: Render TaskProgress and TaskHistory components

Loading

Entity relationship diagram for Account and Task data

erDiagram
  ACCOUNT ||--o{ TASK : has
  ACCOUNT {
    id string
    generate_task_id string
    upload_task_id string
    report_file_paths string[]
  }
  TASK {
    id string
    label string
    action string
    state string
    result string
    progress number
    started_at string
    ended_at string
    duration number
    humanized string
  }

Loading

Class diagram for new and updated UI components

classDiagram
  class TaskProgress {
    +task: Task
    +title: string
    +emptyMessage: string
    +render()
  }
  class TaskHistory {
    +tasks: Task[]
    +title: string
    +render()
  }
  class NavContainer {
    +items: Array
    +render()
  }
  class Dashboard {
    +account: Account
    +render()
  }
  class Task {
    +id: string
    +state: string
    +result: string
    +progress: number
    +started_at: string
    +ended_at: string
    +duration: number
  }
  class Account {
    +generate_task: Task
    +upload_task: Task
    +report_file_paths: string[]
    +id: string
  }
  Dashboard --> NavContainer
  NavContainer --> TaskProgress
  NavContainer --> TaskHistory
  Account --> Task

Loading

Class diagram for new TasksController API endpoints

classDiagram
  class TasksController {
    +current()
    +history()
    -task_json(task)
  }
  class ForemanTasks_Task {
    +id: string
    +label: string
    +action: string
    +state: string
    +result: string
    +progress: number
    +started_at: string
    +ended_at: string
    +duration: number
    +humanized: string
  }
  TasksController --> ForemanTasks_Task

Loading

Class diagram for refactored UploadReportDirectJob

classDiagram
  class UploadReportDirectJob {
    +plan(filename, organization_id)
    +try_execute()
    +upload_file(cer_path)
    +move_to_done_folder()
    +logger()
    +rescue_strategy_for_self()
  }
  class DynflowAction
  UploadReportDirectJob --|> DynflowAction
  UploadReportDirectJob ..> logger
  UploadReportDirectJob ..> move_to_done_folder

Loading

File-Level Changes

Change	Details	Files
Modernize Dashboard and remove obsolete UI components and Redux logic	Replaced Dashboard class component with a functional version using TaskProgress Removed ReportGenerate, ReportUpload, FullScreenModal, and related Redux actions/selectors/reducer Simplified NavContainer to drop full-screen terminal and obsolete props	webpack/ForemanInventoryUpload/Components/Dashboard/Dashboard.js webpack/ForemanInventoryUpload/Components/Dashboard/index.js webpack/ForemanInventoryUpload/Components/NavContainer/NavContainer.js webpack/ForemanInventoryUpload/ForemanInventoryUploadReducers.js
Add PatternFly TaskProgress and TaskHistory components for current and historical tasks	Created TaskProgress component with progress bar, status labels, and details link Created TaskHistory component with DataList of past tasks and empty state Added styles for both components and index exports	webpack/ForemanInventoryUpload/Components/TaskProgress/TaskProgress.js webpack/ForemanInventoryUpload/Components/TaskProgress/taskProgress.scss webpack/ForemanInventoryUpload/Components/TaskProgress/index.js webpack/ForemanInventoryUpload/Components/TaskHistory/TaskHistory.js webpack/ForemanInventoryUpload/Components/TaskHistory/taskHistory.scss webpack/ForemanInventoryUpload/Components/TaskHistory/index.js
Integrate ForemanTasks for current and history endpoints and update AccountsController	Added TasksController with current and history JSON endpoints and routes Updated AccountsController#index to fetch latest tasks via ForemanTasks and keep backward-compatible status strings Added helper methods latest_task_for, task_status_string, and task_json	app/controllers/foreman_inventory_upload/api/tasks_controller.rb app/controllers/foreman_inventory_upload/accounts_controller.rb config/routes.rb test/unit/rh_cloud_permissions_test.rb
Refactor UploadReportDirectJob to remove ProgressOutput and instance_label logic	Removed progress_output, clear_task_output, and instance_label usage Simplified plan and try_execute to use logger and move_to_done_folder Dropped explicit upload_report wrapper in favor of upload_file direct call	lib/foreman_inventory_upload/async/upload_report_direct_job.rb
Cleanup and update tests for new task API and removed functionality	Removed obsolete ProgressOutput assertions from job tests Updated AccountsControllerTest to assert new JSON keys Added new task routes to rh_cloud permissions test	test/jobs/upload_report_direct_job_test.rb test/controllers/accounts_controller_test.rb test/unit/rh_cloud_permissions_test.rb

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey there - I've reviewed your changes - here's some feedback:

• NavContainer is now using useSelector but doesn’t import it from react-redux—please add the missing import to avoid runtime errors.
• The history endpoint accepts a raw limit param; consider validating/sanitizing it (e.g. enforce a minimum of 1 and a reasonable maximum) to prevent unintended zero or unbounded queries.
• For better scalability, think about adding pagination metadata (total count, next page token, etc.) or stronger page controls to the tasks history API rather than returning only a simple limit-bound list.

Prompt for AI Agents

Please address the comments from this code review:

## Overall Comments
- NavContainer is now using useSelector but doesn’t import it from react-redux—please add the missing import to avoid runtime errors.
- The history endpoint accepts a raw limit param; consider validating/sanitizing it (e.g. enforce a minimum of 1 and a reasonable maximum) to prevent unintended zero or unbounded queries.
- For better scalability, think about adding pagination metadata (total count, next page token, etc.) or stronger page controls to the tasks history API rather than returning only a simple limit-bound list.

## Individual Comments

### Comment 1
<location> `test/jobs/upload_report_direct_job_test.rb:229-232` </location>
<code_context>
       action.send(:try_execute)
     end
-
-    # Verify progress output shows error
-    label = ForemanInventoryUpload::Async::UploadReportDirectJob.output_label(@organization.id)
-    output = ForemanInventoryUpload::Async::ProgressOutput.get(label).full_output
-    assert_match(/Upload failed/, output)
   end

</code_context>

<issue_to_address>
**suggestion (testing):** Removed assertions for progress output error reporting.

Please ensure tests verify that errors are properly captured in the updated task result and status fields, particularly for failed uploads and timeouts.
</issue_to_address>

### Comment 2
<location> `test/jobs/upload_report_direct_job_test.rb:314-317` </location>
<code_context>
       # Verify file still exists (not moved or deleted)
       assert File.exist?(test_file), "File should remain when upload is aborted"
-
-      # Verify progress output mentions missing certificate
-      label = ForemanInventoryUpload::Async::UploadReportDirectJob.output_label(@organization.id)
-      output = ForemanInventoryUpload::Async::ProgressOutput.get(label).full_output
-      assert_match(/Skipping organization.*no candlepin certificate/, output)
-
-      # Verify status indicates abortion
</code_context>

<issue_to_address>
**suggestion (testing):** Removed test for progress output when certificate is missing.

Add a test to verify that missing certificates result in the correct task status or result, ensuring error handling is properly validated.

Suggested implementation:

```ruby
      # Verify file still exists (not moved or deleted)
      assert File.exist?(test_file), "File should remain when upload is aborted"

      # Verify that missing certificates result in the correct task status or result
      job = ForemanInventoryUpload::Async::UploadReportDirectJob.new
      result = job.perform(@organization.id, test_file)
      assert result.failed?, "Job should fail when certificate is missing"
      assert_match(/no candlepin certificate/, result.error_message)

```

- If `ForemanInventoryUpload::Async::UploadReportDirectJob` does not return a result object with `failed?` and `error_message`, you may need to adjust the assertions to match the actual error reporting mechanism (e.g., checking a status field, exception, or log output).
- Ensure that the test setup triggers the missing certificate scenario (e.g., by stubbing or removing the certificate for the organization).
</issue_to_address>

### Comment 3
<location> `test/controllers/accounts_controller_test.rb:11-20` </location>
<code_context>
-    generate_label = ForemanInventoryUpload::Async::GenerateReportJob.output_label(test_org.id)
</code_context>

<issue_to_address>
**suggestion (testing):** Test updated to verify new API response structure for account tasks.

Please add assertions for the state, result, and timestamps within 'generate_task' and 'upload_task' to fully validate the API response.
</issue_to_address>

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}

[qcjames53]

Hey Jeremy, nice job on this PR! The new inventory upload page seems to be really solid and /api/v2/rh_cloud/inventory_upload/tasks/current and /api/v2/rh_cloud/inventory_upload/tasks/history work great when testing manually. Big fan of how the current org is auto-populated in the search field. I have a few suggested changes / questions:

I'd prefer there to be a margin between the text and the settings window. Maybe put this text in an box clearly separated from the rest of form? I'm happy to chat about this but it seems odd to put this description front and center instead of the web form?

Are customers going to be okay with the output of the inventory status sync? Should this be a more permanent part of the page rather than a notification?

The three obfuscation settings remain enabled after toggling back to 'Analytics' from 'Minimal'. I love how minimal flips them on and disables the field. How hard would it be for them to retain their previous values when going back to Analytics though? I notice they maintain state through refresh so if this is too tricky I think it's fine to nix.

[qcjames53]

I was trying to find examples but the best I could come up with was the about page: table on the left and lots of text on the right. Could you implement something like that here? Or if not, could the text be limited to ~80ch?

[jeremylenz]

@qcjames53 Sorry, I should have been more clear that this redesign only affects the bottom half of the page - the Generate & Upload Report section. We do have mocks for a redesign of the top half (which look MUCH better than this) but that's coming at a later date. 😄

[qcjames53]

I made it through all the changed controllers. Lots of concerns about auth. For what it's worth, loading the /foreman_rh_cloud/inventory_upload webpage is allowed and just throws some 403s:

[qcjames53]

@qcjames53 Sorry, I should have been more clear that this redesign only affects the bottom half of the page - the Generate & Upload Report section. We do have mocks for a redesign of the top half (which look MUCH better than this) but that's coming at a later date. 😄

Understood, thanks :)

[qcjames53]

Hey @jeremylenz! I was thinking about this over the weekend and perhaps it would be better to fix all the foreman_rh_cloud controller authentication issues as a part of a separate bug fix? You didn't introduce any of these vulnerabilities so I don't think it should be on you to fix them. I'd still very much appreciate if you could add input validation though, if you don't mind.

[jeremylenz]

Added input validation for organization_id, limit, and disconnected parameters as requested. Also backported deletions from part3 (GenerateReportJob → HostInventoryReportJob transition) to reduce part3 PR size by 52%.

[jeremylenz]

@qcjames53 thanks for the reviews! Updated. To address your comments about unused code I pulled in some of the changes from part3 to part2.

Impact on PR sizes:

  - Part 2: Increased by +324 lines (but all deletions - easy to review)
  - Part 3: Will decrease by -464 lines (52% reduction!) 🎉

[qcjames53]

Awesome! Yeah, I forgot that might be the case for unused code since this was a multi-parter. I can probably put a few hours into this review tomorrow afternoon depending on how other things go; you can expect movement then :)

[qcjames53]

Damn this PR got hands

[jeremylenz]

@qcjames53 Updated 👍

[qcjames53]

There are a few outstanding comments that need to be addressed. The other requested changes seem good to me; all tests passing locally.

[qcjames53]

@jeremylenz I'm unable to un-resolve the five comments above but the issues are still outstanding.

[jeremylenz]

@qcjames53 Updated. I think I got 'em all this time. (either addressed or replied to.) Let me know if it works for you

[qcjames53]

Thanks! I'll take a look tomorrow!

[qcjames53]

I gave this the final once-over and the PR looks good to me! The new ForemanTasks additions, /api/v2/rh_cloud/inventory_upload/tasks/current, and /api/v2/rh_cloud/inventory_upload/tasks/history all work well and seem robust. The new components in the UI for the inventory upload dashboard looks pretty snazzy to me as well. As I understand it we're certain that page is getting cleaned up before we ship a major version.

I'm sure I missed MULTIPLE issues but we can always go back and fix bugs later if need be. We're looking solid, though. Thank you for all the work you put into this, Jeremy.

[chris1984]

Testing wise it works great, tested with subscription_connection_enabled set to false and generate and upload button is disabled. I see that Generating and Uploading tabs are no longer doing anything, are we able to remove that out of this pr or the next one? I could see users confused why nothing is showing up there. Starting to look at the code

[jeremylenz]

I see that Generating and Uploading tabs are no longer doing anything, are we able to remove that out of this pr or the next one?

Are those still there? If so they will definitely be removed in part 3, which is the followup PR that I am opening right after this is merged. Or if you prefer I can just combine them now. but I don't want to ruin @qcjames53's life

[chris1984]

I see that Generating and Uploading tabs are no longer doing anything, are we able to remove that out of this pr or the next one?

Are those still there? If so they will definitely be removed in part 3, which is the followup PR that I am opening right after this is merged. Or if you prefer I can just combine them now. but I don't want to ruin @qcjames53's life

Let's push it to part 3 that you will open up next, I don't want to throw anything new into the mix since it's already beefy

[chris1984]

Looked through all the code and most of the stuff that jumped out is what @qcjames53 commented on. I am sure I missed a few things but with this pr having hands and being beefy we can fix things in part 3 like the tabs I mentioned. Tested and works well and looks a hell of a lot better than the old page. ACK