-
Notifications
You must be signed in to change notification settings - Fork 10
fix(logs): fixed total count for success and changed events names #3865
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
Conversation
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 PR fixes logging issues in the import functionality with some important corrections: fixes the total count calculation for successful imports, renames log action types for consistency, and improves test quality by removing mocks. However, there's a critical syntax error in the controller method that needs immediate attention.
packages/backend/src/usagers/controllers/import/import.controller.spec.ts
Show resolved
Hide resolved
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3865 +/- ##
==========================================
- Coverage 65.92% 65.91% -0.02%
==========================================
Files 971 972 +1
Lines 16255 16260 +5
Branches 2255 2252 -3
==========================================
+ Hits 10716 10717 +1
- Misses 5334 5340 +6
+ Partials 205 203 -2
Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
0a06bc7
to
824b3d5
Compare
packages/backend/src/usagers/controllers/import/import.controller.spec.ts
Outdated
Show resolved
Hide resolved
packages/backend/src/usagers/controllers/import/import.controller.ts
Outdated
Show resolved
Hide resolved
|
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 PR fixes logging issues by correcting the total count calculation for successful imports and standardizing event naming. The changes improve test quality by replacing mocks with actual database operations and fix a missing user context issue in document download logging. The migration properly handles existing data migration for the renamed events.
expect(logs[0].action).toBe("IMPORT_USAGERS_SUCCESS"); | ||
expect(logs[0].context).toEqual({ | ||
nombreActifs: 19, | ||
nombreTotal: 19, |
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.
The updated assertion nombreTotal: 19
correctly reflects the fix where total count now represents processed users rather than all preview rows. This aligns with the controller change that uses usagersRows.length
instead of importPreviewRows.length
.
|
||
export class ManualMigration1756117243336 implements MigrationInterface { | ||
public async up(queryRunner: QueryRunner): Promise<void> { | ||
await queryRunner.query(` |
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.
The migration properly handles the event name changes with proper rollback capability. Consider adding a verification query after the update to ensure the migration succeeded and log the number of affected rows for operational visibility.
🎉 This PR is included in version 2.220.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
No description provided.