Skip to content

Conversation

@jeremylenz
Copy link
Member

@jeremylenz jeremylenz commented Dec 5, 2025

What are the changes introduced in this pull request?

This PR completes the removal of the legacy Content Hosts UI from Katello. The changes include:

First commit - Remove legacy Content Hosts UI:

  • Removed all legacy Content Hosts list and detail pages (73 files)
  • Removed AngularJS routes, controllers, and views for content-hosts module
  • Removed content-hosts specific helpers and services from plugin.rb
  • Removed concerns from HostsController that handled CSV export

Second commit - Clean up remaining legacy Content Hosts UI:

  • Removed all legacy Content Hosts UI tests (38 test files)
  • Updated links throughout the codebase to use new host details pages
  • Added backward-compatible redirects with friendly-id support:
    • /content_hosts/new/hosts (index)
    • /content_hosts/:id/new/hosts/:id (details, supports both hostname and database ID)
    • /content_hosts/:id/:tab/new/hosts/:id#/Content/:tab (with hash fragments for tabs)
  • Restored bulk action modals needed by Host Collections feature
  • Updated bulk actions to navigate to tasks.details state instead of removed content-hosts.bulk-task
  • Used Rails URL helpers (host_details_page_path) instead of hard-coded paths
  • Updated email templates to use new URLs with hash fragments

Key architectural decision:
The content-hosts AngularJS module was retained but reduced to contain only:

  • Bulk action modals (7 controllers + views) used by Host Collections
  • Helper services (ContentHostsModalHelper, ContentHostsHelper, ContentHostStatusController) required by Host Collections, Errata, and Activation Keys pages

Considerations taken when implementing this change?

  • Backward compatibility: Added redirect routes to ensure old /content_hosts URLs continue to work, with support for the friendly-id gem (both hostname and database ID work in URLs)
  • Preserving functionality: While removing the legacy UI, we preserved the bulk action modals that are still actively used by the Host Collections feature for managing multiple hosts at once
  • URL helpers: Used Rails route helpers throughout instead of hard-coding paths to ensure consistency and maintainability
  • Tab navigation: Maintained tab-specific redirects using hash fragments (e.g., #/Content/errata) to preserve deep-linking functionality
  • Incremental removal: This completes the removal process that was started in a previous commit, ensuring no orphaned code or broken references remain

What are the testing steps for this pull request?

Manual testing completed:

  1. ✅ Host Collections bulk actions (Packages, Errata, Environment, System Purpose, Module Streams, Host Collections, Subscriptions)
  2. ✅ Redirects from old /content_hosts URLs to new host details pages
  3. ✅ Redirect with database ID: /content_hosts/123/new/hosts/123
  4. ✅ Redirect with hostname: /content_hosts/host.example.com/new/hosts/host.example.com
  5. ✅ Tab redirects: /content_hosts/:id/errata/new/hosts/:id#/Content/errata
  6. ✅ Errata content hosts page (/errata/:id/content-hosts)
  7. ✅ Activation key content hosts associations
  8. ✅ Subscription hypervisor links navigate correctly
  9. ✅ Email links in errata notifications use correct format

Automated testing:

  • JavaScript linting passes
  • Existing tests should continue to pass (legacy content-hosts tests removed)

To test:

  1. Navigate to /host_collections/1 and verify all bulk action buttons work (Package Installation, Errata Installation, etc.)
  2. Try old URLs like /content_hosts/123 or /content_hosts/hostname.example.com and verify they redirect correctly
  3. Visit /errata/:id/content-hosts page and verify content hosts list displays
  4. Check activation key associations page shows content hosts correctly
  5. Verify subscription hypervisor links work in subscriptions table

Summary by Sourcery

Remove the legacy Content Hosts UI in favor of the new host details pages while preserving required bulk action functionality and URL compatibility.

New Features:

  • Add redirects from legacy /content_hosts URLs to the new host details UI, including tab-specific deep links.

Bug Fixes:

  • Ensure subscription hypervisor links and errata email links now point to the new host details pages instead of the removed legacy Content Hosts UI.

Enhancements:

  • Simplify the content-hosts AngularJS module to only provide bulk action modals and helpers used by Host Collections and related pages.
  • Update bulk actions and related flows to navigate to the generic tasks details view instead of removed legacy content-host routes.
  • Replace hard-coded content host paths across the UI with new host details URLs and Rails URL helpers.

Tests:

  • Remove obsolete tests tied to the legacy Content Hosts UI and update mailer tests to match the new host details URLs.

jeremylenz and others added 2 commits December 5, 2025 11:14
This completes the removal of the legacy Content Hosts UI by:

- Removing all legacy Content Hosts UI tests
- Updating links to use new host details pages via redirects
- Adding /content_hosts redirects for backward compatibility
  - /content_hosts -> /new/hosts (index)
  - /content_hosts/:id -> /new/hosts/:id (details)
  - /content_hosts/:id/:tab -> /new/hosts/:id with hash fragments
- Using Rails URL helpers instead of hard-coded paths
- Retaining bulk action modals needed by Host Collections
- Updating bulk actions to use tasks.details state

The content-hosts AngularJS module now only contains bulk action
functionality used by Host Collections. All Content Hosts list/detail
pages have been removed.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

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

  • The new redirects centralize host URLs nicely, but there’s still a mix of patterns (e.g. React SubscriptionTypeFormatter still points to /content_hosts/:id while other spots use /new/hosts or host_details_page_path); consider standardizing on the helper-based new host details URL instead of hitting the redirect.
  • In katello-features.run.js the remoteActions array is now empty; if it’s no longer used at all it might be cleaner to drop the variable and any related feature-flag plumbing, or otherwise keep only the entries still needed by the remaining bulk action modals.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The new redirects centralize host URLs nicely, but there’s still a mix of patterns (e.g. React `SubscriptionTypeFormatter` still points to `/content_hosts/:id` while other spots use `/new/hosts` or `host_details_page_path`); consider standardizing on the helper-based new host details URL instead of hitting the redirect.
- In `katello-features.run.js` the `remoteActions` array is now empty; if it’s no longer used at all it might be cleaner to drop the variable and any related feature-flag plumbing, or otherwise keep only the entries still needed by the remaining bulk action modals.

## Individual Comments

### Comment 1
<location> `engines/bastion_katello/app/assets/javascripts/bastion_katello/bastion-katello-bootstrap.js:33-40` </location>
<code_context>

</code_context>

<issue_to_address>
**issue (code-quality):** Use `const` or `let` instead of `var`. ([`avoid-using-var`](https://docs.sourcery.ai/Reference/Rules-and-In-Line-Suggestions/JavaScript/Default-Rules/avoid-using-var))

<details><summary>Explanation</summary>`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code).
`let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than
function-scoped.

From the [Airbnb JavaScript Style Guide](https://airbnb.io/javascript/#references--prefer-const)
</details>
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

- Update SubscriptionTypeFormatter to use new host URL directly
  instead of relying on redirect (use urlBuilder with 'new/hosts/:id')
- Remove empty remoteActions array and addStates call from
  katello-features.run.js since no states need feature flag routing
- Update Jest snapshot to match new URL format

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <[email protected]>
@jeremylenz jeremylenz force-pushed the 38933-remove-legacy-ch branch from d89a19d to 52e529c Compare December 5, 2025 20:19
@jeremylenz
Copy link
Member Author

/packit build

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant