Skip to content
This repository was archived by the owner on Jun 10, 2019. It is now read-only.

Conversation

@apex-omontgomery
Copy link
Member

Includes the route to the backend without the api

Will still need to add the environment variable for this in k8s.

Copy link
Member

@kylemh kylemh left a comment

Choose a reason for hiding this comment

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

this isn't the fix we want...

why doesn't the password reset work with /v1 appended to the API url?

@apex-omontgomery
Copy link
Member Author

I replied in the backend, oops:
OperationCode/operationcode_backend#379 (comment)

validatePasswordConfirm = value => value === '' || value === this.state.password;

handleOnClick = (e) => {
handleOnClick = e => {
Copy link

Choose a reason for hiding this comment

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

Travis isn't liking this bit

./src/scenes/home/resetPassword/setPassword/setPassword.js
  Line 34:  Expected parentheses around arrow function argument having a body with curly braces  arrow-parens
Search for the keywords to learn more about each error.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
make: *** [build_site] Error 1

@apex-omontgomery
Copy link
Member Author

Fixed the things, added api/v1 and hooked into sentry logging so we know sooner when this happens.

Also added a password change route at api/v1/update if you want to make a form or something for that. You can probably merge this while we wait to push to backend, or you can wait.

})
.catch(() => {
.catch((err) => {
Raven.setUserContext({
Copy link
Member

Choose a reason for hiding this comment

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

this shouldn't actually be necessary, as raven catches all exceptions, but i'll leave it cuz maybe context is helpful

Copy link
Member Author

Choose a reason for hiding this comment

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

Context was the only reason I put this in here.

this.setState({ success: true, error: null });
})
.catch(() => {
this.setState({ error: 'We were unable to set the password for this email' });
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should add next steps in this message, such as: "please email staff@operationcode.org for help"

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 think the toasts are fairly small, and this is a big amount of text, what would you recommend?

Copy link
Member

Choose a reason for hiding this comment

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

Just the next steps then. It will be red indicating a failure. It's not accessible, but in terms of getting a message across, let's just say to email us.

Also, is the toaster fixed height? I would hope that would expand to fit content.

Copy link
Member

@kylemh kylemh left a comment

Choose a reason for hiding this comment

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

lgtm - moved the import to appease linting

@kylemh kylemh merged commit a648353 into master Nov 7, 2018
@kylemh kylemh deleted the change_host branch November 7, 2018 01:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants