/synapse

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Require user to have left room to forget room #673

Merged
merged 3 commits into from Mar 30, 2016

Conversation

Reviewers
No reviews
Assignees

@NegativeMjark NegativeMjark

Labels
None yet
Projects
None yet
Milestone
No milestone
3 participants
@erikjohnston @NegativeMjark @oddvar
@erikjohnston
Owner

erikjohnston commented Mar 30, 2016

This dramatically simplifies the forget API code - in particular it no longer generates a leave event.

matrix-org/sytest#220
@erikjohnston
Require user to have left room to forget room 
This dramatically simplifies the forget API code - in particular it no
longer generates a leave event.

All checks have passed

8 successful checks
@matrixbot
Flake8 + Packaging (Commit) — Build #221 origin/erikj/forget succeeded in 28 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #224 origin/erikj/forget succeeded in 5 min 8 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #225 origin/erikj/forget succeeded in 3 min 57 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #266 origin/erikj/forget succeeded in 1 min 10 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
fddb6fd

@erikjohnston erikjohnston assigned NegativeMjark Mar 30, 2016

@oddvar oddvar added the in progress label Mar 30, 2016

This was referenced Mar 30, 2016

Merged
Closed

@NegativeMjark NegativeMjark commented on an outdated diff Mar 30, 2016

View full outdated diff
synapse/handlers/room.py
- return self.store.forget(user.to_string(), room_id)
+ user_id = user.to_string()
+
+ member = yield self.state_handler.get_current_state(
+ room_id=room_id,
+ event_type=EventTypes.Member,
+ state_key=user_id
+ )
+ membership = member.membership if member else None
+
+ if membership is not None and membership != Membership.LEAVE:
+ raise SynapseError(400, "User %s in room %s" % (
+ user_id, room_id
+ ))
+
+ yield self.store.forget(user_id, room_id)
@NegativeMjark

NegativeMjark Mar 30, 2016

Contributor

Does store.forget work if the user doesn't have a membership in the room?

@NegativeMjark NegativeMjark commented on an outdated diff Mar 30, 2016

View full outdated diff
synapse/rest/client/v1/room.py
@@ -405,6 +405,43 @@ def on_GET(self, request, room_id, event_id):
defer.returnValue((200, results))
+class RoomForgetRestServlet(ClientV1RestServlet):
+ def register(self, http_server):
+ # /rooms/$roomid/[invite|join|leave]
@NegativeMjark

NegativeMjark Mar 30, 2016

Contributor

The comment is probably misleading.

erikjohnston added some commits Mar 30, 2016

@erikjohnston
Remove spurious comment
08a8514
@erikjohnston
Only forget room if you were in the room

All checks have passed

8 successful checks
@matrixbot
Flake8 + Packaging (Commit) — Build #226 origin/erikj/forget succeeded in 25 sec
Details
@matrixbot
Flake8 + Packaging (Merged PR) — Build finished.
Details
@matrixbot
Sytest Postgres (Commit) — Build #229 origin/erikj/forget succeeded in 5 min 24 sec
Details
@matrixbot
Sytest Postgres (Merged PR) — Build finished.
Details
@matrixbot
Sytest SQLite (Commit) — Build #230 origin/erikj/forget succeeded in 4 min 12 sec
Details
@matrixbot
Sytest SQLite (Merged PR) — Build finished.
Details
@matrixbot
Unit Tests (Commit) — Build #271 origin/erikj/forget succeeded in 1 min 14 sec
Details
@matrixbot
Unit Tests (Merged PR) — Build finished.
Details
73b6bf4
@erikjohnston
Owner

erikjohnston commented Mar 30, 2016

@NegativeMjark PTAL
@NegativeMjark
Contributor

NegativeMjark commented Mar 30, 2016

Should forgetting fail if you aren't in the room?

Otherwise LGTM
@erikjohnston
Owner

erikjohnston commented Mar 30, 2016

Should forgetting fail if you aren't in the room?

I quite like that it doesn't, since then you can't use the API to tell if you had forgotten the room or were never in it.

@erikjohnston erikjohnston merged commit 178c9fb into develop Mar 30, 2016
8 checks passed

8 checks passed

Flake8 + Packaging (Commit) Build #226 origin/erikj/forget succeeded in 25 sec
Details
Flake8 + Packaging (Merged PR) Build finished.
Details
Sytest Postgres (Commit) Build #229 origin/erikj/forget succeeded in 5 min 24 sec
Details
Sytest Postgres (Merged PR) Build finished.
Details
Sytest SQLite (Commit) Build #230 origin/erikj/forget succeeded in 4 min 12 sec
Details
Sytest SQLite (Merged PR) Build finished.
Details
Unit Tests (Commit) Build #271 origin/erikj/forget succeeded in 1 min 14 sec
Details
Unit Tests (Merged PR) Build finished.
Details

@oddvar oddvar removed the in progress label Mar 30, 2016

@richvdh richvdh deleted the erikj/forget branch Dec 1, 2016

@non-Jedi non-Jedi referenced this pull request in matrix-org/matrix-doc Oct 2, 2017

Open

Document that users must now call the endpoint to leave a room before calling endpoint to forget it #1011

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