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.

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

Unconfirmed user #354

Closed
wants to merge 16 commits into from
Closed

Unconfirmed user #354

wants to merge 16 commits into from

Conversation

mariha
Copy link
Collaborator

@mariha mariha commented Apr 14, 2020

When user creates an account the e-mail address they provided needs to be verified. Util they confirm, they can log in but they don't have access to most resources so the server returns 403 errors. I changed the code so that they get a message with some hints what is going on. This solves #327.

This could happen when loading users on the Map, Searching for specific users or checking Messages. I updated all these places. We might also consider sending a request to check permissions when user logs in for the first time on the device (seems for me more intuitive).

I added filtering to exception handling. It is based on http error codes, so that:

  • 403 Forbidden response results in returning access_denied string from the resources (added), mapped to
    Access denied. Please check your email for account confirmation message. (proposition)
  • 500 Internal server error response results in returning internal_server_error string (added), mapped to
    Oops... This is on us, sorry! Something has gone wrong on the server side. Please try again later.

Also:

  • message_thread_create_failed, messages_reload_failed and message_send_failed are not used anywhere any more so I removed them

I was thinking of changing error handling, moving towards having one place where different exception types and http error codes are translated to messages and displayed, so that it is more uniform across that app and we could easily hook some more robust logging/analytics. So far I started to placed it in HttpErrorHelper. And I changed Errors to HttpExceptions where applicable so we could differentiate exception handling behavior based on ex type and error code.

I tried (quite hard) to write tests for my changes, gave up with threads though. I still hope to come back to it later.

mariha and others added 16 commits March 30, 2020 13:01
… is not confirmed yet.

`403 Forbidden` and `500 Internal server error` responses are handled for loading users on a Map and loading threads of Messages. Accordingly `access_denied` and `internal_server_error` strings are added to the resources.
… is not confirmed yet.

`403 Forbidden` and `500 Internal server error` responses are handled for searching for users.
…icable. This way HttpErrorHelper can differentiate exception handling behaviour based on the exception type.
Single quotes instead of double quotes resulted in WS_API_USER_ID and
WS_API_KEY being set to variable names instead of the variables'
contents.
The automatic message reloading was not working properly, especially
after a reboot of the device.

This also increases the default reload interval to 15 minutes.
Co-authored-by: Simone <cioccasimone97@users.noreply.github.com>
Co-authored-by: Jakob <jakobhanna@users.noreply.github.com>
Closes: warmshowers#353
Adjusted the TravisCI deploy script to monitor the new location of the
deobfuscation file.
…ad_create_failed and messages_reload_failed)
…t they were doing so instead of simple failure message may be more interested in what actually went wrong.

Remove message_send_failed from the resources as it is not used anywhere any more.
@mariha
Copy link
Collaborator Author

mariha commented Apr 15, 2020

Hey @saemy, it should be good for the review. I thought I rebased my branch on warmshowrs/develop, looks like I didn't though... so I had to merge at the end. If it was an issue let me know and I'll create fresh branch with clean history.

@mariha
Copy link
Collaborator Author

mariha commented Apr 17, 2020

Please disregard this PR, I created #355 instead.

@mariha mariha closed this Apr 17, 2020
@mariha mariha deleted the unconfirmed_user branch April 17, 2020 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants