fix(j-s): Handling of wrong appeal info #16643
Conversation
WalkthroughThe pull request introduces modifications primarily to the Changes
Possibly related PRs
Suggested labels
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
apps/judicial-system/api/src/app/modules/case-list/interceptors/caseList.interceptor.ts (1)
26-34: Add JSDoc documentation for better maintainability
The function is well-implemented and follows good practices. Consider adding JSDoc documentation to explain the purpose and parameters.
+/**
+ * Checks if both prosecutor and accused have accepted the appeal in court
+ * @param prosecutorAppealDecision - The prosecutor's appeal decision
+ * @param accusedAppealDecision - The accused's appeal decision
+ * @returns true if both parties accepted the appeal, false otherwise
+ */
const wasAcceptedInCourt = (
prosecutorAppealDecision?: CaseAppealDecision,
accusedAppealDecision?: CaseAppealDecision,apps/judicial-system/api/src/app/modules/case/interceptors/case.transformer.spec.ts (1)
596-617: LGTM! Consider adding a clarifying comment.
The test case effectively verifies the handling of mismatched appeal data, particularly when both parties have accepted the appeal decision. This aligns well with the PR objective of fixing appeal info handling.
Consider adding a comment to clarify this edge case:
+ // When both parties have accepted the appeal decision, the case should not be
+ // marked as appealed, regardless of the appeal state, to handle edge cases
+ // where a case is reopened and subsequently closed
it('should transform appeal state and dates if appeal data does not match up', () => {📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- apps/judicial-system/api/src/app/modules/case-list/interceptors/caseList.interceptor.ts (3 hunks)
- apps/judicial-system/api/src/app/modules/case/interceptors/case.transformer.spec.ts (1 hunks)
- apps/judicial-system/api/src/app/modules/case/interceptors/case.transformer.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/judicial-system/api/src/app/modules/case-list/interceptors/caseList.interceptor.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/api/src/app/modules/case/interceptors/case.transformer.spec.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/api/src/app/modules/case/interceptors/case.transformer.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
🔇 Additional comments (4)
apps/judicial-system/api/src/app/modules/case-list/interceptors/caseList.interceptor.ts (2)
11-14: LGTM: Clean import organization
The new import is properly organized and follows TypeScript best practices.
55-63: Document the edge case more explicitly and consider a permanent solution
The comment indicates this is a temporary fix for handling reopened cases. To ensure maintainability:
- Document the specific edge case and its impact more explicitly
- Consider implementing a more permanent solution if this is a recurring issue
Let's verify if this is a common scenario:
apps/judicial-system/api/src/app/modules/case/interceptors/case.transformer.ts (2)
64-70: LGTM! Clear and well-structured appeal acceptance checks.
The new boolean variables improve code readability and provide a clear separation of concerns for tracking appeal acceptance states.
71-71: LGTM! Improved appeal status logic.
The updated condition correctly handles cases where both parties accept in court, fixing the issue with reopened and closed cases mentioned in the PR objectives.
apps/judicial-system/api/src/app/modules/case/interceptors/case.transformer.ts
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16643 +/- ##
==========================================
+ Coverage 36.63% 36.71% +0.08%
==========================================
Files 6873 6859 -14
Lines 143121 142634 -487
Branches 40798 40656 -142
==========================================
- Hits 52432 52368 -64
+ Misses 90689 90266 -423
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 41 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 5 Total Test Services: 0 Failed, 5 Passed Test Services
|
oddsson
added
the
deprecated:automerge
What
Why
Strange behaviour when case is reopened and closed with both having accepted the appeal
Checklist:
Summary by CodeRabbit
getAppealInfofunction to validate appeal state conditions.