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

Implement API for username availability #432

Merged
merged 2 commits into from
May 11, 2017
Merged

Implement API for username availability #432

merged 2 commits into from
May 11, 2017

Conversation

lukebarnard1
Copy link
Contributor

@lukebarnard1 lukebarnard1 commented May 10, 2017 •
edited
Loading

Requires synapse with matrix-org/synapse#2183 and matrix-org/synapse#2209

Update: this needs to be changed to reflect matrix-org/synapse#2213

@lukebarnard1 lukebarnard1 mentioned this pull request May 10, 2017
Closed
lukebarnard1 pushed a commit to matrix-org/matrix-react-sdk that referenced this pull request May 10, 2017
Redesign mxID chooser, add availability checking
ad2ed12 
Requires matrix-org/matrix-js-sdk#432 for availability checking.

Changes:
 - Redesign the dialog to look more like element-hq/element-web#3604 (comment)
 - Attempt to fix wrong password being stored by generating one per SetMxIdDialog (there's no issue tracking this for now, I shall open one if it persists)
 - Backwards compatible with servers that don't support register/availability - a spinner will appear the first time a username is checked because server support can only be determined after a request.
 - Rate-limited by a 2s debounce
 - General style improvements
@lukebarnard1 lukebarnard1 mentioned this pull request May 10, 2017
Merged
@krombel
Copy link
Contributor

krombel commented May 10, 2017

To be correct it requires matrix-org/synapse#2209

@lukebarnard1
Copy link
Contributor Author

thanks, @krombel

@richvdh
Copy link
Member

richvdh commented May 10, 2017

looks ok, but why is this a POST rather than a GET?

@richvdh richvdh assigned lukebarnard1 and unassigned richvdh May 10, 2017
@lukebarnard1
Copy link
Contributor Author

@richvdh I made the mistake of thinking a GET can include a body, which inherently it cannot. Hence matrix-org/synapse#2209

@richvdh
Copy link
Member

richvdh commented May 10, 2017

yes, but why put the user id in the body rather than a query param?

@lukebarnard1
Copy link
Contributor Author

yes, but why put the user id in the body rather than a query param?

Good question. When I originally proposed this, the response was largely "small things like this don't need a proposal". Things like this would probably have been looked at if a proposal was made.

@lukebarnard1
Copy link
Contributor Author

I shall modify this PR to reflect the changes to the synapse API - it shall be a GET with a query parameter instead.

@lukebarnard1 lukebarnard1 assigned richvdh and unassigned lukebarnard1 May 10, 2017
richvdh
richvdh approved these changes May 10, 2017
View reviewed changes
Copy link
Member

@richvdh richvdh left a comment

Choose a reason for hiding this comment

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

lgtm

@richvdh richvdh assigned lukebarnard1 and unassigned richvdh May 10, 2017
@lukebarnard1 lukebarnard1 merged commit cb9a9e8 into develop May 11, 2017
@lukebarnard1
Copy link
Contributor Author

lukebarnard1 commented May 11, 2017 •
edited
Loading

@krombel if the endpoint is not available on the server, a MatrixError will be raised with the errcode set to M_UNRECOGNIZED but this is generated by the server itself.

@krombel
Copy link
Contributor

krombel commented May 11, 2017

OK. Good to know. Thanks

@t3chguy t3chguy deleted the luke/feature-username-availability branch May 10, 2022 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richvdh richvdh richvdh approved these changes

Assignees

@lukebarnard1 lukebarnard1

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@lukebarnard1 @krombel @richvdh