Remove course template from user page and user talk page when user disenrolls

[gabina]

What this PR does

The purpose of this PR is to add a way to remove course templates from user page and user talk page when user gets dis-enrolled from a specific course.

When a user enrolls in a course, a template pointing to the course is added to the user page and the user talk page (example user page with participating template: https://en.wikipedia.org/wiki/User:Jfitz19)

This PR adds a new DisenrollFromCourseWorker worker to remove course templates from user page and user talk page. It basically "reverts" what the EnrollInCourseWorker does (except for the user's sandbox action).

Now, when a user gets removed from a course, the EnrollmentController#remove schedules a job for the DisenrollFromCourseWorker to remove the participating templates from user page and user talk page.

Closes #2842

Screenshots

Open questions and concerns

[gabina]

Created this because got a "Method has too many lines" offense from rubocop

[ragesoss]

I think a better way to reorganize this code would be to extract some of the setup and guard code at the start of the method. Ideally, the code in a method is at roughly the same level of abstraction, and the most detailed bits are the first ones to extract.

Extracting these two lines does not really make the method any easier to understand, which is the purpose behind that rubocop rule.

[gabina]

Makes sense, I see your point. I tried to do that on 9939233

[gabina]

Sanity checks:

1. Should I do something with the template in sandbox to revert the add_template_to_sandbox action?
2. Should I take enrollment_edits_enabled into account for disenrolling? I mean, validate the disenroll_from_course action, similar to how it's done for enroll_in_course, by checking the enrollment_edits_enabled flag in the validate method.

  def validate(action)
    yield unless course_edits_allowed?
    # action-specific checks
    case action
    when :update_course
      yield unless @course.wiki_course_page_enabled?
    when :update_assignments, :remove_assignment
      yield unless @course.assignment_edits_enabled?
    when :enroll_in_course
      yield unless @course.enrollment_edits_enabled?
    end
  end

[ragesoss]

I don't think that's necessary, as that's not a place other editors will look to understand the context for the un-enrolled student's edits.

[gabina]

I think I edited the message adding a second question after you answered, so just checking you saw both questions.

[ragesoss]

Ah, I missed the second question. I think yes, checking enrollment_edits_enabled? for this action makes sense.

[gabina]

Added the check and spec for that on 00cf010

[gabina]

Note: I was able to enable wiki edits on my local environment, and I reproduced this by adding and then removing my own user to a course. The history of how my user page and user talk page was modified can be check on https://en.wikipedia.org/w/index.php?title=User:Gabinaluz&action=history.

I didn't feel very confident reproducing that for fear of unintentional changes, so I didn't try more than that.

[gabina]

CI failed with the following errors:

Finished in 31 minutes 9 seconds (files took 12.19 seconds to load)
1825 examples, 4 failures, 17 pending

Failed examples:

rspec ./spec/features/article_finder_spec.rb:17 # article finder performs searches and returns results
rspec ./spec/lib/wiki_pageviews_spec.rb:72 # WikiPageviews.views_for_article for an unviewed article returns an empty hash
rspec ./spec/lib/wiki_pageviews_spec.rb:151 # WikiPageviews.average_views_for_article for an article that does not exist returns 0
rspec ./spec/lib/wiki_pageviews_spec.rb:164 # WikiPageviews.average_views_for_article for an article that exist but has no view data returns 0

Will check if something is related to my changes.

Edit: the error for the wiki pageviews seems to be

WikiPageviews::PageviewApiError:
       {"detail":"The date(s) you used are valid, but we either do not have data for those date(s), or the project you asked for is not loaded yet. Please check documentation for more information.","method":"get","status":404,"title":"Not Found","type":"about:blank","uri":"/metrics/pageviews/per-article/fr.wikisource/all-access/user/Voyages,_aventures_et_combats%2FChapitre_18/daily/2023082500/2023101300"}

It doesn't look like it's related to my changes.

[ragesoss]

Note: I was able to enable wiki edits on my local environment, and I reproduced this by adding and then removing my own user to a course. The history of how my user page and user talk page was modified can be check on https://en.wikipedia.org/w/index.php?title=User:Gabinaluz&action=history.

I didn't feel very confident reproducing that for fear of unintentional changes, so I didn't try more than that.

It looks like the edit summaries were buggy: User has disenrolled in [[]]. Do you know why that is?

[gabina]

It looks like the edit summaries were buggy: User has disenrolled in [[]]. Do you know why that is?

Oh good point.
The disenrollment_summary uses the course.wiki_title to build the summary, which needs the wiki_course_page_enabled? to return true to be other than nil. Due to the fact that on my local environment the submitted field for the course example was set to 0, course.wiki_title ended up being nil. I was playing around modifying the settings on my local environment and could get something like:

• User has enrolled in Project:Outreach_Dashboard/Stanford_Law_School/Advanced_Legal_Research_Winter_2020_(Winter).
• User has disenrolled in Project:Outreach_Dashboard/Stanford_Law_School/Advanced_Legal_Research_Winter_2020_(Winter).

You can check that on https://en.wikipedia.org/w/index.php?title=User:Gabinaluz&action=history
BTW, the page for the course doesn't exist (it was automatically created by my action, but I removed it).

On the other hand, while taking a look at your point, I noticed a small bug in the disenrollment action, that wasn't caught by the tests. If you compare the last two versions, you can notice that an extra new line is added when removing the course template, which is wrong. I modified the tests to catch the error, and then fixed it using a regular expression on 48d7a55

[gabina]

I added an optional new line in the regexp, but maybe we can use just something like #{Regexp.quote(content)}\n, as the new line should be always added when writing the course template

[ragesoss]

I guess the newline should be removed if present, but if there is not a newline, it should still remove the template.

[gabina]

Makes sense. Then it looks like using /#{Regexp.quote(content)}(\n)?/ (as it's now) is the right way. If the new line after the content exists, it will match the content and the new line. If the new line doesn't exist, it will match only the content.

Let me know if there is something for me to review. Otherwise, I understand I already addressed all the suggestions/questions.

[sentry-io]

Suspect Issues

This pull request was deployed and Sentry observed the following issues:

• ‼️ **NoMethodError: undefined method home_wiki' for nil:NilClass** Users::EnrollmentController#index` View Issue

_{Did you find this useful? React with a 👍 or 👎}

[gabina]

I sent a request to join to wiki-education organization for Sentry because I cannot see the issue in Sentry

[ragesoss]

@gabina that error is definitely not related to your change; I haven't deployed it to Programs & Events Dashboard yet.