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

feat(j-s): Display defendant appealed verdict tag for public prosecutor #17303

Merged
merged 5 commits into from
Jan 7, 2025

Conversation

unakb
Copy link
Member

@unakb unakb commented Dec 19, 2024

Asana

What

Display a tag if a defendant appealed the verdict in an indictment case on the public prosecutors office cases page.

Why

So that the public prosecutors office can easily see whether or not the defendant appealed the verdict.

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Formatting passes locally with my changes
  • I have rebased against main before asking for a review

Summary by CodeRabbit

  • New Features

    • Added a new message for "The defendant appeals" in the reviewed cases list.
    • Enhanced the layout of the CasesReviewed component to display a tag indicating if a defendant has appealed the verdict.
    • Introduced a new field in the GraphQL query for retrieving the appeal date related to the verdict for each defendant.
  • Bug Fixes

    • Improved the rendering logic for the indictment review decision in the CasesReviewed component.

@unakb unakb requested a review from a team as a code owner December 19, 2024 15:40
Copy link
Contributor

coderabbitai bot commented Dec 19, 2024

Walkthrough

This pull request introduces updates to the judicial system web application's case review functionality. Key changes include adding a new internationalization message for indicating when a defendant has appealed a verdict, implementing a new GraphQL field for retrieving the appeal date for defendants, and modifying the CasesReviewed component to conditionally display a tag for defendants who have appealed their verdict.

Changes

File Change Summary
apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.strings.ts Added new internationalization message tagDefendantAppealedVerdict with Icelandic text "Ákærði áfrýjar"
apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.tsx - Updated import to include Box component
- Added hasDefendantAppealedVerdict function
- Modified rendering to conditionally display defendant appeal tag
apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql Added verdictAppealDate field to defendants object in Cases query

Sequence Diagram

sequenceDiagram
    participant GraphQL as GraphQL Service
    participant Component as CasesReviewed Component
    participant UI as User Interface

    GraphQL->>Component: Fetch cases with verdictAppealDate
    Component->>Component: Check defendant appeal status
    Component->>UI: Render case list with appeal tags
Loading

Possibly related PRs

Suggested labels

automerge

Suggested reviewers

  • oddsson

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5f533eb and e4d9775.

📒 Files selected for processing (1)
  • apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: prepare

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.tsx (2)

85-89: Add unit tests for the hasDefendantAppealedVerdict function

The helper function is well-implemented with proper null checks and type safety. However, it would benefit from unit tests to verify edge cases.

Would you like me to help generate unit tests for this function? The tests should cover:

  • Empty defendants array
  • Null defendants
  • Mixed cases with some defendants having appeal dates
  • No defendants having appeal dates

156-174: Consider improving tag visual hierarchy

The implementation correctly displays the appeal tag, but consider adjusting the visual hierarchy:

  1. The appeal tag might be more logically placed before the review decision tag since it represents a status change
  2. Both tags could be wrapped in a single Box for consistent spacing

Consider this alternative structure:

-                    <>
-                      <Box marginRight={1}>
-                        <Tag variant="darkerBlue" outlined disabled truncate>
-                          {row.indictmentReviewDecision &&
-                            indictmentReviewDecisionMapping(
-                              row.indictmentReviewDecision,
-                              row.indictmentRulingDecision ===
-                                CaseIndictmentRulingDecision.FINE,
-                            )}
-                        </Tag>
-                      </Box>
-                      {hasDefendantAppealedVerdict(row.defendants) && (
-                        <Box marginTop={1}>
-                          <Tag variant="red" outlined disabled truncate>
-                            {formatMessage(strings.tagDefendantAppealedVerdict)}
-                          </Tag>
-                        </Box>
-                      )}
-                    </>
+                    <Box display="flex" flexDirection="column" gap={1}>
+                      {hasDefendantAppealedVerdict(row.defendants) && (
+                        <Tag variant="red" outlined disabled truncate>
+                          {formatMessage(strings.tagDefendantAppealedVerdict)}
+                        </Tag>
+                      )}
+                      <Tag variant="darkerBlue" outlined disabled truncate>
+                        {row.indictmentReviewDecision &&
+                          indictmentReviewDecisionMapping(
+                            row.indictmentReviewDecision,
+                            row.indictmentRulingDecision ===
+                              CaseIndictmentRulingDecision.FINE,
+                          )}
+                      </Tag>
+                    </Box>
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 53fccb6 and e730044.

📒 Files selected for processing (3)
  • apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.strings.ts (1 hunks)
  • apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.tsx (3 hunks)
  • apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql (1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql (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/web/src/routes/PublicProsecutor/Tables/CasesReviewed.tsx (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/web/src/routes/PublicProsecutor/Tables/CasesReviewed.strings.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."
📓 Learnings (1)
apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.tsx (1)
Learnt from: oddsson
PR: island-is/island.is#14673
File: apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesForReview.tsx:79-79
Timestamp: 2024-11-12T15:15:11.835Z
Learning: The implementation of `mapIndictmentCaseStateToTagVariant` in `TagCaseState.tsx`, which maps only `CaseState.ACCEPTED` to a specific tag and defaults others to an "unknown" status, is intentional as per the application's requirements.
🔇 Additional comments (3)
apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql (1)

31-31: LGTM! Verify GraphQL schema compatibility

The addition of the verdictAppealDate field to the defendants object is well-placed and follows GraphQL naming conventions.

Let's verify the GraphQL schema compatibility:

✅ Verification successful

LGTM - Field is properly defined in the schema

The verdictAppealDate field is properly defined in the backend models and DTOs:

  • It exists as an optional Date field in the Defendant model (apps/judicial-system/backend/src/app/modules/defendant/models/defendant.model.ts)
  • It's included in the update DTO (apps/judicial-system/backend/src/app/modules/defendant/dto/updateDefendant.dto.ts)
  • The field is used consistently across the frontend components for displaying and handling appeal dates
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any GraphQL schema validation errors and field usage

# Search for GraphQL type definitions containing verdictAppealDate
rg -A 5 "type.*Defendant.*{|input.*Defendant.*{" 

# Search for other usages of verdictAppealDate field
rg "verdictAppealDate"

Length of output: 2331


Script:

#!/bin/bash
# Let's fix the regex and search for GraphQL type definitions
rg -A 5 "type\s+Defendant\s*{|input\s+Defendant\s*{"

# Let's also check the GraphQL schema files specifically
fd ".graphql$" --exec cat {}

Length of output: 67559


Script:

#!/bin/bash
# Let's search for Defendant type definition in TypeScript files since GraphQL schema might be generated from them
rg -A 10 "class\s+Defendant|interface\s+Defendant|type\s+Defendant" --type ts

# Also search for verdictAppealDate in TypeScript model/type definitions
rg "verdictAppealDate.*:.*Date" --type ts

Length of output: 17573

apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.strings.ts (1)

63-68: LGTM! String definition follows established patterns

The new message definition for the defendant appeal tag follows the existing conventions for tag messages, with appropriate ID structure and clear description.

apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.tsx (1)

5-5: LGTM! Import statement is properly organized

The Box component import is appropriately added to the existing island-ui/core imports.

Copy link
Member

@thorhildurt thorhildurt left a comment

Choose a reason for hiding this comment

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

Nice, tested and works well. Just left tiny nits but will approve 👍

@oddsson oddsson added the deprecated:automerge (Disabled) Merge this PR as soon as all checks pass label Jan 6, 2025
Copy link

codecov bot commented Jan 6, 2025

Codecov Report

Attention: Patch coverage is 0% with 10 lines in your changes missing coverage. Please review.

Project coverage is 35.67%. Comparing base (a7ec960) to head (4d34e87).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...c/routes/PublicProsecutor/Tables/CasesReviewed.tsx 0.00% 10 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main   #17303      +/-   ##
==========================================
+ Coverage   35.65%   35.67%   +0.02%     
==========================================
  Files        6939     6936       -3     
  Lines      149120   149007     -113     
  Branches    42638    42578      -60     
==========================================
- Hits        53164    53159       -5     
+ Misses      95956    95848     -108     
Flag Coverage Δ
judicial-system-backend 55.85% <ø> (+<0.01%) ⬆️
judicial-system-message 66.66% <ø> (ø)
judicial-system-message-handler 48.41% <ø> (ø)
judicial-system-scheduler 71.17% <ø> (ø)
judicial-system-web 27.76% <0.00%> (-0.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...s/PublicProsecutor/Tables/CasesReviewed.strings.ts 0.00% <ø> (ø)
...c/routes/PublicProsecutor/Tables/CasesReviewed.tsx 0.00% <0.00%> (ø)

... and 10 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a7ec960...4d34e87. Read the comment docs.

@datadog-island-is
Copy link

datadog-island-is bot commented Jan 6, 2025

Datadog Report

Branch report: j-s/defendant-appealed-verdict-tag
Commit report: 6c8a91e
Test service: judicial-system-web

✅ 0 Failed, 336 Passed, 0 Skipped, 57.22s Total Time
➡️ Test Sessions change in coverage: 1 no change

@kodiakhq kodiakhq bot removed the deprecated:automerge (Disabled) Merge this PR as soon as all checks pass label Jan 6, 2025
Copy link
Contributor

kodiakhq bot commented Jan 6, 2025

This PR currently has a merge conflict. Please resolve this and then re-add the automerge label.

@oddsson oddsson added the deprecated:automerge (Disabled) Merge this PR as soon as all checks pass label Jan 7, 2025
@kodiakhq kodiakhq bot merged commit d7a639d into main Jan 7, 2025
37 checks passed
@kodiakhq kodiakhq bot deleted the j-s/defendant-appealed-verdict-tag branch January 7, 2025 14:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deprecated:automerge (Disabled) Merge this PR as soon as all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants