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

Implement minimal routes #11

Merged
merged 26 commits into from
Mar 25, 2020
Merged

Implement minimal routes #11

merged 26 commits into from
Mar 25, 2020

Conversation

jankapunkt
Copy link
Member

@jankapunkt jankapunkt commented Mar 21, 2020

This PR implements #1 by creating all required minimal routes

Status:

  • Routes defined
  • Pages added
  • Not found redirect implemented
  • Not logged-in redirect to login implemented
  • logged-in redirect to myClasses implemented
  • Login implemented
  • Logout implemented
  • Layout implemented

@jankapunkt jankapunkt added the work in progress A pull request that is not yet complete, so no attention is required yet label Mar 21, 2020
@jankapunkt jankapunkt self-assigned this Mar 21, 2020
@jankapunkt jankapunkt added this to In progress in First Prototype via automation Mar 21, 2020
@jankapunkt jankapunkt added this to the Pre implementation milestone Mar 21, 2020
@jankapunkt
Copy link
Member Author

@tushar13293 I added you as a reviewer for this PR. This is basically the foundation for you to work with this app.

Please checkout this branch via git checkout minimal-routes && git pull and test the app for the following things:

  • can you setup the newly required accounts app just from reading the updated README
  • can you understand the basic application structure / do you know where is what, where to start if you would want to implement a feature (please go through the code carefully, take your time)
  • can you login, logout
  • can you find and execute the local scripts for linting and testing

If there is any issue in the code you don't understand, please use the review to add comments.

Copy link
Contributor

@tushar13293 tushar13293 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One issue i found was when i log in for the first time server asks for username/password (as it should) but after logging out it doesn't ask for login details again and logs me in automatically.

I think this is intended?
error

Everything else works great.

First Prototype automation moved this from In progress to Review in progress Mar 25, 2020
@jankapunkt
Copy link
Member Author

One issue i found was when i log in for the first time server asks for username/password (as it should) but after logging out it doesn't ask for login details again and logs me in automatically.

I think this is intended?
error

Everything else works great.

I see, this is because the OAuth workflow logs you in across apps, so if you log out from the teacher app you are not finally logged out. But for teachers we should implement a logout call to the OAuth server so they are 100% logged out. This however requires this first to be solved in the accounts application.

Since you found nothing else I will merge this now and create the respective issues. Thank you 👍

@jankapunkt jankapunkt merged commit 4a568ff into develop Mar 25, 2020
First Prototype automation moved this from Review in progress to Done Mar 25, 2020
@jankapunkt jankapunkt mentioned this pull request Apr 7, 2020
7 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
work in progress A pull request that is not yet complete, so no attention is required yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants