-
Notifications
You must be signed in to change notification settings - Fork 714
Stage #9197
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
Stage #9197
Conversation
…-0.3.26 chore(deps): bump typeorm from 0.3.24 to 0.3.27
* fix: desktop-timer error message, offline timer * fix: cspell Behaviour * fix: missing startedAt while update timer status synced * fix: adding manual timelog when timer started and stopped in offline mode and already have another starting timer * style: remove unused code * fix: handle another condition offline timer * fix: remove unnecessary options * fix: change error plain text to internationalization * fix: rename key and added NETWOR_ERROR translation
* Merge pull request #9183 from ever-co/dependabot/npm_and_yarn/typeorm-0.3.26 chore(deps): bump typeorm from 0.3.24 to 0.3.27 * fix: desktop-timer error message, offline timer (#9180) * fix: desktop-timer error message, offline timer * fix: cspell Behaviour * fix: missing startedAt while update timer status synced * fix: adding manual timelog when timer started and stopped in offline mode and already have another starting timer * style: remove unused code * fix: handle another condition offline timer * fix: remove unnecessary options * fix: change error plain text to internationalization * fix: rename key and added NETWOR_ERROR translation --------- Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: syns <[email protected]>
|
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Greptile Overview
Greptile Summary
This PR merges changes from the develop branch into stage-apps, bringing in error handling improvements and offline timer sync enhancements for the desktop timer application.
Key Changes:
- Introduced centralized
ErrorMappingservice to provide user-friendly, translated error messages for HTTP errors - Refactored error handling across components to use the new notification service with duplicate prevention
- Enhanced offline timer synchronization logic to handle edge cases: timers started/stopped offline without IDs, and proper state reconciliation
- Added three new API methods (
getTimeLogById,updateTimeLog,addTimeLog) to support complex offline sync scenarios - Updated TypeORM from 0.3.24 to 0.3.27 across all packages
- Added comprehensive error message translations in 13 languages
Issues Found:
- Critical:
UnauthorizedInterceptorhas a broken navigation flow on line 40-44 whereconcatMapis called but not returned, preventing redirect to login page on 401 errors
Confidence Score: 2/5
- This PR contains a critical bug that will break user authentication flow when sessions expire
- Score reflects the critical logic error in
UnauthorizedInterceptorthat prevents proper redirect on 401 errors. TheconcatMapoperator is created but never returned, making the navigation non-functional. This will break the authentication flow when tokens expire, leaving users unable to re-authenticate properly. The rest of the code appears sound with good improvements to error handling and offline sync. - Critical attention needed for
packages/desktop-ui-lib/src/lib/interceptors/unauthorized.interceptor.ts- the navigation logic is broken and must be fixed before merge
Important Files Changed
File Analysis
| Filename | Score | Overview |
|---|---|---|
| packages/desktop-ui-lib/src/lib/interceptors/unauthorized.interceptor.ts | 1/5 | Added error mapping but introduced critical bug - navigation logic not returning concatMap result, breaking unauthorized redirect |
| packages/desktop-ui-lib/src/lib/services/error-mapping.service.ts | 5/5 | New service for centralizing HTTP error message mapping with translation support |
| packages/desktop-ui-lib/src/lib/time-tracker/time-tracker.service.ts | 4/5 | Added three new methods: getTimeLogById, updateTimeLog, and addTimeLog for offline sync improvements |
| packages/desktop-ui-lib/src/lib/offline-sync/concretes/sequence-queue.ts | 4/5 | Enhanced offline timer sync logic to handle edge cases: existing time logs, stopped timers without IDs, and proper state checking |
Sequence Diagram
sequenceDiagram
participant User
participant TimeTrackerComponent
participant TimeTrackerService
participant HttpInterceptor
participant ErrorMapping
participant ToastrNotification
participant SequenceQueue
participant Backend
User->>TimeTrackerComponent: Start/Stop Timer (Offline)
TimeTrackerComponent->>TimeTrackerService: toggleApiStart/Stop()
TimeTrackerService->>HttpInterceptor: HTTP Request
alt Network Available
HttpInterceptor->>Backend: Forward Request
Backend-->>HttpInterceptor: Response
HttpInterceptor-->>TimeTrackerService: Success
TimeTrackerService-->>TimeTrackerComponent: Timer Data
else Network Error
HttpInterceptor->>ErrorMapping: mapErrorMessage(error)
ErrorMapping-->>HttpInterceptor: Translated Error Message
HttpInterceptor-->>TimeTrackerService: Throw Error
TimeTrackerService->>ToastrNotification: error(message)
ToastrNotification-->>User: Display Error Toast
TimeTrackerComponent->>SequenceQueue: Queue Offline Timer
end
Note over SequenceQueue: When Back Online
SequenceQueue->>TimeTrackerService: getTimeLogById(timelogId)
TimeTrackerService->>Backend: GET /timesheet/time-log/:id
Backend-->>TimeTrackerService: TimeLog Data
alt Timer Started Offline (No TimeLog ID)
SequenceQueue->>TimeTrackerService: addTimeLog(payload)
TimeTrackerService->>Backend: POST /timesheet/time-log
Backend-->>SequenceQueue: New TimeLog
else Timer Stopped Offline (TimeLog Exists)
SequenceQueue->>TimeTrackerService: updateTimeLog(id, payload)
TimeTrackerService->>Backend: PUT /timesheet/time-log/:id
Backend-->>SequenceQueue: Updated TimeLog
end
SequenceQueue->>ToastrNotification: success("Synced")
ToastrNotification-->>User: Display Success Toast
Additional Comments (1)
-
packages/desktop-ui-lib/src/lib/interceptors/unauthorized.interceptor.ts, line 40-44 (link)logic:
concatMapis called but its result is never used or returned, making the navigation non-functional
30 files reviewed, 1 comment
PR
Please note: we will close your PR without comment if you do not check the boxes above and provide ALL requested information.