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 #36799 - Add Salt image path to ConfigReportsController #9849

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

bastian-src
Copy link
Contributor

For Salt reports, no icon is printed in the "origins" column.

This PR adds the Salt image path to the ConfigReportsController. The image is already part of Foreman core.
(It has probably been forgotten when adding the Reports tab to the new host details page here: 616c0d3)

@theforeman-bot
Copy link
Member

Issues: #36799

@bastian-src bastian-src changed the title Fixes #36799 - Add Salt image path to ConfigReportsController [WIP] Fixes #36799 - Add Salt image path to ConfigReportsController Oct 4, 2023
sbernhard
sbernhard previously approved these changes Oct 4, 2023
ekohl
ekohl previously approved these changes Oct 4, 2023
@ekohl
Copy link
Member

ekohl commented Oct 4, 2023

@bastian-src GitHub has PR drafts (https://github.blog/2019-02-14-introducing-draft-pull-requests/) and these days we use those instead of [WIP]. You can easily exclude them in searches.

@bastian-src bastian-src marked this pull request as draft October 4, 2023 11:57
@bastian-src
Copy link
Contributor Author

Alright, thanks! I've marked it as a draft again to add a test.

@bastian-src bastian-src changed the title [WIP] Fixes #36799 - Add Salt image path to ConfigReportsController Fixes #36799 - Add Salt image path to ConfigReportsController Oct 4, 2023
@bastian-src bastian-src dismissed stale reviews from ekohl and sbernhard via 1b85e53 October 5, 2023 08:30
@bastian-src bastian-src marked this pull request as ready for review October 5, 2023 09:08
@bastian-src
Copy link
Contributor Author

@ekohl I've added some tests here, but I'm a but unsure how good/ok they are 🤞

Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

I appreciate the tests.

test/controllers/config_reports_controller_test.rb Outdated Show resolved Hide resolved
test/controllers/config_reports_controller_test.rb Outdated Show resolved Hide resolved
Copy link
Member

@ekohl ekohl left a comment

Choose a reason for hiding this comment

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

Thanks! Tests are still running, but can be merged once green.

@sbernhard
Copy link
Contributor

Tests are through. The failed test should be something different and not releated to this issue. @ekohl

@ekohl ekohl merged commit d3aeb14 into theforeman:develop Oct 5, 2023
8 of 9 checks passed
@bastian-src bastian-src deleted the salt_fix_report_origin_icon branch October 5, 2023 14:32
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