WikiEducationFoundation / WikiEduDashboard

Wiki Education Foundation's Wikipedia course dashboard system
https://dashboard.wikiedu.org
MIT License
392 stars 630 forks source link

Create an Alert when an active course Timeline gets removed #4859

Closed ragesoss closed 1 year ago

ragesoss commented 2 years ago

Current Behavior:

On dashboard.wikiedu.org, a course must have a Timeline before it gets submitted, and Wiki Education staff will only approve courses with detailed timelines that include the standard set of training modules (among other things that we check for).

However, after course approval, an instructor can still modify their Timeline. In rare cases, they may remove the standard Timeline content entirely. We don't have a way of detecting cases like this automatically, but it's important to know about them quickly so that we can contact the instructor.

Desired Behavior:

When an instructor blanks the Timeline or updates to include no training modules, the Dashboard should generate an Alert and email the staff (ie, Alert#email_course_admins).

This should only apply for wiki_education mode, and only for the ClassroomProgramCourse type.

shekinahmanyi commented 2 years ago

Hi @ragesoss please i will like to work on this issue

ragesoss commented 2 years ago

@shekinahmanyi hi! are you working on this?

shekinahmanyi commented 2 years ago

@shekinahmanyi hi! are you working on this?

Yes @ragesoss

ragesoss commented 2 years ago

Great! Let me know if you have any questions or get stuck with anything.

shekinahmanyi commented 2 years ago

Great! Let me know if you have any questions or get stuck with anything.

Ok thanks! I'll do so

marklocklear commented 2 years ago

@ragesoss How can we tell if a course is approved? Is the approved? method in course.rb sufficient?

ragesoss commented 2 years ago

@marklocklear yes, for an individual Course, #approved? is the way to tell. (We don't have scope for getting the collection of all approved courses, but the same logic could be turned into a scope if needed.)

marklocklear commented 2 years ago

@ragesoss here is a first stab at this https://github.com/marklocklear/WikiEduDashboard/tree/timeline_removal_alert. Let me know if I'm headed down the right path. Also probably need more logic in the deleted_timeline? method in deleted_timeline_alert_manager.rb.

Also unclear about what email template gets called from alert_mailer.rb; like what is @alert.message?

ragesoss commented 2 years ago

Thanks @marklocklear. I think following the pattern of the other alert managers is probably not the right way to do this, because the situation we're trying to detect may look like this:

In a case like that, by the time the alert manager runs to find courses that have no timeline, it will already have new (minimal, unhelpful) timeline content, so no alert would be created.

My suggestion would be to create a service object that acts on a single course, determining whether it has a timeline (and meets the other conditions) and then creates an alert if appropriate... and then use this as part of the handling for any actions that remove timeline content (ie, WeeksController#destroy, BlocksController#destroy, and CoursesController#delete_all_weeks).

The default email template for AlertMailer is app/views/alert_mailer/alert.html.haml. This is inferred via Rails magic from the method name in AlertMailer where mail is called. @alert.message is an instance method for the particular Alert record; message is one of the ActiveRecord database columns for that table, so the message will be whatever was put there when the record was created. In most or all of the alert types from the alert managers, we don't use this column, so the value will be nil. We use it for alerts that are generated by users requesting help, via AlertsController, to hold whatever message the user includes in their help request.

marklocklear commented 2 years ago

@ragesoss another stab at this here https://github.com/WikiEducationFoundation/WikiEduDashboard/compare/master...marklocklear:delete_timeline_alert?expand=1. Any suggestions or docs on configuring mail so I can send locally with my own mailgun credentials? I tried adding my key to config/env/dev.rb and no cigar.

sylvia-boyani commented 2 years ago

Hello @ragesoss, Is it okay if i start on this issue?

ragesoss commented 2 years ago

@sylvia-boyani yes, if you're not already working on another issue, feel free to work on this one.

sylvia-boyani commented 1 year ago

Looking forward to understand the codebase more from kunalthedev's contribution.