Skip to content

Conversation

itsisak
Copy link
Contributor

@itsisak itsisak commented May 12, 2025

Description

Send websocket messages for new comments and deleted comments.

Currently a bit unsure of how i should implement the groups.

There should imo be a seperate group for each comment section, such that you only get messages from those you have access to (eg. "comments-events.event-1"). The issue is that always connecting to all groups a user has access to seems inefficient since there exists so many events, meetings etc.

Testing

  • The code quality is at a minimum required level of quality, readability, and performance.
  • I have thoroughly tested my changes.

Resolves ABA-1465

@itsisak itsisak requested a review from eikhr May 12, 2025 10:40
@itsisak itsisak self-assigned this May 12, 2025
@itsisak itsisak added new-feature Pull requests that introduce or issues that suggest a new feature review-needed Pull requests that need review labels May 12, 2025
Copy link

linear bot commented May 12, 2025

@itsisak itsisak marked this pull request as draft May 12, 2025 10:41
Comment on lines 19 to +23
def to_representation(self, value):
return None
try:
return instance_to_string(value)
except Exception:
pass
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Dont know why GenericRelationField was write-only, it prevented comments from returning content_target. If it should stay how it was, it is easy enough to add it to the websocket meta data instead of this.

Copy link
Contributor

@Arashfa0301 Arashfa0301 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice job

@itsisak itsisak marked this pull request as ready for review May 29, 2025 15:01
@itsisak itsisak force-pushed the comment-websockets branch from f2a5963 to 212caca Compare May 29, 2025 15:05
Copy link

codecov bot commented Sep 17, 2025

Codecov Report

❌ Patch coverage is 34.04255% with 62 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.79%. Comparing base (84c2f53) to head (3846fff).
⚠️ Report is 7 commits behind head on master.

Files with missing lines Patch % Lines
lego/apps/websockets/consumers.py 0.00% 33 Missing ⚠️
lego/apps/websockets/groups.py 31.25% 11 Missing ⚠️
lego/apps/websockets/constants.py 0.00% 10 Missing ⚠️
lego/apps/comments/websockets.py 60.00% 4 Missing ⚠️
lego/apps/comments/action_handlers.py 50.00% 2 Missing ⚠️
lego/utils/serializers.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3835      +/-   ##
==========================================
- Coverage   89.97%   89.79%   -0.19%     
==========================================
  Files         763      767       +4     
  Lines       24401    24477      +76     
==========================================
+ Hits        21956    21978      +22     
- Misses       2445     2499      +54     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@eikhr eikhr removed their request for review September 22, 2025 12:22
Copy link
Contributor

@ch0rizo ch0rizo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! 🐺

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-feature Pull requests that introduce or issues that suggest a new feature review-needed Pull requests that need review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants