Fix how to calculate error_count in flags['update_logs'] for courses

[arafats1]

This PR addresses issue #5580 by ensuring that the error_count in flags['update_logs'] properly tracks the number of actual errors during the course update process.

• Modified UpdateServiceErrorHelper.update_error_stats to accept an optional new_errors_count parameter. This parameter should default to 1 if not provided.
• ModifiedApiErrorHandling.log_error to pass the correct error count using the sentry_extra[:new_errors_count] value (if available).
• Added isolated tests for ApiErrorHandling focusing on the log_error method to ensure it accurately increments the error count based on various scenarios.
• Added isolated tests for update_service_error_helper_spec.rb to validate the functionality of the update_error_stats method.

[EmmanuelEmp]

Hey @arafats1, I think changes in your "schema.rb" file are auto-generated on setup. And should not be included in your commits.

[arafats1]

Hey @arafats1, I think changes in your "schema.rb" file are auto-generated on setup. And should not be included in your commits.

Hi @EmmanuelEmp, Oh yes, I missed that, should I run rake erd orientation=vertical and annotate, Will that rectify it? Kindly guide me, thank you.

[gabina]

Please avoid including changes to unrelated files like .gitignore, db/schema.rb, or yarn.lock in this PR. Kindly add a commit to revert changes in those files.

[ragesoss]

In addition to the cleanup of unrelated file changes, please include a detailed explanation of how you've tested this change. With this sort of change, it is not easy to verify the correctness just by looking at the code.

[arafats1]

Yes, after usingputs logging, the other errors are from web requests being prevented. The WebMock exceptions I'm encountering seem to be affecting multiple revision IDs. The log indicates that the tests are attempting to make HTTP requests, which are being intercepted and blocked by WebMock.

At the start of the logs

At the end

[ragesoss]

That's the expected behavior, as we only allow web requests within explicit VCR blocks from the test suite.

In this case, I think the best strategy would be to test the changed functionality in a more isolated way, without relying on UpdateCourseStats, since your test was specific to the LiftWingApi.

[arafats1]

I took

That's the expected behavior, as we only allow web requests within explicit VCR blocks from the test suite.

In this case, I think the best strategy would be to test the changed functionality in a more isolated way, without relying on UpdateCourseStats, since your test was specific to the LiftWingApi.

I took into consideration isolated testing, I reverted back to the master branch and did the specific changes as I tested them independently. I did not make any changes to the LiftWingApi class. Given that the LiftWingApi class already implements error handling and calls log_error with the necessary context, it doesn't seem to require changes for the error_count calculation in flags['update_logs'].

Based on the provided information, I made and tested changes in the UpdateServiceErrorHelper and ApiErrorHandling classes

1. For changes in UpdateServiceErrorHelper : I added isolated tests for the module to validate the functionality of the update_error_stats method. (file: update_service_error_helper_spec.rb)

The tests cover the following scenarios: Default Increment: The first test verifies that calling update_error_stats without any arguments correctly increments the error_count by 1.

Custom Increment: The second test checks that when a specific number of errors is provided as an argument, update_error_stats increments the error_count by that exact number.

Multiple Increments: The third test confirms that the error_count can be incremented multiple times with different values, ensuring cumulative updates work as intended.

2. For changes in ApiErrorHandling: I also added isolated tests for this module focusing on the log_error method to ensure it accurately increments the error count based on various scenarios. (file: api_error_handling_helper_spec.rb)

The tests include the following: Default Increment: The first test verifies that calling log_error without a new_errors_count argument correctly increments the error_count by 1.

Custom Increment: The second test checks that when a specific new_errors_count is provided in the sentry_extra argument, log_error increments the error_count by that exact number.

Missing Argument Handling: The third test confirms that if the sentry_extra argument is missing, the method defaults to incrementing the error_count by 1.

I pushed the changes with the test files for review, let me know if I should remove the files and re-commit.

[ragesoss]

There are linting errors in the build.

[arafats1]

There are linting errors in the build.

Let me check them out.

[arafats1]

There are linting errors in the build.

Fixed the linting errors.

[ragesoss]

Nice job. I like the way you implemented the tests for this, mirroring how we actually want to use the logging module.

[arafats1]

Nice job. I like the way you implemented the tests for this, mirroring how we actually want to use the logging module.

Thank you!!!