This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Drop support for calling
/_matrix/client/v3/rooms/{roomId}/invite
without anid_access_token
#13241Drop support for calling
/_matrix/client/v3/rooms/{roomId}/invite
without anid_access_token
#13241Changes from 30 commits
0f6487e
ef1c794
e9c789a
e354ec6
eb5ba09
72313f6
779e78c
5668dcd
6e908e4
242fa48
6698279
48d88f0
6b1413b
dacd33d
9a04234
e115f60
c535ea8
f2a74b2
bb84fe4
8ab4b35
f9600bb
0073118
ebcdbba
b97b27f
23be3b9
d1b6ae8
21d9be1
caeee4d
3792e8c
3dfd8fb
5b237aa
27951c5
c6aa582
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
generally we try to use
http.HTTPStatus
constants here (you may need to add an import):likewise in the other place you do this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suppose we have an
invite_3pid
withid_server
andid_access_token
but nomedium
. We will still log this error. That seems incorrect.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if I understand correctly, medium is always optional and does not have to be in the invite? Can it be ommited or should another error be raised?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Although I think it would just have to log "all of the (4) fields need to be present in an invite, caused by:" if the if above it is correct
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
medium is not optional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, so in newest version all these properties are mentioned as mandatory in error message.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
since you're rewriting this anyway: I think you may as well inline it. Having a separate function just makes it harder to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should only do this check if membership_action == "invite"