Skip to content
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

feat(92): watcher leaving a room, no room deletion #186

Draft
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

mourtisma
Copy link
Contributor

  • Added the leaving the room feature, with a room containing 2 or more watchers
  • Moved findAndEnsureRoomJoined to room-operations

TODO:

  • Delete the room if there's no more watcher
  • Add more tests

backend/functions/disconnect.ts Outdated Show resolved Hide resolved
) => {
if (!room.watchers[watcherConnectionString]) {
console.log(
'[WS-S] A media event was received for someone ho has not joined the room. Dropping',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'[WS-S] A media event was received for someone ho has not joined the room. Dropping',
'[WS-S] A media event was received from someone who has not joined the room. Dropping',

Comment on lines 213 to 217
ensureRoomJoined(room, watcherConnectionString);
delete room.watchers[watcherConnectionString];

// TODO delete the room if there is no watcher
return updateRoom(room, tableName, dynamoDb);
Copy link
Contributor

@Startouf Startouf Jul 4, 2020

Choose a reason for hiding this comment

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

Non pour le coup pas possible d'utiliser updateRoom pour retirer un watcher. Il nous faut obligatoirement une commande AWS DynamoDB qui supprime la clé + le sous-doc uniquement, et pas une fonction qui mette à jour toute la liste des watchers (ce qui est le cas d'updateRoom ajd je crois).

EDIT : non en réalité justement pour cette raison, la méthode updateRoom ne met pas à jour la liste des watchers, donc ce code ne marchera tt simplement pas

SInon, ça va péter car on aura des pb de race condition avec les autres watchers.

@sonarcloud
Copy link

sonarcloud bot commented Jul 4, 2020

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities (and Security Hotspot 0 Security Hotspots to review)
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants