Add service to update namespace stats. #5075
Conversation
|
|
||
| # live, tracked articles filtered by wiki and namespace | ||
| def articles_filtered_by_wiki_namespace | ||
| @course.articles_courses.joins(:article).where(articles: { wiki: @wiki, namespace: @namespace }) |
There was a problem hiding this comment.
This doesn't include generating ArticlesCourses for non-mainspace articles, correct? So this will always be zero for this PR?
There was a problem hiding this comment.
Oh right. I missed it. Will correct it.
There was a problem hiding this comment.
@ragesoss, right now tracked_namespaces are not known, so should I enable article_courses for all namespaces?
There was a problem hiding this comment.
no, i don't think so. I think it would be better to include a minimal version of tracked_namespaces in this PR such that it could be deployed without changing current behavior, but would still allow for testing.
One approach would be to define tracked_namespaces for course.rb but just have it return [0] (ie, hard-code the current behavior of mainspace being the only tracked namespace for any course), and then in the test you can stub that method to override it and add cookbook. This would be the simplest way i could think of for this PR, although it would mean a small amount of rewriting for the next stage.
Another approach would be include the new database table in this PR, so that tracked_namespaces could be determined from the database (which will be necessary for the later PRs anyway).
There was a problem hiding this comment.
But, are you going to deploy it right away? I mean before we have tracked_namespaces from CourseWikiNamespaces ?
There was a problem hiding this comment.
Maybe! Part of the point of the separate PRs is to make it possible to deploy before everything else is ready.
|
What additional changes would be necessary to ensure that the correct stats are being generated (via an rspec test)? I guess generating ArticlesCourses for non-mainspace articles is the main thing? |
|
Now with the |
app/models/course.rb
Outdated
| @@ -282,6 +282,10 @@ def tracked_revisions | |||
| .where(wiki_id: wiki_ids) | |||
| end | |||
|
|
|||
| def tracked_namespaces | |||
| return ['0'] | |||
There was a problem hiding this comment.
Should this be an integer instead of a string?
There was a problem hiding this comment.
WikiEduDashboard/app/models/course_data/articles_courses.rb
Lines 131 to 133 in dbf2c87
Yeah, but in articles_courses it was a string, so I kept it a string.
There was a problem hiding this comment.
Ah. I don't think there's any reason for it to be a string, as namespace is an integer in the database.
|
|
||
| require 'rails_helper' | ||
|
|
||
| describe UpdateWikiNamespaceStats do |
| expect(stats).to have_key(:user_count) | ||
| expect(stats).to have_key(:word_count) | ||
| expect(stats).to have_key(:reference_count) | ||
| expect(stats).to have_key(:view_count) |
There was a problem hiding this comment.
What values do these have? Does this test exercise the generation of non-zero values for all of these stats? (I'm guessing no for the view count, and yes for the most of the rest?)
There was a problem hiding this comment.
Actually, I haven't checked for the values.
So, should I get the count for each attribute and write a test for it?
There was a problem hiding this comment.
Also, I wasn't sure if I should include the test where it checks for each attribute in key. Should I keep it?
There was a problem hiding this comment.
Yes. We might find that such values can vary a little bit over time (if any of the edits get deleted at some point) but for now, I think it's better to have a little extra test focus on verifying that these stats are actually being counted the way we think they should be.
There was a problem hiding this comment.
Also, I wasn't sure if I should include the test where it checks for each attribute in key. Should I keep it?
Yes, I think that's fine. It makes it a little more explicit what shape we expect this data to take.
There was a problem hiding this comment.
@ragesoss, have updated the test.
What this PR does
Defines a service to generate/update namespace stats to CourseStats.
Steps to reproduce for cookbook namespace
UpdateCourseStats.new(course)UpdateWikiNamespaceStats.new(course, wiki, 102)