-
Notifications
You must be signed in to change notification settings - Fork 658
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
Query Fix: remove dependency on depreciated revision table #5998
Query Fix: remove dependency on depreciated revision table #5998
Conversation
Please include before/after screenshots to demonstrate the UI changes associated with this PR. |
@ragesoss , Okay I'd update this PR with the UI changes |
Hello @ragesoss, I'd like you to please revisit this PR as I've included the before/after screenshots demonstrating the UI changes. |
article_edits[:earliest_revision] = edit.date if earliest_rev_yet?(edit, article_edits) | ||
article_edits[:average_views] ||= edit.article.average_views | ||
@articles_edited[edit.article_id] = article_edits | ||
course.articles_courses.where(new_article: true).each do |edit| |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might not be a viable strategy. An ArticlesCourses record represents changes made to an article by (potentially) all participants in a course, not just this user.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the clarifications. I'd remove the condition: .where(new_article: true)
, for all participants to be accounted for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ragesoss I've update the code implementation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still will count contributions from other students in the same course, not just the contributions from the student. I don't think there's a good way to use the ArticlesCourses data to get individual statistics.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I understand now. I'd updated the function to account for only the user contribution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ragesoss, updated! now it accounts for article contributions by the user.
Please test your changes locally before pushing them. |
Some test scripts associated with the user profile pages are resulting in check failures. I would look into the tests also. |
@ragesoss, I've tested locally, identified bugs, and fixed them. However, a test appears to fail since the function relies on data from the revision table. Kindly review. the failing test:Image of the dashboard now(statistics): |
In that case, you'll need to adjust the test appropriately. |
Okay, I'd do that. |
Please reopen if you return to this. |
What this PR Does
Notable Changes
articles_edited
toarticles_created
.individual_mainspace_edits
query (which used therevisions
table) with a new approach that uses thearticles_courses
table.Expected Data Output
Before
(Prior Data Fields):
course_string_prefix
individual_article_count
individual_article_views
individual_articles_created
individual_courses_count
individual_references_count
individual_upload_count
individual_upload_usage_count
individual_word_count
After
(Updated Data Fields):
course_string_prefix
individual_article_views
![Views on user articles]individual_articles_created
individual_courses_count
individual_upload_count
individual_word_count
![Words added by the user via article created]