Implementation of login/logout.

oss2019 / crispy-ai

An intelligent bot that listens and summarises lectures to deliver content that is crisp and to-the-point

MIT License

7 stars 14 forks source link

Implementation of login/logout. #18

Closed vedangj044 closed 5 years ago

vedangj044 commented 5 years ago

Closes #12 Closes #13 Closes #14 Closes #16 Closes #19 Read commit messages.

vedangj044 commented 5 years ago

Created a 'Stylish Header'

NeuralFlux commented 5 years ago

Thanks for the PR @vedangj044 . I've found some small bugs, other than that it's perfect. Also, the header looks great! So here are the bugs

General: There were a few template files in users/templates directly, instead of being inside users/templates/users. This may seem redundant but it helps in avoiding bugs later on.
13 requires localhost:8000/users/ to display the user home, whereas in the PR localhost:8000 itself displays the user home.
14 requires users.apps.UsersConfig to be added to the INSTALLED_APPS, whereas users is added.
16 requires a logout view too, that is, to display an acknowledgement that a user has been logged out which hasn't been implemented. You can make a template html for this.

NeuralFlux commented 5 years ago

This video on routing might help!

vedangj044 commented 5 years ago

We can have only base.html in users/templates directly rest everything in users/templates/users.
In this PR, i had made two home.html pages, one routed to /users/, other routed to / (empty url).
do you want me to redirect the localhost:8000 to localhost:8000/users/ and display the homepage ?

In am working on the rest.

NeuralFlux commented 5 years ago

There seems to be some problem with the routing, when I go to localhost:8000, there's an error saying ERR_TOO_MANY_REDIRECTS. I guess there's a routing loop, please look into this.

Also, you're right about the redirection. localhost:8000/ should redirect to localhost:8000/users/ if a session is logged in. Else it should redirect to localhost:8000/users/login where the user can login. Once logged in, redirect to localhost:8000/users/ where the User Home is rendered.

NeuralFlux commented 5 years ago

The redirection problem persists, is it only me or it happens in your PC too?

vedangj044 commented 5 years ago

I didn't face ERR_TOO_MANY_REDIRECTS up till now.

NeuralFlux commented 5 years ago

Some bugs have popped up in my PC I guess, but the problem is I can't merge before reviewing it. Hence I'll take some time to fix it, please don't mind.

svp19 commented 5 years ago

Woah, @vedangj044 you have done great work...but you should have opened up a separate PR for each issue separately. Also some GitHub courtesy rules, do not submit PRs to close issues you were not assigned too. Wait for @NeuralFlux to review and decide.

vedangj044 commented 5 years ago

adding permanent=False did solve the ERR_TOO_MANY_REDIRECTS issue, but i am not sure how it happened. Kindly, review these change. @svp19 Sure, I would keep this in mind.

NeuralFlux commented 5 years ago

Amazing work there @vedangj044 . I feel it's ready to be merged. Do keep in mind the things @svp19 said, as we wouldn't want too many conflicts due to major code being reworked on each forks.

oss2019 / crispy-ai

Implementation of login/logout. #18

13 requires `localhost:8000/users/` to display the user home, whereas in the PR `localhost:8000` itself displays the user home.

14 requires `users.apps.UsersConfig` to be added to the `INSTALLED_APPS`, whereas `users` is added.

16 requires a logout view too, that is, to display an acknowledgement that a user has been logged out which hasn't been implemented. You can make a template html for this.

oss2019 / crispy-ai

Implementation of login/logout. #18

13 requires localhost:8000/users/ to display the user home, whereas in the PR localhost:8000 itself displays the user home.

14 requires users.apps.UsersConfig to be added to the INSTALLED_APPS, whereas users is added.

16 requires a logout view too, that is, to display an acknowledgement that a user has been logged out which hasn't been implemented. You can make a template html for this.

13 requires `localhost:8000/users/` to display the user home, whereas in the PR `localhost:8000` itself displays the user home.

14 requires `users.apps.UsersConfig` to be added to the `INSTALLED_APPS`, whereas `users` is added.