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
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.

2 participants