You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This PR would bring in edit functionality for the teams card in teams permission page.
Changes Made
There is a new reactor built for the edit team, so have integrated new reactor call with the new edit modal. Have used the concept of add team modal to achieve the same.
How to Test
Go to team permissions page
Click on the three dots in the card you want to edit and click on Edit team
Edit modal will open - modify whatever you want to change and click on update.
The editTeam method sends type=previousType and newType=type, which may invert intended values and break the server-side update. Confirm correct parameter assignment and endpoint.
The initial state for previousType is set to id, which is incorrect. Initialize it from the type or isCustomGroup prop to accurately reflect the prior team type.
Why: The state previousType is initialized to id which is incorrect for capturing the prior team type; using isCustomGroup ? 'Custom' : type ensures correct behavior when editing.
Medium
Add fallback for team matching
If previousTeamName is undefined or changed by casing, this comparison may fail. Also fall back to the original id prop to ensure the correct team is updated.
Why: Including t.id === id as a fallback ensures the correct team is updated if previousTeamName is undefined or mismatched, though this is a minor edge-case improvement.
Low
General
Switch to URLSearchParams
Manually concatenating form data risks missing ampersands or encoding issues. Use URLSearchParams to build the request body.
Why: Building the request body with URLSearchParams guarantees proper encoding and placement of &, reducing manual string concatenation errors and improving maintainability.
Medium
Guard previous values update on open
Only update previousTeamName and previousType when the modal actually opens to avoid stale values on prop changes. Wrap the setters in an if (open) check.
useEffect(() => {
reset({
TEAM_NAME: id || '',
TEAM_DESCRIPTION: description || '',
TEAM_TYPE:
(isCustomGroup || type === 'custom' ? 'Custom' : type) || '',
});
-- // Update the previous team name when the modal is opened+ if (open) {
setPreviousTeamName(id);
setPreviousType(isCustomGroup ? 'Custom' : type);
-}, [id, type, description, isCustomGroup, reset]);+ }+}, [open, id, type, description, isCustomGroup, reset]);
Suggestion importance[1-10]: 6
__
Why: Wrapping setPreviousTeamName and setPreviousType in if (open) prevents overwriting the previous values when the modal is closed, avoiding stale state issues.
Initialize previousType state with the original type prop instead of id to correctly track the previous team type. This ensures the component stores the actual previous type value.
Why: Initializing previousType with id is incorrect; using the type prop ensures the component tracks the actual previous team type and prevents wrong values in the API call.
Medium
Fallback previousTeamName and preserve fields
Provide a fallback to the new team.id when previousTeamName is undefined and spread existing fields to preserve any other properties on the team object.
Why: Spreading t retains all existing team properties and using a fallback key prevents matching against undefined, ensuring accurate updates without data loss.
Medium
General
Use URLSearchParams for payload
Use URLSearchParams to build the request body with proper encoding and set the Content-Type header to application/x-www-form-urlencoded for clearer, safer parameter handling.
Why: Switching to URLSearchParams and explicitly setting the Content-Type header improves encoding safety and makes the request payload handling clearer and more maintainable.
The initial state for previousType is incorrectly using id instead of the existing type. Initialize it using type (or 'Custom' when isCustomGroup is true) so that the old team type is tracked properly.
Why: Initializing previousType with id instead of the actual type will break tracking of the old team’s type, so correcting this value prevents a logic bug.
Medium
Guard undefined previousTeamName
If previousTeamName is undefined the API will receive groupId=undefined. Default to the current groupId parameter or guard the call so the endpoint always receives a valid identifier.
Why: If previousTeamName is undefined, the API receives groupId=undefined, causing invalid requests, so defaulting or guarding ensures a valid identifier is sent.
Low
General
Normalize custom type check
The comparison against 'Custom' may fail if casing differs. Normalize both sides to the same case (e.g., lowercase) before comparing to avoid branching errors.
The reason will be displayed to describe this comment to others. Learn more.
It is just the same thing, useState was not imported explicitly in the code, so I just used it directly using React.useState, it doesn't really matter at all. Let me know if you still want me to change it.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR would bring in edit functionality for the teams card in teams permission page.
Changes Made
There is a new reactor built for the edit team, so have integrated new reactor call with the new edit modal. Have used the concept of add team modal to achieve the same.
How to Test
Notes