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

activate_user.php #98

Closed
ltbarclay opened this issue Aug 23, 2014 · 5 comments
Closed

activate_user.php #98

ltbarclay opened this issue Aug 23, 2014 · 5 comments
Assignees
Labels
confirmed bug Something isn't working

Comments

@ltbarclay
Copy link
Contributor

When a user clicks the activation link in the email, it takes them to activate_user.php with a token attached.

However, it looks like the 'else if' on line 63 just echoes a json string if an error occurs (which then gets displayed in the browser).

If setUserActive() is successful, it look like the 'if' on line 82 always returns false and line 85 redirects the user to getReferralPage() (which should only happen if ajaxMode is false - but making it look like everything is working fine).

So, if you took out line 85, the page would just display a json string or nothing, and not redirect the user anywhere.

Am I seeing this right?

@alexweissman
Copy link
Member

So the concept here is that API pages should be accessible in two different ways - via an AJAX request, or via a plain ordinary page request. The idea is that if you wanted to make your site accessible to non-JS users (or users using an email link, like in the case of activate_user), it would still work.

That being said, the implementation is admittedly far from perfect. Something that needs to be done is to revise the API pages to use the functions I created in models/error_functions.php. This provides a uniform way of checking for AJAX requests, and processing errors via apiReturnError. I've already done this in api/user_resend_activation.php, but it needs to be done in the rest of the API pages as well. Any chance you'd want to work on that? We should probably also implement an apiReturnSuccess function to process successful requests.

I'm also open to modifying/completely changing the way we do error handling. So far the "error stream" approach is the best I've come up with, but maybe there is a way to improve it.

@ltbarclay
Copy link
Contributor Author

Okay, but it still looks like if an error occurs, the script immediately exits and doesn't redirect to any page.

header("Location: " . getReferralPage()) only gets called if no errors occur.

@alexweissman
Copy link
Member

Yeah this is why I said we should write a apiReturnSuccess function that does the same thing as apiReturnError, except returns a success code instead of an error code. Would you want to make this change for me?

@ltbarclay
Copy link
Contributor Author

I'd be happy to look into it. I'm still getting familiar with the inner workings and wouldn't want to screw anything up.

@lilfade
Copy link
Contributor

lilfade commented Aug 24, 2014

That's the awesomeness of github if you mess it up you can always pull a new copy haha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants