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

In Site Registration #76

Merged
merged 21 commits into from Sep 14, 2012

Conversation

Projects
None yet
2 participants
@jordanbyron
Copy link
Member

jordanbyron commented Sep 11, 2012

Shiny new registration process for Practicing Ruby without all those nasty MailChimp dependencies.

@jordanbyron

This comment has been minimized.

Copy link
Member

jordanbyron commented Sep 11, 2012

@sandal when you have a moment can you review and update the text on the registration pages. Once that is all set we can merge this into master and deploy it.

@practicingruby

This comment has been minimized.

Copy link
Member

practicingruby commented Sep 12, 2012

@jordanbyron: Copy is updated, I think this is ready to go.

practicingruby and others added some commits Sep 12, 2012

@jordanbyron

This comment has been minimized.

Copy link
Member

jordanbyron commented on app/views/home/subscribe.html.haml in fb20296 Sep 12, 2012

What the hell is this? No Inline styles! WHO ARE YOU!! 😱

@practicingruby

This comment has been minimized.

Copy link
Member

practicingruby commented on fb20296 Sep 12, 2012

I started with inline styles! Then I figured I had complained about enough stuff today, I didn't want to do yet another thing to annoy you :)

@jordanbyron

This comment has been minimized.

Copy link
Member

jordanbyron commented Sep 12, 2012

@sandal I've made those few changes we discussed to the problems page. Just take a look at the text on that page and make sure it looks good to you. Thanks! 😄

@practicingruby

This comment has been minimized.

Copy link
Member

practicingruby commented Sep 12, 2012

@jordanbyron: I think the tests should verify the whole resubscirbe process rather than just verifying that they can reach the subscribe page. In theory, that should take mostly the same path through the code, but I think a complete test would help us catch current or future edge cases.

Other than that, looks good.

@jordanbyron

This comment has been minimized.

Copy link
Member

jordanbyron commented Sep 13, 2012

@sandal I fixed up those tests as requested. If everything looks good I'll try deploying again tomorrow.

@practicingruby

This comment has been minimized.

Copy link
Member

practicingruby commented Sep 13, 2012

Make it so.

@jordanbyron

This comment has been minimized.

Copy link
Member

jordanbyron commented Sep 13, 2012

Make it so

@practicingruby

This comment has been minimized.

Copy link
Member

practicingruby commented Sep 13, 2012

@jordanbyron: One question, what happens when a previous subscriber attempts to visit the subscribe page while not logged in? Does it authenticate them and display the problems page, and then they need to click the resubscribe link? If so, that's acceptable, but it may be better to handle that automatically so that they can seamlessly resubscribe without having to explicitly hit the /restart route.

This is not a blocker for deploying this code unless it somehow leads to an inconsistent state to visit /subscribe as a former subscriber that is not logged in, but is something we should look into.

@jordanbyron

This comment has been minimized.

Copy link
Member

jordanbyron commented Sep 13, 2012

What happens when a previous subscriber attempts to visit the subscribe page while not logged in? Does it authenticate them and display the problems page, and then they need to click the resubscribe link?

Yes that is the flow. Initially I thought about just re-routing them to the registration process, but that could be super confusing if they never knew their account was disabled by MailChimp.

I also considered changing the MailChimp webhooks to mark those accounts as status = 'payment_pending', however I thought that would just make things even more confusing when we asked them to start paying down the road. Most user's accounts are canceled without their knowledge and I think it's best if we show them the problems page and have them go through the full registration process.

Let me know what you think. I plan on deploying this branch within the next hour or so.

@practicingruby

This comment has been minimized.

Copy link
Member

practicingruby commented Sep 13, 2012

@jordanbyron I guess this boils down to the problem of figuring out their status when they come back from Github. Let's leave it as-is for now, and see how it goes.

@jordanbyron

This comment has been minimized.

Copy link
Member

jordanbyron commented Sep 13, 2012

@sandal deployed 🤘

@jordanbyron jordanbyron merged commit 0d0d1f6 into master Sep 14, 2012

1 check passed

default The Travis build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment