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.

Sign up for GitHub

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

Jump to bottom

Add a test for restricted rooms appearing in the spaces summary (MSC3083). #109

Merged
merged 12 commits into from
Jun 2, 2021
Merged

Add a test for restricted rooms appearing in the spaces summary (MSC3083). #109

Changes from 1 commit
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
c5adad1
Add a test for restricted rooms appearing in the spaces summary.
clokep Apr 28, 2021
bfd9cd9
Add tests for returning restricted rooms for the spaces summary over …
clokep May 13, 2021
d1e32cb
Abstract some code.
clokep May 13, 2021
9e3fed2
Merge branch 'master' into clokep/restricted-rooms-in-space-summary
kegsay May 14, 2021
63a3021
Remove namespace
kegsay May 14, 2021
32532b0
Remove incorrect comment.
clokep May 17, 2021
0114134
Do not export functions.
clokep May 17, 2021
b6a7e0d
Merge remote-tracking branch 'origin/master' into clokep/restricted-r…
clokep May 17, 2021
cdeb591
Merge remote-tracking branch 'origin/master' into clokep/restricted-r…
clokep May 20, 2021
e731fe2
Review comments.
clokep Jun 2, 2021
d1855cb
Fix typos.
clokep Jun 2, 2021
469891e
More clarifying comments.
clokep Jun 2, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions tests/msc3083_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,10 @@ func TestRestrictedRoomsSpacesSummary(t *testing.T) {
//
// The user should be unable to see the room in the spaces summary unless they
// are a member of the space.
//
// This tests the interactions over federation where the space and room are on
// difference homeservers, which might not have the proper information needed to
clokep marked this conversation as resolved.
Show resolved Hide resolved
// decide if a user is in a room.
func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) {
deployment := Deploy(t, b.BlueprintFederationTwoLocalOneRemote)
defer deployment.Destroy(t)
Expand All @@ -265,6 +269,7 @@ func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) {
bob := deployment.Client(t, "hs1", "@bob:hs1")
space := alice.CreateRoom(t, map[string]interface{}{
"preset": "public_chat",
clokep marked this conversation as resolved.
Show resolved Hide resolved
"name": "Space",
"initial_state": []map[string]interface{}{
{
"type": "m.room.history_visibility",
Expand Down Expand Up @@ -310,11 +315,13 @@ func TestRestrictedRoomsSpacesSummaryFederation(t *testing.T) {
},
})

// The room appears for no one at first since hs2 doesn't know about who is in ss1.
// The room appears for neither alice or bob initially. Although alice is in
// the space and should be able to access the room, hs2 doesn't know this!
requestAndAssertSummary(t, alice, space, []interface{}{space})
requestAndAssertSummary(t, bob, space, []interface{}{space})

// Charlie joins ss1 and now hs2 knows that alice is in it.
// charlie joins the space and now hs2 knows that alice is in the space (and
// can join room).
clokep marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

My main question revolves around why hs2 needs to know alice can join the room if we're asking hs1 for the space summary. hs1 knows alice can join the room already.

My assumption is that hs1 needs to ask hs2 this question, but it'd be good to mention that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, hs1 is asking hs2 about "room" while generating the summary. I'll add this.

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated the docstring which talks about the setup of space/room, I hope that's OK. The above is true for every time we attempt to summarize so that seemed like the best spot.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks! And aha, I think my mental model had Alice creating the room, not Charlie, that's why I was confused about why hs1 didn't know about the status of the room.

That realisation and the additional notes make things fairly clear now, thanks!

charlie.JoinRoom(t, space, []string{"hs1"})

// The restricted room should appear for alice (who is in the space).
anoadragon453 marked this conversation as resolved.
Show resolved Hide resolved
Expand Down