WikiEducationFoundation / WikiEduDashboard

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

Revisit specs for `CoursesUsers` in `spec/models/courses_users_spec.rb` #5865

Open gabina opened 4 days ago

gabina commented 4 days ago

What is happening?

It's not clear (for me) what the "updates only updates cache from tracked revisions" spec in spec/models/courses_users_spec.rb is trying to test.

First of all, the spec tries to get CoursesUsers record with id = 1 doing described_class.update_all_caches(described_class.where(id: 1)). I wonder if we can be sure that the id of the recently created CoursesUsers record is 1 (at least, locally) becuase we're not sepcifying the course user id when creating it.

Supposing that we force the CoursesUsers id to be 1 during creation, the other point is that we're doing expectations over course_user (where course_user = courses_user). According to what I see when printing it, it looks like course_user is not being updated as expected (even though the print happens after the update_all_caches call). However, it gets updated if we reload it.

puts course_user.inspect
#<CoursesUsers id: 1, created_at: "2024-06-28 16:59:24.000000000 +0000", updated_at: "2024-06-28 16:59:24.000000000 +0000", course_id: 290, user_id: 267, character_sum_ms: 0, character_sum_us: 0, revision_count: 0, assigned_article_title: "Selfie", role: 0, recent_revisions: 0, character_sum_draft: 0, real_name: "John Smith", role_description: nil, total_uploads: nil, references_count: 0>
puts course_user.reload.inspect
#<CoursesUsers id: 1, created_at: "2024-06-28 16:59:24.000000000 +0000", updated_at: "2024-06-28 16:59:24.000000000 +0000", course_id: 290, user_id: 267, character_sum_ms: 0, character_sum_us: 600, revision_count: 3, assigned_article_title: "Selfie", role: 0, recent_revisions: 0, character_sum_draft: 700, real_name: "John Smith", role_description: nil, total_uploads: 0, references_count: 0>

The point is that if we're only un-tracking the ArticlesCourses record for article (ArticlesCourses.first.update(tracked: false)), why would we expect(course_user.revision_count).to eq(0)? I think revisions for talk_page, sandbox and draft should be counted and we should expect(course_user.revision_count).to eq(3).

Additional context

As part of the data re-architecture project, I'm creating specs for new timeslices models based on the existing specs for non-timeslices models. For example, to create CourseUserWikiTimeslices specs I start by looking at the CoursesUsers specs. That's why I ended up taking a deep look at these specs.

ragesoss commented 1 day ago

That spec was added as part of the project to add support for un-tracking specific articles. The main use case is if you have an event like an edit-a-thon where everyone is working on a specific set of articles related to the theme of the edit-a-thon, but there's a user who also edited a few unrelated articles, you can mark those edited articles as 'untracked' and then they will not be counted in the stats.

It looks like I did a poor job updating the spec when I was trying to make the revision_count behave as expected, in https://github.com/WikiEducationFoundation/WikiEduDashboard/commit/e76ce7d51bf829f384efdd1a8650d1396631c5cf

I think you are correct, we should expect revision_count not to be zero here. The lack of a reload after the cache update action, as you pointed out, is probably the source of the problem.