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

Move confirmation_pending to CommunityMemberships controller #1976

Merged
merged 1 commit into from
Apr 28, 2016

Conversation

rap1ds
Copy link
Member

@rap1ds rap1ds commented Apr 28, 2016

Merge this only after #1960

This PR moves confirmation_pending action from SessionsController to CommunityMemberships controller. pending_email_confirmation is one of the membership statuses, so it makes sense to have the action in CommunityMemberships controller. It has nothing to do with session creation, because session has been created already before user sees this page.

This PR also fixes one small but super annoying bug in the app:

Steps to reproduce:

  1. Sign up
  2. When you see the confirmation page, open another tab and go to your webmail
  3. Click the link in the confirmation email
  4. Go back to the tab where the confirmation page is open
  5. Refresh

Expected result: You are redirected to front-page. You see a message which says that you are already a member.

Actual result: No redirect. The page looks a bit broken because the email is missing.

…oller

- [X] Move confirmation_pending to a more logical place
- [X] Redirect to root if user is already accepted
@rap1ds rap1ds force-pushed the clean-sessions-controller branch from 304d731 to 262035e Compare April 28, 2016 11:51
@sktoiva sktoiva self-assigned this Apr 28, 2016
@ovan
Copy link
Contributor

ovan commented Apr 28, 2016

Looks good.

@sktoiva
Copy link
Contributor

sktoiva commented Apr 28, 2016

LGTM

@rap1ds rap1ds merged commit 083208b into master Apr 28, 2016
@rap1ds rap1ds deleted the clean-sessions-controller branch April 28, 2016 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants