Skip to content
This repository has been archived by the owner on Nov 25, 2024. It is now read-only.

Fix failing federation tests #1579

Closed
wants to merge 4 commits into from

Conversation

S7evinK
Copy link
Contributor

@S7evinK S7evinK commented Nov 12, 2020

This solves #1327 (notifications test is already passing)

I'm a bit unhappy with getting the power levels, is there a better way?

Pull Request Checklist

  • I have added any new tests that need to pass to sytest-whitelist as specified in docs/sytest.md
  • Pull request includes a sign off

rsAPI api.RoomserverInternalAPI,
event *gomatrixserverlib.Event,
) (ok bool, response *util.JSONResponse) {
req := api.QueryLatestEventsAndStateRequest{RoomID: roomID}
Copy link
Contributor

Choose a reason for hiding this comment

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

We should probably be setting StateToFetch in req so that we aren't pulling out lots of state events that we don't need - ultimately we only care about the create event and the existing power level event.

break
}
}
if event.Sender() != creator && r.ServerName != event.Origin() {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think this check is doing the right thing - ultimately what we want to do is check if the event is sent by the same creator only if there isn't a power level event already in the room.

If there is an existing power level event then we don't need to do this, as the soft-fail check will capture it if the current state events don't allow the new event.

}
}

if creator != "" && event.Sender() != creator {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't know if we need a check in the client API for this? The roomserver should reject an event that it isn't happy with, and the client API won't be involved in remote servers pushing state to us.

@S7evinK S7evinK marked this pull request as draft December 18, 2020 18:38
@kegsay kegsay added the stale This issue or PR is at risk of being closed without further feedback label Feb 22, 2021
@kegsay
Copy link
Member

kegsay commented Feb 22, 2021

This PR has been marked as stale and will be closed in 1 week if there are no further commits.

@S7evinK
Copy link
Contributor Author

S7evinK commented Feb 27, 2021

Closed, for being stale and waiting for #1441.

@S7evinK S7evinK closed this Feb 27, 2021
@kegsay
Copy link
Member

kegsay commented Jul 14, 2021

This was fixed in #1920

@S7evinK S7evinK deleted the fix-fed-powerlevels branch August 14, 2021 10:58
brianathere pushed a commit to HereNotThere/dendrite that referenced this pull request Mar 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
stale This issue or PR is at risk of being closed without further feedback
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants