Skip to content
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

Conversation

EmmanuelEmp
Copy link
Contributor

@EmmanuelEmp EmmanuelEmp commented Oct 12, 2024

What this PR Does

Notable Changes

  • The key metric has changed from articles_edited to articles_created.
  • Refactored logic: Replaced the individual_mainspace_edits query (which used the revisions table) with a new approach that uses the articles_courses table.

Expected Data Output

Before

Old Dashboard View

(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

New Dashboard View

(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]

Note: The individual_references_count field has been removed from the data output with the new stats depending on the users articles.

@EmmanuelEmp EmmanuelEmp changed the title Query Fix: remove dependency on deprecited revision table Query Fix: remove dependency on depreciated revision table Oct 12, 2024
@ragesoss
Copy link
Member

Please include before/after screenshots to demonstrate the UI changes associated with this PR.

@EmmanuelEmp
Copy link
Contributor Author

@ragesoss , Okay I'd update this PR with the UI changes

@EmmanuelEmp
Copy link
Contributor Author

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|
Copy link
Member

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.

Copy link
Contributor Author

@EmmanuelEmp EmmanuelEmp Oct 16, 2024

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.

Copy link
Contributor Author

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

Copy link
Member

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.

Copy link
Contributor Author

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

Copy link
Contributor Author

@EmmanuelEmp EmmanuelEmp Oct 17, 2024

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.

@ragesoss
Copy link
Member

Please test your changes locally before pushing them.

@EmmanuelEmp
Copy link
Contributor Author

Some test scripts associated with the user profile pages are resulting in check failures.

I would look into the tests also.

@EmmanuelEmp
Copy link
Contributor Author

EmmanuelEmp commented Oct 19, 2024

@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

Image of the dashboard now(statistics):

image

@ragesoss
Copy link
Member

In that case, you'll need to adjust the test appropriately.

@EmmanuelEmp
Copy link
Contributor Author

Okay, I'd do that.

@ragesoss
Copy link
Member

Please reopen if you return to this.

@ragesoss ragesoss closed this Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants