Fix an issue with F2Fs not closing and appeals not opening immediately after review #23
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ): Promise<void> { | ||
| await this.challengeApiService.completeChallenge(challengeId, winners); | ||
| // Trigger finance payments generation after marking the challenge as completed | ||
| void this.financeApiService.generateChallengePayments(challengeId); |
There was a problem hiding this comment.
[correctness]
The use of void to ignore the promise returned by generateChallengePayments could lead to unhandled errors if the promise is rejected. Consider handling the promise explicitly to ensure any errors are logged or managed appropriately.
| }); | ||
|
|
||
| resourcesService.getMemberHandleMap.mockResolvedValue( | ||
| new Map([['4001', 'resolvedHandle']]), |
There was a problem hiding this comment.
[correctness]
Consider checking if the getMemberHandleMap method could potentially return null or undefined instead of a Map. If so, ensure that the code handles such cases to prevent runtime errors.
| } | ||
|
|
||
| const memberIdRaw = payload.submitterMemberId ?? ''; | ||
| const numericMemberId = Number(memberIdRaw); |
There was a problem hiding this comment.
[correctness]
The conversion of memberIdRaw to numericMemberId using Number() could result in NaN if memberIdRaw is not a valid number. Consider using parseInt() or parseFloat() with appropriate checks to ensure the conversion is intentional and correct.
| challenge.id, | ||
| [String(memberIdRaw)], | ||
| ); | ||
| handle = handleMap.get(String(memberIdRaw)) ?? handle; |
There was a problem hiding this comment.
[💡 readability]
The use of String(memberIdRaw) in the getMemberHandleMap call is redundant since memberIdRaw is already a string. Consider removing the String() conversion for clarity.
| [ | ||
| { | ||
| userId: numericMemberId, | ||
| handle: handle && handle.length ? handle : String(memberIdRaw), |
There was a problem hiding this comment.
[💡 style]
The conditional check handle && handle.length is unnecessary. handle.length alone suffices to check if the string is non-empty.
| appealsSuccessors, | ||
| ); | ||
|
|
||
| if (callbackResult instanceof Promise) { |
There was a problem hiding this comment.
[💡 maintainability]
The check for callbackResult instanceof Promise is redundant since await can handle both promises and non-promises. Consider removing this check to simplify the code.
| ) | ||
| : result.next.phases; | ||
|
|
||
| if (!phasesToOpen.length) { |
There was a problem hiding this comment.
[💡 performance]
The check if (!phasesToOpen.length) is performed after filtering result.next.phases. If result.next.phases is empty, the filter operation is unnecessary. Consider checking the length of result.next.phases before filtering to avoid unnecessary operations.
No description provided.