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

[Active users][Settings][Account settings] Add functionality for checkbox list #139

Merged
merged 1 commit into from
Sep 5, 2023

Conversation

carma12
Copy link
Collaborator

@carma12 carma12 commented Aug 25, 2023

The functionality for the group of checkboxes has been implemented and applied to the following field: 'User authentication types' (ipauserauthtype).

@carma12 carma12 added the WIP Work in Progress (do not merge) label Aug 25, 2023
@carma12 carma12 force-pushed the account-settings-checkbox-list branch from 6182301 to 79e95b0 Compare August 25, 2023 09:47
@carma12 carma12 removed the WIP Work in Progress (do not merge) label Aug 25, 2023
@carma12 carma12 requested a review from pvoborni August 25, 2023 09:47
@carma12
Copy link
Collaborator Author

carma12 commented Aug 25, 2023

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

@carma12 carma12 force-pushed the account-settings-checkbox-list branch 4 times, most recently from b83dadc to 1b1d6ff Compare September 1, 2023 06:43
Copy link
Member

@pvoborni pvoborni left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Added few comments.

There is also one issue which we will probably need to deal with soon - when comparing if we did any changes, the order of values in array might matter in the comparison but in some cases it might not matter in IPA itself.

src/components/UsersSections/UsersAccountSettings.tsx Outdated Show resolved Hide resolved
src/components/UsersSections/UsersAccountSettings.tsx Outdated Show resolved Hide resolved
src/utils/ipaObjectUtils.ts Outdated Show resolved Hide resolved
src/utils/ipaObjectUtils.ts Outdated Show resolved Hide resolved
src/components/Form/IpaCheckboxes.tsx Outdated Show resolved Hide resolved
src/components/Form/IpaCheckboxes.tsx Show resolved Hide resolved
src/components/Form/IpaCheckboxes.tsx Outdated Show resolved Hide resolved
@carma12 carma12 force-pushed the account-settings-checkbox-list branch from 1b1d6ff to 6583906 Compare September 5, 2023 07:56
@carma12
Copy link
Collaborator Author

carma12 commented Sep 5, 2023

The code has been updated and amended based on most of the feedback requested above. Some conversations still need to be clarified.

@carma12 carma12 force-pushed the account-settings-checkbox-list branch from 6583906 to ab3f564 Compare September 5, 2023 12:33
@carma12
Copy link
Collaborator Author

carma12 commented Sep 5, 2023

Updated the code based on feedback provided.

The functionality for the group of checkboxes
needs to be implemented and applied to the
following field: 'User authentication types'
(`ipauserauthtype`).

Signed-off-by: Carla Martinez <carlmart@redhat.com>
@carma12 carma12 force-pushed the account-settings-checkbox-list branch from ab3f564 to add1e6d Compare September 5, 2023 13:20
Copy link
Member

@pvoborni pvoborni left a comment

Choose a reason for hiding this comment

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

LGTM

@pvoborni pvoborni self-assigned this Sep 5, 2023
@carma12 carma12 merged commit 04c1411 into freeipa:main Sep 5, 2023
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.

2 participants