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

Yashwanth individual permission change show #2868

Open
wants to merge 5 commits into
base: development
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ function PermissionChangeLogTable({ changeLogs, darkMode }) {
<th className={`permission-change-log-table--header${addDark}`}>
Log Date and Time (PST)
</th>
<th className={`permission-change-log-table--header${addDark}`}>Role Name</th>
<th className={`permission-change-log-table--header${addDark}`}>Name</th>
<th className={`permission-change-log-table--header${addDark}`}>Permissions</th>
<th className={`permission-change-log-table--header${addDark}`}>Permissions Added</th>
<th className={`permission-change-log-table--header${addDark}`}>
Expand All @@ -111,9 +111,7 @@ function PermissionChangeLogTable({ changeLogs, darkMode }) {
<td className={`permission-change-log-table--cell ${bgYinmnBlue}`}>{`${formatDate(
log.logDateTime,
)} ${formattedAmPmTime(log.logDateTime)}`}</td>
<td className={`permission-change-log-table--cell ${bgYinmnBlue}`}>
{log.roleName}
</td>
<td className={`permission-change-log-table--cell ${bgYinmnBlue}`}>{log.name}</td>
<td className={`permission-change-log-table--cell permissions ${bgYinmnBlue}`}>
{renderPermissions(log.permissions, log._id)}
</td>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,10 +60,11 @@ function PermissionsManagement({ roles, auth, getUserRole, userProfile, darkMode

getInfoCollections();
getUserRole(auth?.user.userid);

const getChangeLogs = async () => {
try {
const response = await axios.get(ENDPOINTS.PERMISSION_CHANGE_LOGS(auth?.user.userid));
const response = await axios.get(
ENDPOINTS.GET_USER_PERMISSION_CHANGE_LOGS(auth?.user.userid),
);
setChangeLogs(response.data);
setLoading(false);
} catch (error) {
Expand Down Expand Up @@ -243,7 +244,7 @@ function PermissionsManagement({ roles, auth, getUserRole, userProfile, darkMode
</div>
{loading && <p className="loading-message">Loading...</p>}
{changeLogs?.length > 0 && (
<PermissionChangeLogTable changeLogs={changeLogs.slice().reverse()} darkMode={darkMode} />
<PermissionChangeLogTable changeLogs={changeLogs} darkMode={darkMode} />
Copy link
Contributor

@nathanah nathanah Nov 16, 2024

Choose a reason for hiding this comment

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

removing reverse? Nvm I see the sort on the backend now.

)}
<br />
<br />
Expand Down
42 changes: 42 additions & 0 deletions src/components/PermissionsManagement/UserPermissionsPopUp.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,16 @@ function UserPermissionsPopUp({
}
}, [actualUserProfile]);

const logPermissionChanges = async (logPermissionChangesUrl, permissionChangeLog) => {
try {
await axios.post(logPermissionChangesUrl, permissionChangeLog);
} catch (error) {
toast.error('Error logging permissions:', {
autoClose: 10000,
});
}
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const logPermissionChanges = async (logPermissionChangesUrl, permissionChangeLog) => {
try {
await axios.post(logPermissionChangesUrl, permissionChangeLog);
} catch (error) {
toast.error('Error logging permissions:', {
autoClose: 10000,
});
}
};

const updateProfileOnSubmit = async e => {
e.preventDefault();
const shouldPreventEdit = cantUpdateDevAdminDetails(actualUserProfile?.email, authUser.email);
Expand All @@ -80,9 +90,40 @@ function UserPermissionsPopUp({
const userId = actualUserProfile?._id;

const url = ENDPOINTS.USER_PROFILE(userId);
const logPermissionChangesUrl = ENDPOINTS.POST_USER_PERMISSION_CHANGE_LOGS;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const logPermissionChangesUrl = ENDPOINTS.POST_USER_PERMISSION_CHANGE_LOGS;

const allUserInfo = await axios.get(url).then(res => res.data);
const newUserInfo = { ...allUserInfo, permissions: { frontPermissions: userPermissions } };

let existingPermissions = allUserInfo.permissions?.frontPermissions || [];
const existingPermissionsSet = new Set(existingPermissions || []);
const newPermissionsSet = new Set(userPermissions || []);
const addedPermissions = (userPermissions || []).filter(
permission => !existingPermissionsSet.has(permission),
);
const removedPermissions = (existingPermissions || []).filter(
permission => !newPermissionsSet.has(permission),
);

existingPermissions = [
...existingPermissions.filter(permission => !removedPermissions.includes(permission)),
...addedPermissions,
];

const permissionChangeLog = {
actualUserProfile: {
firstName: actualUserProfile.firstName,
lastName: actualUserProfile.lastName,
},
authUser: {
email: authUser.email,
role: authUser.role,
},
userId,
existingPermissions, // Existing permissions before change
addedPermissions, // New permissions added
removedPermissions, // Permissions removed
};

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
let existingPermissions = allUserInfo.permissions?.frontPermissions || [];
const existingPermissionsSet = new Set(existingPermissions || []);
const newPermissionsSet = new Set(userPermissions || []);
const addedPermissions = (userPermissions || []).filter(
permission => !existingPermissionsSet.has(permission),
);
const removedPermissions = (existingPermissions || []).filter(
permission => !newPermissionsSet.has(permission),
);
existingPermissions = [
...existingPermissions.filter(permission => !removedPermissions.includes(permission)),
...addedPermissions,
];
const permissionChangeLog = {
actualUserProfile: {
firstName: actualUserProfile.firstName,
lastName: actualUserProfile.lastName,
},
authUser: {
email: authUser.email,
role: authUser.role,
},
userId,
existingPermissions, // Existing permissions before change
addedPermissions, // New permissions added
removedPermissions, // Permissions removed
};

This logic should be on the backend and part of the same API call as the update itself.

await axios
.put(url, newUserInfo)
.then(() => {
Expand All @@ -94,6 +135,7 @@ function UserPermissionsPopUp({
autoClose: 10000,
});
setToastShown(true);
logPermissionChanges(logPermissionChangesUrl, permissionChangeLog);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
logPermissionChanges(logPermissionChangesUrl, permissionChangeLog);

This is not atomic. This request could fail after the permission change request succeeds, so it wouldn't show up in the history. If a dev wanted to test updating permissions manually with something like Postman, they could avoid also logging those changes which could cause confusion or potential problems.
Logging permission changes should be part of the same API call as updating the user's permissions.

}
toggle();
})
Expand Down
2 changes: 2 additions & 0 deletions src/utils/URL.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ const APIEndpoint =
export const ENDPOINTS = {
APIEndpoint: () => APIEndpoint,
USER_PROFILE: userId => `${APIEndpoint}/userprofile/${userId}`,
POST_USER_PERMISSION_CHANGE_LOGS: `${APIEndpoint}/logPermissionChanges`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
POST_USER_PERMISSION_CHANGE_LOGS: `${APIEndpoint}/logPermissionChanges`,

GET_USER_PERMISSION_CHANGE_LOGS: userId => `${APIEndpoint}/logPermissionChanges/${userId}`,
USER_PROFILE_PROPERTY: userId => `${APIEndpoint}/userprofile/${userId}/property`,
USER_PROFILES: `${APIEndpoint}/userprofile/`,
UPDATE_REHIREABLE_STATUS: userId => `${APIEndpoint}/userprofile/${userId}/rehireable`,
Expand Down
Loading