Skip to content

Fixes #39322 - Extend valid controller list - #11743

Merged
jturel merged 1 commit into
Katello:masterfrom
adamruzicka:bookmark-extensions
May 26, 2026
Merged

jturel merged 1 commit into
Katello:masterfrom
adamruzicka:bookmark-extensions

Conversation

@adamruzicka

@adamruzicka adamruzicka commented May 14, 2026

Copy link
Copy Markdown
Member

What are the changes introduced in this pull request?

Previously, the extension overrode whatever Foreman was shipping, which prevented other plugins from applying their own modifications.

Considerations taken when implementing this change?

Following the general pattern where plugins can extend core but shouldn't hard-override it.

What are the testing steps for this pull request?

Also see theforeman/foreman_openscap#610

Summary by CodeRabbit

  • Refactor

    • Restructured bookmark validation logic to improve code organization, clarity, and long-term maintainability of the validation system.
  • Tests

    • Added comprehensive automated test coverage for bookmark validation across all supported controller types and configurations, ensuring proper validation behavior and error handling.

Review Change Stack

instead of directly overriding it to allow other plugins to extend it as well
@coderabbitai

coderabbitai Bot commented May 14, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a0ca4355-2e70-49cb-93ee-35e8bf0d1129

📥 Commits

Reviewing files that changed from the base of the PR and between 4431dd7 and 460de33.

📒 Files selected for processing (2)
  • app/lib/katello/concerns/bookmark_controller_validator_extensions.rb
  • test/lib/validators/bookmark_controller_validator_extensions_test.rb

📝 Walkthrough

Walkthrough

This PR refactors the bookmark controller validator by extracting Katello controller paths into an explicit KATELLO_CONTROLLERS constant and updating valid_controllers_list to extend the parent list. A new test validates bookmarks for all defined controllers.

Changes

Bookmark Controller Validator Refactoring

Layer / File(s) Summary
KATELLO_CONTROLLERS constant and validation
app/lib/katello/concerns/bookmark_controller_validator_extensions.rb, test/lib/validators/bookmark_controller_validator_extensions_test.rb
KATELLO_CONTROLLERS constant is introduced with controller paths, and valid_controllers_list is updated to return super + KATELLO_CONTROLLERS. Test iterates over each controller and validates bookmark creation with FactoryBot.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main change: extending the valid controller list to work with Katello controllers while following plugin extension patterns.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Tip

💬 Introducing Slack Agent: The best way for teams to turn conversations into code.

Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.

  • Generate code and open pull requests
  • Plan features and break down work
  • Investigate incidents and troubleshoot customer tickets together
  • Automate recurring tasks and respond to alerts with triggers
  • Summarize progress and report instantly

Built for teams:

  • Shared memory across your entire org—no repeating context
  • Per-thread sandboxes to safely plan and execute work
  • Governance built-in—scoped access, auditability, and budget controls

One agent for your entire SDLC. Right inside Slack.

👉 Get started


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@adamruzicka

Copy link
Copy Markdown
Member Author

Robottelo approves SatelliteQE/robottelo#21539 (comment)


module Katello
class BookmarkControllerValidatorExtensionsTest < ActiveSupport::TestCase
Katello::Concerns::BookmarkControllerValidatorExtensions::KATELLO_CONTROLLERS.each do |controller|

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

An alternative would be iterating over all controllers that include the katello flavor of the autocomplete concern and check those

@jturel jturel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tested & works well

@adamruzicka

Copy link
Copy Markdown
Member Author

And a green button click? :)

@jturel
jturel merged commit 4ad4001 into Katello:master May 26, 2026
23 checks passed
@adamruzicka
adamruzicka deleted the bookmark-extensions branch June 1, 2026 13:48
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