-
Couldn't load subscription status.
- Fork 4.7k
fix: remove redundant base64 handler #2146
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
base: main
Are you sure you want to change the base?
Conversation
Reviewer's guide (collapsed on small PRs)Reviewer's GuideRefactors base64 handling in ChannelStartupService by dropping redundant mediaUrl backup/restore and simplifying the deletion of the base64 field within the message existence check. Class diagram for updated ChannelStartupService message cleaningclassDiagram
class ChannelStartupService {
+cleanedMessage: object
+cleanMessage(message: object): object
}
class Message {
base64: string
mediaUrl: string
imageMessage: object
}
ChannelStartupService --|> Message
%% Changes:
%% - Removed redundant mediaUrl backup/restore logic
%% - Simplified base64 field deletion within message existence check
%% - No longer restores mediaUrl after cleaning
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey there - I've reviewed your changes - here's some feedback:
- There’s a duplicate delete of cleanedMessage.message.base64; remove the redundant invocation to avoid confusion.
- You’ve removed the mediaUrl preservation logic—double-check that this won’t inadvertently strip mediaUrl from your API response.
- Consider flattening the base64 cleanup block instead of nested ifs to improve readability.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- There’s a duplicate delete of cleanedMessage.message.base64; remove the redundant invocation to avoid confusion.
- You’ve removed the mediaUrl preservation logic—double-check that this won’t inadvertently strip mediaUrl from your API response.
- Consider flattening the base64 cleanup block instead of nested ifs to improve readability.
## Individual Comments
### Comment 1
<location> `src/api/services/channel.service.ts:539-540` </location>
<code_context>
- delete cleanedMessage.message.base64;
-
if (cleanedMessage.message) {
+ if (cleanedMessage.message.base64) {
+ delete cleanedMessage.message.base64;
+ }
+
</code_context>
<issue_to_address>
**suggestion:** Redundant deletion of 'base64' property from cleanedMessage.message.
Only one deletion is needed; please remove the redundant statement.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| if (cleanedMessage.message.base64) { | ||
| delete cleanedMessage.message.base64; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion: Redundant deletion of 'base64' property from cleanedMessage.message.
Only one deletion is needed; please remove the redundant statement.
📋 Description
This part of the code returns
{
"status": 500,
"error": "Internal Server Error",
"response": {
"message": "Cannot read properties of null (reading 'mediaUrl')"
}
}
in /findChats/ API endpoint, e.g:
http://localhost:8080/chat/findChats/EVO-EED-4754-J
🔗 Related Issue
None created yet
🧪 Type of Change
🧪 Testing
📸 Screenshots (if applicable)
✅ Checklist
📝 Additional Notes
Summary by Sourcery
Streamline ChannelStartupService’s message cleaning by removing unnecessary base64 and mediaUrl handling to prevent runtime errors
Bug Fixes:
Enhancements: