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

Add ?teach parameter to see instructor version of homepage regardless of user role (#5812) #5983

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

Abishekcs
Copy link
Contributor

@Abishekcs Abishekcs commented Oct 6, 2024

closes #5812

What this PR does

This PR addresses issue #5812 by adding a ?teach parameter which allows homepage to use this parameter to render the instructor version of the homepage regardless of user role.

Changes:

  1. ?teach

    • The ?teach parameter now allows to see instructor version of the homepage regardless of user role.
  2. Completion of instructor orientation training modules:

  3. Dashboard Buttons:

    • Added margin styling for the "Create Course" button to improve visual layout consistency.
    • Adjusted the background position of the icon for the "Orientation Review" button to align better with the design.

Screenshots

Before:

Before.mp4

Screenshot from 2024-10-06 17-15-53

After:

After.mp4

Screenshot from 2024-10-06 17-15-21

…atus during onboarding

- Added `teach` parameter to the onboarding flow to allow users to override the absence of the instructor role, especially for those coming through enrollment links.
- Updated `onboard` method in `OnboardingController` to check for the `teach` parameter and grant instructor permissions when present.
- Modified view logic to adjust button layouts and added styles for course creation and orientation review buttons on the dashboard.
@ragesoss
Copy link
Member

ragesoss commented Oct 7, 2024

This is different than what I had in mind. Rather than using the ?teach parameter for enrollment or onboarding links, I'd like the homepage itself to use that parameter to render the instructor version regardless of user role. The idea would be that we could include links to https://dashboard.wikiedu.org/?teach that would let a user get access to the instructor view of the My Dashboard page (if logged in).

I think we could also optionally also automatically set the role from student to instructor, but the place to do that would be upon completion of the instructor orientation training module.

@Abishekcs Abishekcs marked this pull request as draft October 8, 2024 14:32
@Abishekcs Abishekcs changed the title Add ?teach parameter to allow instructor role override during onboarding (#5812) Add ?teach parameter to see instructor versions of homepage regardless of user role (#5812) Oct 8, 2024
@Abishekcs Abishekcs changed the title Add ?teach parameter to see instructor versions of homepage regardless of user role (#5812) Add ?teach parameter to see instructor version of homepage regardless of user role (#5812) Oct 8, 2024
@@ -31,6 +32,38 @@ def mark_exercise_complete

private

def instructor_training_complete?
Copy link
Member

Choose a reason for hiding this comment

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

This feature should not be in the same PR. I'd prefer to first merge the simplest possible version that will address the issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay

@@ -15,6 +15,7 @@ def index
set_admin_courses_if_admin
@pres = DashboardPresenter.new(current_courses, past_courses,
@submitted, @strictly_current, current_user)
@instructor_view = params.key?(:teach) and user_signed_in?
Copy link
Member

Choose a reason for hiding this comment

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

I think the better way to handle this is inside the DashboardPresenter, by passing the 'teach' parameter to the presenter.

Using a separate instance variable makes it more complicated to get all the correct behaviors (such as showing the link to instructor orientation but not the create course button, if the user has not already completed that training module). With the presenter approach, I think there will be no changes needed in the view templates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, I will make the required changes. Thank you for the review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add a parameter to override the lack of 'instructor' role to let a user create a course
2 participants