fix/updated useFindManyRecords for if messageChannels is null it returns null #7887
Conversation
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR adds a check for empty message channels in the canAccessMessageThread method to address potential errors when processing message threads.
- Added
if(messageChannels.length===0) return []inpackages/twenty-server/src/modules/messaging/common/query-hooks/message/can-access-message-thread.service.ts - The new check prevents further processing when no message channels are found, potentially avoiding errors with empty arrays
- Consider handling the case where
messageChannelsisnullorundefined, not just empty - The return value
[]may not be appropriate for this method, as it's not clear how this affects the access control logic - Review the overall error handling strategy in this method to ensure consistent behavior across different scenarios
1 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings | Greptile
| @@ -54,6 +54,7 @@ export class CanAccessMessageThreadService { | |||
| 'connectedAccount', | |||
| ); | |||
|
|
|||
| if(messageChannels.length===0) return [] | |||
There was a problem hiding this comment.
style: Consider using return [] instead of return
| @@ -54,6 +54,7 @@ export class CanAccessMessageThreadService { | |||
| 'connectedAccount', | |||
| ); | |||
|
|
|||
| if(messageChannels.length===0) return [] | |||
There was a problem hiding this comment.
style: Add space after if for consistency
| @@ -54,6 +54,7 @@ export class CanAccessMessageThreadService { | |||
| 'connectedAccount', | |||
| ); | |||
|
|
|||
| if(messageChannels.length===0) return [] | |||
There was a problem hiding this comment.
logic: This check should be placed earlier in the method, before using messageChannels
Welcome!
Hello there, congrats on your first PR! We're excited to have you contributing to this project. |
|
Hello @krVatsal, thank you for contributing :) Actually, I think we would prefer to throw an error here, because it shouldn't happen. Also, this won't fix the underlying bug, I still have to investigate to find what is causing this behavior. |
|
/award 100 |
|
Awarding krVatsal: 100 points 🕹️ Well done! Check out your new contribution on oss.gg/krVatsal |
Fixed #7830
Include a check of
if(messageChannels.length===0) return []that if the messageChannels is coming as null then just return and don't perform any further operation, it will allow to go forward only if messageChannels.length>0