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 a new parameter for On-wiki course pages to show all instructors. #5900

Merged

Conversation

Abishekcs
Copy link
Contributor

@Abishekcs Abishekcs commented Jul 17, 2024

closes #1175

What this PR does

Add Support for any number of additional instructors for on-wiki course page. With new parameters, so that the templates will remain backwards-compatible on-wiki.

Screenshots

Before:

Before.mp4

After:

2024-08-05.18-20-33.mp4

@@ -46,7 +48,7 @@ def course_details_and_description
end

def course_details
# TODO: add support for multiple instructors, multiple content experts.
# TODO: add support for multiple content experts.
Copy link
Member

Choose a reason for hiding this comment

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

Can you include an example in a comment of what the output of this template looks like with multiple instructors?

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

@@ -62,6 +64,8 @@ def course_details
| assignment_page = #{@course.wiki_title}
| slug = #{@course.slug}
| campaigns = #{@course.campaigns.pluck(:title).join(', ')}
| all_instructor_usernames = #{all_instructors_username}
| all_instructor_realnames = #{all_instructors_realname}
| #{@dashboard_url} = yes
Copy link
Member

Choose a reason for hiding this comment

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

Currently, the template looks something like this:

{{course details
 | course_name = Foundations II
 | instructor_username = Health policy
 | instructor_realname = Dorie Apollonio
 | support_staff = Ian (Wiki Ed)
 | subject = Pharmacy
 | start_date = 2024-06-01 00:00:00 UTC
 | end_date = 2024-08-17 23:59:59 UTC
 | institution = UCSF
 | expected_students = 125
 | assignment_page = Wikipedia:Wiki_Ed/UCSF/Foundations_II_(Summer_2024)
 | slug = UCSF/Foundations_II_(Summer_2024)
 | campaigns = Communicating Science, Students in the Health Professions, Summer 2024
 | dashboard.wikiedu.org = yes
}}

I think in order for it to work smoothly with additional instructors, instead of combining them all into one parameter, it should include a different parameter for each username after the first. For example, instructor_username_2 and so on, for the additional instructors after the first. That way, the template can be extended to handle them similarly with wikilinks to their user pages.

Also, let's not pass along the real name of additional instructors. We may at some point remove the real name of the first instructor, as the community has come to value privacy a little more highly relative to editor transparency in recent years, and we may be removing the real name of the first instructor as well at some point.

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 necessary changes.

…rivacy

- Added logic to include separate parameters (instructor_username_2, etc.) for each additional instructor.
- Modified the method to conditionally insert additional instructor usernames only if more than one instructor is present.
- Removed passing of real names for additional instructors to enhance privacy.
- Updated the course_details method to improve readability and maintainability.
@Abishekcs
Copy link
Contributor Author

Hi @ragesoss,
I have made the required changes. Could you please review them whenever you have time? Thank you!

@Abishekcs Abishekcs requested a review from ragesoss August 5, 2024 13:06
@ragesoss
Copy link
Member

The newlines are buggy, with a missing newline before the first additional username, and an extra newline after the last one:

{{program details
 | course_name = ddddd
 | instructor_username = Sage (Wiki Ed)
 | instructor_realname = Sage Rossss | instructor_username_2 = Ragesoss
 | instructor_username_3 = Ragesock

 | support_staff = Ragesock
 | subject = Biographies
 | start_date = 2024-07-31 00:00:00 UTC
 | end_date = 2024-08-20 23:59:59 UTC
 | institution = dasdfasdfasdfa
 | expected_students = 7
 | assignment_page = 
 | slug = dasdfasdfasdfa/ddddd_(d)
 | campaigns = 
 | outreachdashboard.wmflabs.org = yes
}}

I think it would be good to make the specs a bit more thorough (for example, by matching for the entire expected output of the program details template that includes the username parameters).

@Abishekcs
Copy link
Contributor Author

The newlines are buggy, with a missing newline before the first additional username, and an extra newline after the last one:

{{program details
 | course_name = ddddd
 | instructor_username = Sage (Wiki Ed)
 | instructor_realname = Sage Rossss | instructor_username_2 = Ragesoss
 | instructor_username_3 = Ragesock

 | support_staff = Ragesock
 | subject = Biographies
 | start_date = 2024-07-31 00:00:00 UTC
 | end_date = 2024-08-20 23:59:59 UTC
 | institution = dasdfasdfasdfa
 | expected_students = 7
 | assignment_page = 
 | slug = dasdfasdfasdfa/ddddd_(d)
 | campaigns = 
 | outreachdashboard.wmflabs.org = yes
}}

I think it would be good to make the specs a bit more thorough (for example, by matching for the entire expected output of the program details template that includes the username parameters).

Okay

@Abishekcs
Copy link
Contributor Author

@ragesoss , I have made the required changes. Could you please review the code again? Thank you!

Screenshot from 2024-08-13 18-49-00

2024-08-13.18-47-28.mp4

@ragesoss ragesoss merged commit 3d84228 into WikiEducationFoundation:master Aug 14, 2024
1 check failed
@ragesoss
Copy link
Member

Nice work, thanks @Abishekcs !

@Abishekcs Abishekcs deleted the on-wiki-course-page branch August 14, 2024 16:59
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.

On-wiki course pages should show all instructors, not just first
2 participants