search

grapheo12 / iqps

Web-app meant for qp.metakgp.org
MIT License
20 stars 21 forks source link

Login utility #37

Closed shobhit10058 closed 3 years ago

shobhit10058 commented 3 years ago

concerns issue #32. some initial changes for including the register user options. Changes -

  1. A non-authenticated user can only see the search options and the login page which can include the link to the register page.
  2. The register page has the nav bar included with it for easy navigation.
  3. An authenticated user can see all the initial tabs like request paper, upload paper , search, and logout.
  4. The home button was included so that user can easily navigate to search options after clicking other tabs
  5. If a non-authenticated user tries to user report, upload or request options by changing the URL he/she is redirected to home
grapheo12 commented 3 years ago

@shobhit10058 Looks good! Just create a variable in app.env called LOGIN_REQUIRED=True, and make your templates so that if this variable is True then only your changes show up, otherwise the old website remains.

grapheo12 commented 3 years ago

And yes! The Home button is unnecessary. The MFQP logo on the left redirects to the homepage.

shobhit10058 commented 3 years ago

oh I did not knew about mfqp button.

shobhit10058 commented 3 years ago

after login the old website is only there just a home button is extra I will remove it. Do I need to add that variable?

shobhit10058 commented 3 years ago

after login image

shobhit10058 commented 3 years ago

oh, I think you want that the admin has privilege then that for him login will be required or not by default for accessing those options

grapheo12 commented 3 years ago

No, I meant, this login feature should be a configurable option. The system admin might want to switch on/off as per requirements. Remember, we are targetting users from different colleges, not just ours. So it will be good to have options.

grapheo12 commented 3 years ago

Also, we had had quite a debate on this issue in MetaKGP. It's best to keep both.

pep8speaks commented 3 years ago

Hello @shobhit10058! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers:

Comment last updated at 2020-12-12 18:37:18 UTC
shobhit10058 commented 3 years ago

should I remove the app.env file here, I kept it as there was a change there so to push I had to change the .gitignore also

grapheo12 commented 3 years ago

Yes remove app.env

shobhit10058 commented 3 years ago

done

shobhit10058 commented 3 years ago

@grapheo12 is there some error?

shobhit10058 commented 3 years ago

is it not used? I have used them that is in the templates but I don't know why it is not working? Can you look once in the code and check the mistake