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

ui - improve self review member approval #2849

Merged
merged 1 commit into from
Jan 16, 2025

Conversation

ArtjomsPorss
Copy link
Contributor

@ArtjomsPorss ArtjomsPorss commented Jan 7, 2025

Description

use member comment to auto populate justification textarea when approving self review roles and groups

Contribution Checklist:

  • The pull request does not introduce any breaking changes
  • I have read the contribution guidelines.
  • Create an issue and link to the pull request.

@ArtjomsPorss ArtjomsPorss changed the title ui - use member comment as justification when approving self review r… ui - improve self review member approval Jan 7, 2025
@ArtjomsPorss ArtjomsPorss marked this pull request as ready for review January 8, 2025 10:34
Copy link
Contributor

@chandrasekhar1996 chandrasekhar1996 left a comment

Choose a reason for hiding this comment

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

adding comment from offline conversation:

  1. instead of using getRoles and getGroups to get details of all roles/groups in a domain, it would be better to do a getRole/Group call for specific collection which have a pending member

  2. It will also be useful to check if the role/group data is already present in store, if not then make API call to get role/group and then store it in redux store.

@ArtjomsPorss
Copy link
Contributor Author

adding comment from offline conversation:

  1. instead of using getRoles and getGroups to get details of all roles/groups in a domain, it would be better to do a getRole/Group call for specific collection which have a pending member
  2. It will also be useful to check if the role/group data is already present in store, if not then make API call to get role/group and then store it in redux store.

will make changes to cover both points

@ArtjomsPorss
Copy link
Contributor Author

ArtjomsPorss commented Jan 13, 2025

@chandrasekhar1996
https://github.com/AthenZ/athenz/pull/2849/files#diff-cb36fec9399cc92922646c454ed2ea4e6def1c54aff8931657551510f2b1a5eeR95-R107
double checked, this part handles point 2 - checks if pending members are in redux storage and not expired (5min), then returns them.
if expired or not present -> get from backend, enrich with selfServe and justification, store in redux storage.

@ArtjomsPorss ArtjomsPorss force-pushed the self-serve-review branch 2 times, most recently from d7624d3 to a3a1681 Compare January 16, 2025 12:50
…oles and grops

Signed-off-by: aporss <art.porss@yahooinc.com>
Comment on lines +43 to +64

const getPendingMemberRole = async (dispatch, state, domainName, roleName) => {
let role = selectPendingMemberRole(state, domainName, roleName);
if (!role || isExpired(role.expiry)) {
role = await API().getRole(domainName, roleName, false, false, true);
role.expiry = getExpiryTime();
dispatch(storePendingRole(role, domainName, roleName));
}
return role;
};
const getPendingMemberGroup = async (
dispatch,
state,
domainName,
groupName
) => {
let group = selectPendingMemberGroup(state, domainName, groupName);
if (!group || isExpired(group.expiry)) {
group = await API().getGroup(domainName, groupName, false, true);
group.expiry = getExpiryTime();
dispatch(storePendingGroup(group, domainName, groupName));
}
Copy link
Contributor Author

@ArtjomsPorss ArtjomsPorss Jan 16, 2025

Choose a reason for hiding this comment

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

@chandrasekhar1996 this part covers selecting specific role/group data from state, if not present or expired - get role/group and store in state for later

Comment on lines +91 to +98
if (category === ROLE) {
promises.push(getPendingMemberRole(dispatch, state, domain, role));
} else if (category === GROUP) {
promises.push(getPendingMemberGroup(dispatch, state, domain, role));
}
});

let data = await Promise.all(promises);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

getting unique roles/groups for selfReview check is done in parallel

@abvaidya abvaidya merged commit 08a1d28 into AthenZ:master Jan 16, 2025
3 checks passed
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.

3 participants