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

Add support for accepting invites #125

Merged
merged 3 commits into from
Feb 24, 2015
Merged

Add support for accepting invites #125

merged 3 commits into from
Feb 24, 2015

Conversation

mixonic
Copy link
Contributor

@mixonic mixonic commented Feb 23, 2015

@bantic for you to review and merge.

Invites can be accepted by signed in users. Users who are not signed in will be redirected to login/signup and redirected back once they
complete the process.

The invite page itself renders, then starts the AJAX request to claim the invitation automatically.

URLs such as:

  • http://localhost:4200/claim?verification_code=abc&invitation_id=59ef9caa-292c-42b0-b90f-000b7432b743
  • http://localhost:4200/claim/59ef9caa-292c-42b0-b90f-000b7432b743/abc-verification-code

Are supported. This is compatible with emails out there in the wild.

This PR introduces general support for redirecting to a path after signing in. See the attemptedTransition property.

this.controllerFor('claim').set('error', `
There was an error accepting this invitation. Perhaps
the verification code has expired?
`);
Copy link
Contributor

Choose a reason for hiding this comment

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

Doing this in didTransition allows the template to render — is that the reason for doing this in didTransition? I'm not clear on the pros/cons of async in this hook. Can the user get in a weird state by clicking away while this save is still inFlight?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There isn't anything to click on, the layout is akin to a login box.

The goal is to show some content on the page which the verification is happening. To show a box letting the user know something is going on.

As I've gone though the passwords flow, I started thinking about this UX more. IMO, at the least the user should see what their current email address is before they accept an invitation. This would help protect from automatic acceptance of an invite when a user clicks on a link.

In fact now that I think of it more, this is technically an XSS attack 0_o. You could make an arbitrary user accept an invite just by linking them, or even opening the URL in an iframe.

So, this flow should really be replaced with a button. @sandersonet if that sounds good to you I can refactor this to have a box prompting the user with whatever easy data we already have for now. At least their current email.

Copy link
Contributor

Choose a reason for hiding this comment

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

👍 to the button

@bantic
Copy link
Contributor

bantic commented Feb 23, 2015

cool — I had a question about the impact of doing the async in the didTransition hook. I'm curious if that can be problematic, but I'm sure it's fine. This LGTM

@skylar-anderson skylar-anderson mentioned this pull request Feb 24, 2015
62 tasks
Invites can be accepted by signed in users. Users who are not signed in
will be redirected to login/signup and redirected back once they
complete the process. This also introduces general support for
redirecting to a path after signin in.
@mixonic
Copy link
Contributor Author

mixonic commented Feb 24, 2015

Adds a button, addresses a few items caught by @bantic.

The screen for acceptance is very, very simple:

Significantly it does't have the organization name. This is passed with the URL from the email, but we should not be trusting that value. Instead, the API needs to be updated to allow us to easily fetch the organization for an invite. We can load that in the model hook and display the name here.

IMO there should also be a "this isn't me" button, or a "cancel" button.

I'd like to make those items a separate issue and keep moving forward though.

@bantic I think this is good to go.

@bantic
Copy link
Contributor

bantic commented Feb 24, 2015

nice. I like the look of that button. So inviting. So accepting.

bantic added a commit that referenced this pull request Feb 24, 2015
Add support for accepting invites
@bantic bantic merged commit a8fcc0a into aptible:master Feb 24, 2015
@bantic bantic deleted the verify branch February 24, 2015 20:37
skylar-anderson pushed a commit to skylar-anderson/dashboard.aptible.com that referenced this pull request Aug 22, 2016
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.

None yet

3 participants