Fixes #35713 - Host details tab for Debian packages #10744

nadjaheitmann · 2023-09-19T10:27:56Z

What are the changes introduced in this pull request?

Host details tab for Debian packages

What are the testing steps for this pull request?

Try viewing, installing, removing and updating deb packages for hosts.

For code review: There are actually a lot of code pieces that were taken from the PackagesTab. Any re-structuring of the code needed?

What is missing:

Tests

theforeman-bot · 2023-09-19T10:28:00Z

Issues: #35713

nadjaheitmann · 2023-10-27T10:27:31Z

Hey @lfu , I saw that you created most of the host details packages tab and I am trying to reuse your code for Debian packages, but I got stuck at the tests. Do you have any idea what needs adjustment? Any hints are highly appreciated!

nadjaheitmann · 2023-10-27T11:52:44Z

Hey @lfu , I saw that you created most of the host details packages tab and I am trying to reuse your code for Debian packages, but I got stuck at the tests. Do you have any idea what needs adjustment? Any hints are highly appreciated!

Or maybe @jturel ? Not sure, who would be the right person to ping.

lfu · 2023-10-27T14:55:11Z

cc @jeremylenz

jeremylenz · 2023-10-27T15:00:45Z

Error: Error: Nock: No match for request {

means the test is making an API request that wasn't planned for with a Nock instance. Each API request in a test must have a corresponding scope that matches the request, and then an assertNockRequest at the end of the test. You can run the tests with

DEBUG=nock.* npx jest <filename>

to see how Nock is doing its matching. Probably some adjustment of the nock .query is needed.

nadjaheitmann · 2023-10-31T07:55:34Z

Thanks @jeremylenz That debug output helped me a lot!

jeremylenz

Haven't tested this yet but here's a couple things I noticed :)

app/controllers/katello/api/v2/debs_controller.rb

jeremylenz · 2023-11-02T20:35:13Z

webpack/components/extensions/HostDetails/Tabs/DebsTab/DebsTab.js

+import { urlBuilder } from 'foremanReact/common/urlHelpers';
+import SelectableDropdown from '../../../../SelectableDropdown';
+import TableWrapper from '../../../../../components/Table/TableWrapper';
+import { useBulkSelect, useTableSort, useUrlParams, useSet } from '../../../../../components/Table/TableHooks';


After #10783 is merged you'll have to import these from foremanReact, just fyi

Thanks, applied! (at least where it was giving me an error)

nadjaheitmann · 2023-11-10T13:52:00Z

@jeremylenz Got the react tests running! Might need some Ruby tests, though. I copied a lot of code from the packages tab. Shall I mention the developer - mostly @lfu I think - in the commit message? I think this should be credited. Another option is to have a common packages tab component for rpm packages and for deb packages and re-use it for the specific tabs. Would be the cleaner implementation, I guess, but would require quite some afford to change the code.

lfu · 2023-11-10T18:20:19Z

... Shall I mention the developer - mostly @lfu I think - in the commit message? I think this should be credited...

Not necessary at all :)

jeremylenz · 2023-11-10T18:32:08Z

Another option is to have a common packages tab component for rpm packages and for deb packages and re-use it for the specific tabs. Would be the cleaner implementation, I guess, but would require quite some afford to change the code.

In our downstream product we rip out or disable most of the Debian-related stuff anyway, so this implementation with separate Deb components and files actually seems cleaner to me with that in mind. 👍

nadjaheitmann · 2023-11-14T08:19:09Z

In our downstream product we rip out or disable most of the Debian-related stuff anyway, so this implementation with separate Deb components and files actually seems cleaner to me with that in mind. 👍

Okay, then let's leave the structure as is then. Theoretically, the PR should not affect rpm repos if it works correctly. It just adds a package tab to Debian repositories.

nadjaheitmann · 2023-12-14T09:35:38Z

@jeremylenz This is ready for review :)

jeremylenz

Thanks @nadjaheitmann !

Spun up a Debian 11 host and everything seems to work ok. Some minor comments below.

app/models/katello/concerns/host_managed_extensions.rb

webpack/components/extensions/HostDetails/Tabs/DebsTab/DebInstallModal.scss

app/models/katello/deb.rb

jeremylenz · 2023-12-15T21:47:34Z

app/models/katello/concerns/host_managed_extensions.rb

@@ -560,6 +560,24 @@ def package_names_for_job_template(action:, search:, versions: nil)
        end
      end

+      def deb_names_for_job_template(action:, search:)


I wonder if we could avoid adding a new method here, and just check self.operatingsystem.family within package_names_for_job_template instead?

@jeremylenz I think, it is a good idea and I have a added a commit on top of the actual commit for the host details page. However, rubocop does not like the changes. Not sure if I can make this any simpler.

[2024-01-05T09:07:35.947Z] Offenses:

[2024-01-05T09:07:35.947Z] /home/jenkins/workspace/katello-pr-test/app/models/katello/concerns/host_managed_extensions.rb:541:7: C: Metrics/AbcSize: Assignment Branch Condition size for package_names_for_job_template is too high. [<9, 56, 21> 60.48/60]

[2024-01-05T09:07:35.947Z] def package_names_for_job_template(action:, search:, versions: nil) ...

[2024-01-05T09:07:35.947Z] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[2024-01-05T09:07:35.947Z] /home/jenkins/workspace/katello-pr-test/app/models/katello/concerns/host_managed_extensions.rb:541:7: C: Metrics/CyclomaticComplexity: Cyclomatic complexity for package_names_for_job_template is too high. [16/14]

[2024-01-05T09:07:35.947Z] def package_names_for_job_template(action:, search:, versions: nil) ...

[2024-01-05T09:07:35.947Z] ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[2024-01-05T09:07:35.947Z] /home/jenkins/workspace/katello-pr-test/app/models/katello/concerns/host_managed_extensions.rb:541:7: C: Metrics/MethodLength: Method has too many lines. [34/30]

[2024-01-05T09:07:35.947Z] def package_names_for_job_template(action:, search:, versions: nil) ...

@nadjaheitmann Two options here. I would be fine with either one:

Extract your if / else block to its own method. I would probably also do it as a case statement (case operatingsystem.family)

Or, just disable that cop and enable it at the end of the method. We do that in several places.

I tried option one, although I did not want to use a case statement. Seems safer to me to have Debian and everything else :)

jeremylenz · 2024-01-09T20:54:38Z

@nadjaheitmann Please squash and/or rename your commit to add 'Refs #35713', then we can merge.

jeremylenz

Thanks @nadjaheitmann!

ACK 👍

(cherry picked from commit e48bd5b)

theforeman-bot added Not yet reviewed Waiting on contributor labels Sep 19, 2023

nadjaheitmann force-pushed the add_host_details_debs_tab branch from 284b57f to 4d861db Compare September 21, 2023 11:53

theforeman-bot removed the Waiting on contributor label Sep 21, 2023

nadjaheitmann force-pushed the add_host_details_debs_tab branch 2 times, most recently from 4a8c724 to d97b173 Compare September 21, 2023 12:15

nadjaheitmann force-pushed the add_host_details_debs_tab branch 2 times, most recently from a3e56c3 to 51969bb Compare October 26, 2023 12:06

nadjaheitmann force-pushed the add_host_details_debs_tab branch from 51969bb to a9d347d Compare October 31, 2023 11:41

jeremylenz reviewed Nov 2, 2023

View reviewed changes

nadjaheitmann force-pushed the add_host_details_debs_tab branch 6 times, most recently from df17355 to bdb5865 Compare November 10, 2023 12:49

nadjaheitmann changed the title ~~WIP: Fixes #35713 - Host details tab for Debian packages~~ Fixes #35713 - Host details tab for Debian packages Nov 14, 2023

nadjaheitmann force-pushed the add_host_details_debs_tab branch 2 times, most recently from 6fa8d38 to c818918 Compare November 16, 2023 10:38

nadjaheitmann requested a review from jeremylenz November 16, 2023 10:41

nadjaheitmann force-pushed the add_host_details_debs_tab branch from c818918 to f9ba1a9 Compare November 17, 2023 09:54

jeremylenz reviewed Dec 15, 2023

View reviewed changes

pr-processor bot added the Waiting on contributor label Jan 4, 2024

nadjaheitmann force-pushed the add_host_details_debs_tab branch from eb77ed5 to 8abc54d Compare January 4, 2024 11:07

pr-processor bot added Waiting on contributor and removed Waiting on contributor labels Jan 4, 2024

nadjaheitmann force-pushed the add_host_details_debs_tab branch from 0484df6 to 57b957a Compare January 4, 2024 11:24

pr-processor bot added Waiting on contributor and removed Waiting on contributor labels Jan 4, 2024

nadjaheitmann force-pushed the add_host_details_debs_tab branch 4 times, most recently from 6acdc28 to e8cc177 Compare January 9, 2024 20:06

Fixes #35713 - Host details tab for Debian packages

bb4e9ee

nadjaheitmann force-pushed the add_host_details_debs_tab branch from e8cc177 to bb4e9ee Compare January 10, 2024 07:09

pr-processor bot removed the Waiting on contributor label Jan 10, 2024

jeremylenz approved these changes Jan 10, 2024

View reviewed changes

pr-processor bot removed the Not yet reviewed label Jan 10, 2024

jeremylenz merged commit e48bd5b into Katello:master Jan 10, 2024
6 checks passed

maximiliankolb deleted the add_host_details_debs_tab branch January 10, 2024 16:15

jeremylenz mentioned this pull request Jan 11, 2024

Refs #35713 - correctly call methods with kwargs #10845

Merged

nadjaheitmann mentioned this pull request Feb 7, 2024

Fixes #37144 - Use OS family for host details page packages tab #10881

Merged

sbernhard pushed a commit to ATIX-AG/katello that referenced this pull request Feb 29, 2024

Fixes #35713 - Host details tab for Debian packages (Katello#10744)

b84d497

(cherry picked from commit e48bd5b)

sbernhard pushed a commit to ATIX-AG/katello that referenced this pull request Mar 8, 2024

Fixes #35713 - Host details tab for Debian packages (Katello#10744)

951c1e1

(cherry picked from commit e48bd5b)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #35713 - Host details tab for Debian packages #10744

Fixes #35713 - Host details tab for Debian packages #10744

nadjaheitmann commented Sep 19, 2023 •

edited

Loading

theforeman-bot commented Sep 19, 2023

nadjaheitmann commented Oct 27, 2023

nadjaheitmann commented Oct 27, 2023

lfu commented Oct 27, 2023

jeremylenz commented Oct 27, 2023

nadjaheitmann commented Oct 31, 2023

jeremylenz left a comment

jeremylenz Nov 2, 2023

nadjaheitmann Nov 10, 2023 •

edited

Loading

nadjaheitmann commented Nov 10, 2023

lfu commented Nov 10, 2023

jeremylenz commented Nov 10, 2023

nadjaheitmann commented Nov 14, 2023

nadjaheitmann commented Dec 14, 2023

jeremylenz left a comment

jeremylenz Dec 15, 2023

nadjaheitmann Jan 5, 2024

jeremylenz Jan 8, 2024

nadjaheitmann Jan 9, 2024 •

edited

Loading

jeremylenz commented Jan 9, 2024

jeremylenz left a comment

Fixes #35713 - Host details tab for Debian packages #10744

Fixes #35713 - Host details tab for Debian packages #10744

Conversation

nadjaheitmann commented Sep 19, 2023 • edited Loading

What are the changes introduced in this pull request?

What are the testing steps for this pull request?

theforeman-bot commented Sep 19, 2023

nadjaheitmann commented Oct 27, 2023

nadjaheitmann commented Oct 27, 2023

lfu commented Oct 27, 2023

jeremylenz commented Oct 27, 2023

nadjaheitmann commented Oct 31, 2023

jeremylenz left a comment

Choose a reason for hiding this comment

jeremylenz Nov 2, 2023

Choose a reason for hiding this comment

nadjaheitmann Nov 10, 2023 • edited Loading

Choose a reason for hiding this comment

nadjaheitmann commented Nov 10, 2023

lfu commented Nov 10, 2023

jeremylenz commented Nov 10, 2023

nadjaheitmann commented Nov 14, 2023

nadjaheitmann commented Dec 14, 2023

jeremylenz left a comment

Choose a reason for hiding this comment

jeremylenz Dec 15, 2023

Choose a reason for hiding this comment

nadjaheitmann Jan 5, 2024

Choose a reason for hiding this comment

jeremylenz Jan 8, 2024

Choose a reason for hiding this comment

nadjaheitmann Jan 9, 2024 • edited Loading

Choose a reason for hiding this comment

jeremylenz commented Jan 9, 2024

jeremylenz left a comment

Choose a reason for hiding this comment

nadjaheitmann commented Sep 19, 2023 •

edited

Loading

nadjaheitmann Nov 10, 2023 •

edited

Loading

nadjaheitmann Jan 9, 2024 •

edited

Loading