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

Automember > 'Host group rules' - Action buttons #630

Merged
merged 7 commits into from
Feb 12, 2025

Conversation

carma12
Copy link
Collaborator

@carma12 carma12 commented Feb 4, 2025

The action buttons for the 'Hostgroup rules' page have been implemented. This includes:

  • 'Add'
  • 'Delete'
  • Set default group via Selector component in toolbar
  • Integration test to check its functionality

This PR depends on this one to be merged: #628

@carma12 carma12 added the WIP Work in Progress (do not merge) label Feb 4, 2025
@carma12 carma12 force-pushed the automem-host-group-action-buttons branch 2 times, most recently from 96ab28b to 66676ff Compare February 7, 2025 13:56
The 'Delete' button functionality must be
implemented to handle the deletion of the
Automember rules (User groups).

Signed-off-by: Carla Martinez <[email protected]>
The Selector located in the toolbar should show
the default group in the automembers page. This
functionality should also allow to modify the
default group via API command and show the new
one.

Signed-off-by: Carla Martinez <[email protected]>
The 'Add' button must allow the
addition of new hostgroup rules
to the table.

Signed-off-by: Carla Martinez <[email protected]>
The 'Remove' button must manage the
deletion of hostgroup rules in the
table.

Signed-off-by: Carla Martinez <[email protected]>
The default hostgroup functionality is handled
through the selector located in the main
toolbar.

Signed-off-by: Carla Martinez <[email protected]>
@carma12 carma12 force-pushed the automem-host-group-action-buttons branch from 66676ff to 0ba870a Compare February 11, 2025 16:11
@carma12 carma12 added needs-review This PR is waiting on a review tests PR related to testing and removed WIP Work in Progress (do not merge) labels Feb 11, 2025
@carma12 carma12 requested a review from mreynolds389 February 11, 2025 16:12
Copy link
Collaborator

@mreynolds389 mreynolds389 left a comment

Choose a reason for hiding this comment

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

LGTM, but I think the automember table entries should be links to the referenced entry. I guess a separate PR is needed :-)

@mreynolds389 mreynolds389 added the ack Pull Request approved, it can be merged label Feb 11, 2025
The integration test should cover all use cases in
the Automember > Hostgroup rules page.

Signed-off-by: Carla Martinez <[email protected]>
@carma12 carma12 force-pushed the automem-host-group-action-buttons branch from 0ba870a to 4bde204 Compare February 12, 2025 09:14
@carma12
Copy link
Collaborator Author

carma12 commented Feb 12, 2025

LGTM, but I think the automember table entries should be links to the referenced entry. I guess a separate PR is needed :-)

Indeed. But I plan to implement this functionality (related to the Settings page) in a different PR.

@carma12 carma12 requested a review from duzda February 12, 2025 09:16
The `hostgroup_members.feature` test needs to use
an already-created host group to perform the
checks on operations.

Also, there are some duplicated components in
the `AutoMemUserRules` component (most likely
due to a rebase) that need to be corrected.

Signed-off-by: Carla Martinez <[email protected]>
@carma12 carma12 force-pushed the automem-host-group-action-buttons branch from a4c8c0b to e7a2fd1 Compare February 12, 2025 15:01
@carma12 carma12 merged commit 4a82415 into freeipa:main Feb 12, 2025
4 checks passed
Copy link

@duzda duzda left a comment

Choose a reason for hiding this comment

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

Seems to fix other tests as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, it can be merged needs-review This PR is waiting on a review tests PR related to testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants