dhasegan / connect.academy

Educational platform
www.connect.academy
MIT License
2 stars 0 forks source link

Not activated users cannot change anything #115

Open dhasegan opened 10 years ago

dhasegan commented 10 years ago

but they dont know why since there is not info about this

dhasegan commented 9 years ago

We have to make a easy way of handling errors on the page. I will post my ideas somewhere

tpllaha commented 9 years ago

General errors (that are not associated with a particular form field) could be handled by filling the error or warning box that we have on top of base.html. This should be easy if we have a convention on the format of ajax data.

On Mon, Dec 1, 2014 at 7:47 AM, Daniel Hasegan notifications@github.com wrote:

We have to make a easy way of handling errors on the page. I will post my ideas somewhere

— Reply to this email directly or view it on GitHub https://github.com/dhasegan/connect.academy/issues/115#issuecomment-65026484 .

dhasegan commented 9 years ago

ok, an idea for general way of handling errors like this. when we have a POST request we should NEVER render html. Instead we redirect to: connect.academy/the/relevant/page?error=coded_string and we make a general filter to handle things like this: coded_string1 => bad request error coded_string2 => you fucked up ...

I will write this is some place else for better visibility. We need a board where we post things. Aah I know, we should make a mailing list :)

dhasegan commented 9 years ago

create a decorator that checks if a user is active (email is activated) and if not, do not let them do anything and reroute them back to the page where they came from. Also set a message with messages.error

tpllaha commented 9 years ago

we have that decorator. it's called require_active_user. And we're using it, but instead of redirecting back to the page they came from we're raising Http404. Just pointing it out, so that whoever picks this up knows that they should edit the existing decorator instead of writing a new one.

On Sun, Dec 7, 2014 at 9:45 PM, Daniel Hasegan notifications@github.com wrote:

create a decorator that checks if a user is active (email is activated) and if not, do not let them do anything and reroute them back to the page where they came from. Also set a message with messages.error

— Reply to this email directly or view it on GitHub https://github.com/dhasegan/connect.academy/issues/115#issuecomment-65954147 .

uneeb123 commented 9 years ago

I am slightly confused. There is no 404 error. And the decoraters seem fine. What exactly do you mean by "not activated users can't do anything"?

tpllaha commented 9 years ago

Well the idea is that users who are not active SHOULD NOT be allowed to POST. And they are not allowed indeed. But instead of giving them a nice error message, I think we return 404 when a non-active user tries to POST. If this is not true, then someone has fixed this issue.

On Sun, Jan 18, 2015 at 8:26 PM, Uneeb Agha notifications@github.com wrote:

I am slightly confused. There is no 404 error. And the decoraters seem fine. What exactly do you mean by "not activated users can't do anything"?

— Reply to this email directly or view it on GitHub https://github.com/dhasegan/connect.academy/issues/115#issuecomment-70421651 .

uneeb123 commented 9 years ago

Ah, alright. Got it.

uneeb123 commented 9 years ago

I was going to Google stuff, but I think it will be better if I just ask you guys about it.

I am not sure how SteamingHttpResponse functions in the following method: https://github.com/dhasegan/connect.academy/blob/master/app/course/views.py#L118

Does it render only part of the html? The decorator @require_active_user is supposed to display a message if the non-active user posts. I made some changes, but the message is not being displayed in this case. I was wondering whether it is caused because of the StreamingHttpResponse not rendering the full page?

uneeb123 commented 9 years ago

By the way @dhasegan, I just realized while looking at commits that exactly one year ago we were sitting in your room bored out of our minds discussing various bullshit with Catalin, when we suddenly gave birth to jCourse which later transformed into this, lol.

tpllaha commented 9 years ago

StreamingHttpResponse should be used for ajax requests. It's just like a HttpResponse (which is what render() returns), but it doesn't have to contain a html page. It can be anything like a JSON string or whatever. So it's meant to be used for services rather than for pages. Otherwise it does the same thing. You can replace it with a HttpRepsonse, but then django will warn you that using HttpRepsonse in such cases is deprecated. Hope it answers your question.

On Tue, Jan 27, 2015 at 10:26 AM, Uneeb Agha notifications@github.com wrote:

By the way @dhasegan https://github.com/dhasegan, I just realized while looking at commits that exactly one year ago we were sitting in your room bored out of our minds discussing various bullshit with Catalin, when we suddenly gave birth to jCourse which later transformed into this, lol.

— Reply to this email directly or view it on GitHub https://github.com/dhasegan/connect.academy/issues/115#issuecomment-71615078 .

uneeb123 commented 9 years ago

Yeah, thanks. It does explain what StreamingHttpResponse does. I am still confused why the error message is not being displayed. Anyway, I'll investigate a bit more.