Navigate to index page after OK logo clicked in staff mode #1345
Conversation
Royserg
force-pushed
the
bug/Royserg/OK-logo-navigate-to-index-page
branch
from
ee59405
to
9f27c5b
Compare
Clicking OK logo as a staff was redirecting to assignment page of the first course user is assigned to as a staff. ticket okpy#1344
Royserg
force-pushed
the
bug/Royserg/OK-logo-navigate-to-index-page
branch
from
9f27c5b
to
699add2
Compare
| <a href="{{ url_for('admin.index') }}" class="navbar-brand"> | ||
| {% else %} | ||
| <a href="{{ url_for('student.index') }}" class="navbar-brand"> | ||
| {% endif %} |
There was a problem hiding this comment.
If the current user is an admin, isn't the url for student.index the same as the url for admin.index? In other words, isn't this entire code block the same as the following?
<a href="{{ url_for('student.index') }}" class="navbar-brand">
There was a problem hiding this comment.
I don't think so. Those are two entirely different routes.
There was a problem hiding this comment.
I'm running a local server and when I impersonate a random non-admin user, I can view both http://localhost:5000/ and http://localhost:5000/admin/course/. But when I'm impersonating an admin, http://localhost:5000/ (aka student.index) seems to redirect to http://localhost:5000/admin/course/ (admin.index) automatically.
There was a problem hiding this comment.
Is that true even if you enroll the admin as a student in a course?
There was a problem hiding this comment.
@pbitutsky @colinschoen
I think that replacing that if else block with link to student.index only won't do the trick
<a href="{{ url_for('student.index') }}" class="navbar-brand">
If only link to student.index would stay, then an user who is an admin(has is_admin true) by clicking on OK logo will be redirected to admin dashboard of a first course they have on the list. Even on the all courses view clicking logo would redirect to that one particular course and it feels weird. Little demo below. Logged in as: chelseacurry9@berkeley.edu
Leaving that if else block introduced in the commit redirects in this case admins always to their admin view list of all courses (same as clicking courses > View All Courses) and rest of the users behave normal as follow:
In case of those users who are also a staff for a course, they will get to the student.index but staff edition (staff dashboard). Logged in as: ctf3@gmail.com
And finally normal student (not a staff for any course) will always get redirected to the student.index and is not able to access admin dashboard
Logged in as: gzg@gmail.com
Please, let me know if I understood it correctly and if any more changes are neccessary :)
There was a problem hiding this comment.
In that case, this PR LGTM.
|
The issue @pbitutsky filed suggests one of the places the header can link to is the student index. I don't think this is super useful. I think we should just link to the list of courses (in the staff view) for everyone (not just admins). |
|
@colinschoen If I'm understanding you correctly, you would like for the OK logo to navigate to http://localhost:5000/admin/course/. However, there is already navigation to get there: in the navbar, under courses > View All Courses. However, there is no way to navigate to the student index (http://localhost:5000/) via the UI. So won't it be better if the logo were to navigate to the student index? |
|
@pbitutsky Ah, ok that does sound ok too. Yeah navigating to the student index makes sense. |
Instead of getting assignments page, navigate to the student mode index page.
My first contribution to OS, not sure if I did it right 🤞🤞
fix #1344