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

Implement error logging in update_average_pageviews method in UpdateCourseStats #6023

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

arafats1
Copy link
Contributor

This PR addresses issue #4340 by extending error logging during the average pageviews update process in the UpdateCourseStats service. The method update_average_pageviews now includes the update_service parameter when calling AverageViewsImporter.update_outdated_average_views, allowing for improved tracking of errors and Sentry IDs during the course data update process.

I have implemented an isolated test (update_course_stats_average_views_importer_spec.rb) for the update_average_pageviews method within the UpdateCourseStats class. The test validates two main functionalities:

-Updating Outdated Average Views: The test ensures that the AverageViewsImporter.update_outdated_average_views method is called with the correct parameters, specifically checking that it receives the list of articles associated with the course and the update_service instance.

-Logging Progress: The test verifies that the progress of updating average pageviews is logged correctly. This includes checking that the log_update_progress method is invoked with the expected symbol :average_pageviews_updated.

image

@arafats1
Copy link
Contributor Author

arafats1 commented Nov 1, 2024

Hi @ragesoss

Could you kindly take a look at my PR? Thanks

@@ -4,7 +4,8 @@

class AverageViewsImporter
DAYS_UNTIL_OUTDATED = 14
def self.update_outdated_average_views(articles)
def self.update_outdated_average_views(articles, update_service: nil)
Rails.logger.info("Updating average views for articles with update_service: #{update_service}")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume this was just for testing?

update_service.send(:update_average_pageviews)
end

it 'logs the progress of average pageviews updates' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The important aspect of the update service is to collect and log errors that happen during the update, (such as when an API is unavailable or overloaded). Have you tested that functionality?

@arafats1 arafats1 marked this pull request as draft November 7, 2024 06:56
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