Handle missing group gracefully in role page#3274
Conversation
Signed-off-by: t4niwa <taniwa@lycorp.co.jp>
t4niwa
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the error handling logic for fetching group members in ui/src/server/handlers/api.js to resolve the promise when a 404 status is encountered. This change is implemented across five different locations in the file. The review feedback suggests refactoring this duplicated logic into a shared helper function and using early returns to reduce code nesting and improve maintainability.
| if (err) { | ||
| reject(err); | ||
| } | ||
| if (data && data.groupMembers) { | ||
| roleMember.groupMembers = data.groupMembers; | ||
| roleMember.groupMembers.forEach((member) => { | ||
| member.memberFullName = | ||
| userService.getUserFullName( | ||
| member.memberName | ||
| ); | ||
| }); | ||
| if (err.status === 404) { | ||
| resolve(); | ||
| } else { | ||
| reject(err); | ||
| } | ||
| } else { | ||
| if (data && data.groupMembers) { | ||
| roleMember.groupMembers = data.groupMembers; | ||
| roleMember.groupMembers.forEach((member) => { | ||
| member.memberFullName = | ||
| userService.getUserFullName( | ||
| member.memberName | ||
| ); | ||
| }); | ||
| } | ||
| resolve(); | ||
| } |
There was a problem hiding this comment.
The logic for fetching group members and handling 404 errors is duplicated in five different places within this file (lines 1303, 1453, 1540, 2348, and 2448). This redundancy makes the code harder to maintain and increases the risk of inconsistencies if the logic needs to change in the future.\n\nConsider refactoring this into a single shared helper function. Additionally, the implementation can be simplified by using an early return for the error case, which reduces nesting and improves readability.
if (err) {\n return err.status === 404 ? resolve() : reject(err);\n }\n if (data && data.groupMembers) {\n roleMember.groupMembers = data.groupMembers;\n roleMember.groupMembers.forEach((member) => {\n member.memberFullName =\n userService.getUserFullName(\n member.memberName\n );\n });\n }\n resolve();
|
@t4niwa please fix the lint errors. before submitting any PR, you can just run mvn clean install in the appropriate directory to make sure it passes |
Signed-off-by: t4niwa <taniwa@lycorp.co.jp>
2 participants
Description
Add group consistency check to template deletion. #3273
Contribution Checklist:
Attach Screenshots (Optional)