Feature/api response contract#32
Draft
Arzu-N wants to merge 6 commits into
Draft
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (17)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (13)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughImplements the full backend data contract for the dashboard API. Adds shared enums and embeddable value objects in ChangesDashboard API data contract
Sequence DiagramsequenceDiagram
participant Client
participant DashboardController
participant DashboardService
participant CleatMapper
participant Repositories
Client->>DashboardController: GET /api/dashboard/dataset (X-Account-Id: uuid)
DashboardController->>DashboardService: getDataset(accountId)
DashboardService->>Repositories: findById(accountId) → AccountEntity or NotFoundException
DashboardService->>Repositories: findAllByAccountId(accountId) → repos, secrets, vulns, members, events
DashboardService->>Repositories: findByAccountId(accountId) → Optional<UsageEntity>
DashboardService->>CleatMapper: toAccountDto, toRepoDto, toSecretFindingDto, toVulnerabilityDto, toUsageDto, toMemberDto, toActivityEventDto
CleatMapper-->>DashboardService: mapped response DTOs
DashboardService->>CleatMapper: toDatasetDto(account, repos, secrets, vulns, usage, members, events)
CleatMapper-->>DashboardService: DatasetDto
DashboardService-->>DashboardController: DatasetDto
DashboardController-->>Client: 200 OK DatasetDto
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 OpenGrep (1.23.0)backend/libs/github-client/src/main/java/dev/cleat/githubclient/config/GitHubConfig.java┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.39][ERROR]: unable to find a config; path backend/libs/github-client/src/main/java/dev/cleat/githubclient/service/TokenManager.java┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.21][ERROR]: unable to find a config; path backend/libs/github-client/src/test/java/dev/cleat/githubclient/config/TestGitHubConfig.java┌──────────────┐ �[32m✔�[39m �[1mOpengrep OSS�[0m [00.17][ERROR]: unable to find a config; path
🔧 PMD (7.25.0)backend/apps/api/src/test/java/dev/cleat/api/AbstractIntegrationTest.java[ERROR] Cannot load ruleset .coderabbit-pmd-ruleset.xml: Cannot resolve rule/ruleset reference '.coderabbit-pmd-ruleset.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath. backend/libs/github-client/src/main/java/dev/cleat/githubclient/config/GitHubConfig.java[ERROR] Cannot load ruleset .coderabbit-pmd-ruleset.xml: Cannot resolve rule/ruleset reference '.coderabbit-pmd-ruleset.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath. backend/libs/github-client/src/main/java/dev/cleat/githubclient/service/TokenManager.java[ERROR] Cannot load ruleset .coderabbit-pmd-ruleset.xml: Cannot resolve rule/ruleset reference '.coderabbit-pmd-ruleset.xml'. Make sure the resource is a valid file or URL and is on the CLASSPATH. Use --debug (or a fine log level) to see the current classpath.
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Nitpick comments (1)
backend/libs/domain/build.gradle.kts (1)
6-6: 📐 Maintainability & Code Quality | 🔵 TrivialRemove unused
spring-boot-starter-webdependency fromlibs/domain.The domain module does not use Spring Web/MVC APIs or servlet functionality. It only uses
@Serviceand@Transactionalannotations. Replace this heavy web starter with minimal Spring context and transaction dependencies to avoid unnecessary coupling and transitive bloat.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@backend/libs/domain/build.gradle.kts` at line 6, The spring-boot-starter-web dependency in the build.gradle.kts file is too heavy for the domain module which only needs Spring context and transaction support for `@Service` and `@Transactional` annotations. Remove the spring-boot-starter-web implementation dependency and replace it with lighter Spring framework dependencies such as spring-context and spring-tx that provide only the core DI and transaction management capabilities without the unnecessary web server and servlet overhead.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@backend/apps/api/src/main/java/dev/cleat/api/controller/DashboardController.java`:
- Around line 22-23: The getDashboardData method in DashboardController accepts
the accountId directly from the client-controlled X-Account-Id header without
validating that the authenticated user has access to that account. To fix this,
extract the authenticated user's identity from the security context (such as
through SecurityContextHolder or a Spring Security Principal parameter),
retrieve the list of accounts the user has access to, verify that the requested
accountId is in that list, and only then proceed with the
dashboardService.getDataset call. If the user does not have access to the
requested accountId, return an appropriate error response (such as 403
Forbidden).
In `@backend/libs/common/build.gradle.kts`:
- Line 8: The implementation dependency on `:libs:persistence` added to the
common module's build.gradle.kts file creates a circular dependency because
persistence already depends on common and imports extensively from it (enums
like Severity, Reachable, AccountType, Plan, Role, EventCategory, Visibility,
Validity and DTOs like BreakdownItem). Remove the line
implementation(project(":libs:persistence")) from the common module's
build.gradle.kts file at line 8, as common should be a foundational module that
other modules depend on, not depend on them. If common requires specific types
defined in persistence, those shared types should be moved or refactored to the
common module instead to maintain proper dependency hierarchy.
In
`@backend/libs/common/src/main/java/dev/cleat/common/dto/request/ActivityEventRequestDto.java`:
- Line 13: The `toActiveEventEntity()` method is missing mappings for two
fields, causing data loss during DTO to entity conversion. Update the method to
map the `repo` field from the DTO to the entity, and map the `eventCategory`
field using the `.setCategory()` setter on the entity (note that the entity
field is named `category`, not `eventCategory`). Ensure both fields are properly
set when converting ActivityEventRequestDto to ActivityEventEntity.
- Around line 8-13: The ActivityEventRequestDto class is missing a member
variable declaration for the `repo` field which is already being used in the
constructor and accessor methods (getter and setter), causing a compile-time
error. Add a private String field declaration for `repo` in the member variables
section alongside the other fields like `type`, `severity`, `actor`, `target`,
`message`, and `eventCategory`. Additionally, update the `toActiveEventEntity`
method in CleatMapper.java to include the mapping of the `repo` field from the
ActivityEventRequestDto to the corresponding field in the entity to prevent data
loss during the DTO-to-entity transformation.
In
`@backend/libs/common/src/main/java/dev/cleat/common/dto/response/ActivityEventResponseDto.java`:
- Line 18: The eventCategory field declared in ActivityEventResponseDto is not
being populated by the CleatMapper.toActivityEventDto() method, causing it to
always be null in responses. Either remove the eventCategory field entirely from
ActivityEventResponseDto along with its accessor methods (getEventCategory and
setEventCategory), or update the CleatMapper.toActivityEventDto() method to
invoke setEventCategory() and populate it from the
activityEventEntity.getEventCategory() method (assuming the entity class has
this attribute).
In
`@backend/libs/common/src/main/java/dev/cleat/common/exception/NotFoundException.java`:
- Around line 5-6: The `@ResponseStatus` annotation on the NotFoundException class
is missing the code parameter, which causes it to default to HTTP 500 status.
Add the code parameter to the `@ResponseStatus` annotation on the
NotFoundException class and set it to HttpStatus.NOT_FOUND to return HTTP 404
status instead of 500, which is the appropriate status code for "not found"
scenarios.
In `@backend/libs/common/src/main/java/dev/cleat/common/mapper/CleatMapper.java`:
- Around line 245-257: The toUsageDto method does not null-check the series
field before calling stream() on it. When DashboardService creates a default
UsageEntity for accounts without usage history, the series field remains null,
causing a NullPointerException. Add a defensive null check for
usageEntity.getSeries() in the setSeries call, either by verifying the series is
not null before streaming or by using a conditional operator to return an empty
list as a fallback when series is null.
In `@backend/libs/domain/src/main/java/dev/cleat/domain/DashboardService.java`:
- Around line 65-66: The fallback UsageEntity() created when findByAccountId
returns empty has a null series field, which causes an NPE when
CleatMapper.toUsageDto attempts to call getSeries().stream(). Fix this by
initializing the series field to an empty collection in the fallback UsageEntity
instance, ensuring that when cleatMapper.toUsageDto receives the entity, it can
safely iterate over the series without encountering a null pointer exception.
In
`@backend/libs/persistence/src/main/java/dev/cleat/persistence/entity/ActivityEventEntity.java`:
- Line 20: The `@Table` annotation on the ActivityEventEntity class specifies the
table name as "activity-entity", but the database migration creates a table
named "activity_event", causing a mismatch that will result in Hibernate failing
to find the table at runtime. Update the `@Table`(name = "activity-entity")
annotation in the ActivityEventEntity class to use the correct table name
"activity_event" to match the actual table created by the migration.
- Around line 27-46: The toActivityEventDto() method dereferences the account
and repo relations without null checks. Since both account and repo are
`@ManyToOne` fields that default to optional (nullable), calling
getAccount().getId() and getRepo().getName() directly will cause
NullPointerException when these relations are null. Add null checks in the
toActivityEventDto() method to verify that activityEventEntity.getAccount() and
activityEventEntity.getRepo() are not null before dereferencing them to call
getId() and getName() respectively, ensuring the mapper handles cases where
these relations may be absent.
In
`@backend/libs/persistence/src/main/java/dev/cleat/persistence/entity/MemberEntity.java`:
- Around line 43-53: Add a `@ManyToOne` relationship field to `AccountEntity` in
the `MemberEntity` class with appropriate `@JoinColumn(name = "account_id")`
annotation to support the `findAllByAccountId(UUID accountId)` derived query
method in `MemberRepository`. Update the constructor (lines 56-75) to accept and
initialize the account parameter alongside the existing fields. Additionally,
create a database schema migration that adds the `account_id` column to the
member table with a foreign key constraint referencing the account table's id
column.
In
`@backend/libs/persistence/src/main/java/dev/cleat/persistence/entity/SecretFindingEntity.java`:
- Line 20: The `@Table` annotation on the SecretFindingEntity class uses
kebab-case naming "secret-finding" which does not match the database schema
table name "secret_finding" that uses snake_case. Update the name parameter in
the `@Table` annotation from "secret-finding" to "secret_finding" to match the
actual database table created by the migration and maintain consistency with the
column naming conventions used elsewhere in this entity.
In
`@backend/libs/persistence/src/main/java/dev/cleat/persistence/entity/UsageEntity.java`:
- Around line 44-63: The constructor in UsageEntity directly assigns the series
list without maintaining the bidirectional relationship. Since UsagePointEntity
owns the foreign key (indicated by mappedBy = "usage"), when assigning series in
the constructor, iterate through each UsagePointEntity in the series list and
call setUsage(this) on each item to ensure the back-reference is properly set.
This ensures data consistency when the entities are persisted to the database.
Apply the same fix to any other place where series is assigned, as mentioned in
the review comment.
- Around line 34-37: The `breakdown` field in UsageEntity has an invalid
`@Column(name = "breakdown")` annotation on an `@ElementCollection` that
contains the embeddable type `BreakdownItem`. The `@Column` annotation is
designed for scalar types only and causes JPA metadata issues with embeddables.
Remove the `@Column(name = "breakdown")` annotation entirely and let JPA
auto-map the three fields from the `BreakdownItem` embeddable class, or
alternatively use `@AttributeOverride` annotations if you need explicit control
over how each embeddable field maps to the database columns. Note that the
underlying database schema will need to be updated to support the three
embeddable fields instead of the current single VARCHAR column.
In
`@backend/libs/persistence/src/main/java/dev/cleat/persistence/repository/UsageRepository.java`:
- Line 9: The findByAccountId() method in UsageRepository references an
accountId field that does not exist in UsageEntity. Either remove the
findByAccountId() method declaration from the UsageRepository interface, or add
the missing field by declaring a private UUID accountId field in the UsageEntity
class and adding a corresponding account_id UUID column to the usage table
schema in the database migration. Choose whichever approach aligns with your
application requirements.
In
`@backend/libs/persistence/src/main/resources/db/migration/V2__update_repo_and_add_tables.sql`:
- Around line 31-42: In the activity_event table migration, the column is
defined as repo but the fk_activity_repo foreign key constraint incorrectly
references repo_id which does not exist. Update the CONSTRAINT fk_activity_repo
foreign key definition to reference the actual column name repo instead of
repo_id to match the table structure and allow the migration to execute
successfully.
- Around line 122-129: In the CREATE TABLE vulnerability statement, rename the
column `packageName` to `package_name` to match the JPA entity mapping used in
VulnerabilityEntity. This change ensures consistency with the snake_case naming
convention used for other columns in the table and aligns with the ORM
configuration to prevent read/write failures.
- Around line 45-54: Add the missing `account_id` column to both the `member`
table (defined at lines 45-54) and the `usage` table (defined at lines 90-97) in
the migration script. These columns are required by the
`MemberRepository.findAllByAccountId()` and `usageRepository.findByAccountId()`
methods respectively. Add an `account_id` column of appropriate type (likely
UUID or similar) to each table definition, ensuring it can be used as a foreign
key reference to link members and usage records to their associated accounts.
- Around line 1-12: The SQL migration file V2__update_repo_and_add_tables.sql
contains multiple syntax errors that will block Flyway execution. Remove the
duplicate closing parenthesis and semicolon at the end of the scorecard_check
table definition (there should be only one `);` to terminate the CREATE TABLE
statement). Additionally, audit the entire migration file for the following
issues: eliminate any trailing commas on the final column before closing
parentheses in CREATE TABLE statements, ensure semicolons only appear at the end
of complete SQL statements (not inside CREATE TABLE definitions), and verify all
column definitions have proper commas between them (check for missing commas
after column definitions like `DEFAULT 0.0` before the next column).
In `@backend/README.md`:
- Around line 60-61: The "Local dependencies (Postgres, Redis) are expected to
run via Docker Compose" statement appears twice in the README.md file, once
around line 60-61 and again around line 80, with different endings that create
inconsistency. Identify both instances of this duplicated statement and remove
one of the duplicate occurrences, keeping the version that provides the most
complete and useful information for setup guidance. This will eliminate the
redundancy and ensure consistent documentation.
---
Nitpick comments:
In `@backend/libs/domain/build.gradle.kts`:
- Line 6: The spring-boot-starter-web dependency in the build.gradle.kts file is
too heavy for the domain module which only needs Spring context and transaction
support for `@Service` and `@Transactional` annotations. Remove the
spring-boot-starter-web implementation dependency and replace it with lighter
Spring framework dependencies such as spring-context and spring-tx that provide
only the core DI and transaction management capabilities without the unnecessary
web server and servlet overhead.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 435934ee-3685-4dc0-b53a-85f72daf163a
📒 Files selected for processing (58)
backend/README.mdbackend/apps/api/src/main/java/dev/cleat/api/CleatApiApplication.javabackend/apps/api/src/main/java/dev/cleat/api/controller/DashboardController.javabackend/gradlewbackend/libs/common/build.gradle.ktsbackend/libs/common/src/main/java/dev/cleat/common/dto/BreakdownItem.javabackend/libs/common/src/main/java/dev/cleat/common/dto/UsagePointDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/request/AccountRequestDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/request/ActivityEventRequestDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/request/MemberRequestDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/request/RepoRequestDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/request/ScorecardCheckRequestDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/request/SecretFindingRequestDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/request/VulnerabilityRequestDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/response/AccountResponseDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/response/ActivityEventResponseDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/response/DatasetDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/response/MemberResponseDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/response/RepoResponseDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/response/ScorecardCheckResponseDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/response/SecretFindingResponseDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/response/UsageResponseDto.javabackend/libs/common/src/main/java/dev/cleat/common/dto/response/VulnerabilityResponseDto.javabackend/libs/common/src/main/java/dev/cleat/common/enums/AccountType.javabackend/libs/common/src/main/java/dev/cleat/common/enums/EventCategory.javabackend/libs/common/src/main/java/dev/cleat/common/enums/Plan.javabackend/libs/common/src/main/java/dev/cleat/common/enums/Reachable.javabackend/libs/common/src/main/java/dev/cleat/common/enums/Role.javabackend/libs/common/src/main/java/dev/cleat/common/enums/Severity.javabackend/libs/common/src/main/java/dev/cleat/common/enums/Validity.javabackend/libs/common/src/main/java/dev/cleat/common/enums/Visibility.javabackend/libs/common/src/main/java/dev/cleat/common/exception/NotFoundException.javabackend/libs/common/src/main/java/dev/cleat/common/mapper/CleatMapper.javabackend/libs/domain/build.gradle.ktsbackend/libs/domain/src/main/java/dev/cleat/domain/DashboardService.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/AccountType.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/Plan.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/Visibility.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/entity/AccountEntity.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/entity/ActivityEventEntity.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/entity/MemberEntity.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/entity/RepoEntity.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/entity/ScorecardCheckEntity.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/entity/SecretFindingEntity.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/entity/UsageEntity.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/entity/UsagePointEntity.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/entity/VulnerabilityEntity.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/repository/AccountRepository.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/repository/ActivityEventRepository.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/repository/MemberRepository.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/repository/RepoRepository.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/repository/ScorecardCheckRepository.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/repository/SecretFindingRepository.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/repository/UsagePointRepository.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/repository/UsageRepository.javabackend/libs/persistence/src/main/java/dev/cleat/persistence/repository/VulnerabilityRepository.javabackend/libs/persistence/src/main/resources/db/migration/V2__update_repo_and_add_tables.sqlbackend/libs/persistence/src/test/java/dev/cleat/persistence/AccountRepositoryTest.java
💤 Files with no reviewable changes (3)
- backend/libs/persistence/src/main/java/dev/cleat/persistence/AccountType.java
- backend/libs/persistence/src/main/java/dev/cleat/persistence/Plan.java
- backend/libs/persistence/src/main/java/dev/cleat/persistence/Visibility.java
Comment on lines
+22
to
+23
| public ResponseEntity<DatasetDto> getDashboardData(@RequestHeader("X-Account-Id") UUID accountId) { | ||
| return ResponseEntity.ok(dashboardService.getDataset(accountId)); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Do not trust tenant scope from client-controlled header alone.
Using X-Account-Id directly as data scope enables cross-account access unless a trusted gateway overwrites it and server-side auth enforces membership.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/apps/api/src/main/java/dev/cleat/api/controller/DashboardController.java`
around lines 22 - 23, The getDashboardData method in DashboardController accepts
the accountId directly from the client-controlled X-Account-Id header without
validating that the authenticated user has access to that account. To fix this,
extract the authenticated user's identity from the security context (such as
through SecurityContextHolder or a Spring Security Principal parameter),
retrieve the list of accounts the user has access to, verify that the requested
accountId is in that list, and only then proceed with the
dashboardService.getDataset call. If the user does not have access to the
requested accountId, return an appropriate error response (such as 403
Forbidden).
Comment on lines
+8
to
+13
| private String type; | ||
| private Severity severity; | ||
| private String actor; | ||
| private String target; | ||
| private String message; | ||
| private EventCategory eventCategory; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
cat -n backend/libs/common/src/main/java/dev/cleat/common/dto/request/ActivityEventRequestDto.javaRepository: Devlaner/cleat
Length of output: 3105
🏁 Script executed:
# Search for mapper classes that might process ActivityEventRequestDto
rg "ActivityEventRequestDto" backend/libs/common --type java -A 5 -B 5Repository: Devlaner/cleat
Length of output: 12705
🏁 Script executed:
# Get the full mapper method for toActiveEventEntity
sed -n '/<ActivityEventRequestDto>/,/^ }/p' backend/libs/common/src/main/java/dev/cleat/common/mapper/CleatMapper.java | head -30Repository: Devlaner/cleat
Length of output: 152
🏁 Script executed:
# More targeted search for the toActiveEventEntity method
ast-grep run --pattern 'toActiveEventEntity' --lang java backend/libs/common/src/main/java/dev/cleat/common/mapper/CleatMapper.javaRepository: Devlaner/cleat
Length of output: 334
🏁 Script executed:
# Search for the exact method with broader context
rg "toActiveEventEntity" backend/libs/common/src/main/java/dev/cleat/common/mapper/CleatMapper.java -A 15Repository: Devlaner/cleat
Length of output: 962
Add missing repo field declaration.
The repo field is used in the constructor (line 30), getter (line 71), and setter (line 75) but is not declared in the member variables (lines 8-13), causing a compile-time error.
Additionally, the mapper method toActiveEventEntity in CleatMapper.java (line 122) does not map the repo field from the request DTO, causing data loss at the DTO-to-entity boundary.
Proposed fix
private String target;
+ private String repo;
private String message;The mapper should also include:
.setActor(activityEventRequestDto.getActor())
.setTarget(activityEventRequestDto.getTarget())
+ .setRepo(activityEventRequestDto.getRepo())
.setMessage(activityEventRequestDto.getMessage())📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| private String type; | |
| private Severity severity; | |
| private String actor; | |
| private String target; | |
| private String message; | |
| private EventCategory eventCategory; | |
| private String type; | |
| private Severity severity; | |
| private String actor; | |
| private String target; | |
| private String repo; | |
| private String message; | |
| private EventCategory eventCategory; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/libs/common/src/main/java/dev/cleat/common/dto/request/ActivityEventRequestDto.java`
around lines 8 - 13, The ActivityEventRequestDto class is missing a member
variable declaration for the `repo` field which is already being used in the
constructor and accessor methods (getter and setter), causing a compile-time
error. Add a private String field declaration for `repo` in the member variables
section alongside the other fields like `type`, `severity`, `actor`, `target`,
`message`, and `eventCategory`. Additionally, update the `toActiveEventEntity`
method in CleatMapper.java to include the mapping of the `repo` field from the
ActivityEventRequestDto to the corresponding field in the entity to prevent data
loss during the DTO-to-entity transformation.
| private String actor; | ||
| private String target; | ||
| private String message; | ||
| private EventCategory eventCategory; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, locate the ActivityEventRequestDto file
fd -type f -name "ActivityEventRequestDto.java"Repository: Devlaner/cleat
Length of output: 323
🏁 Script executed:
# Also locate the mapper file
fd -type f -name "CleatMapper.java"Repository: Devlaner/cleat
Length of output: 323
🏁 Script executed:
# Check the ActivityEventEntity file
fd -type f -name "ActivityEventEntity.java"Repository: Devlaner/cleat
Length of output: 323
🏁 Script executed:
fd -type f "ActivityEventRequestDto.java"Repository: Devlaner/cleat
Length of output: 323
🏁 Script executed:
fd -type f "CleatMapper.java"Repository: Devlaner/cleat
Length of output: 323
🏁 Script executed:
fd -type f "ActivityEventEntity.java"Repository: Devlaner/cleat
Length of output: 323
🏁 Script executed:
fd -t f "ActivityEventRequestDto.java"Repository: Devlaner/cleat
Length of output: 244
🏁 Script executed:
fd -t f "CleatMapper.java"Repository: Devlaner/cleat
Length of output: 227
🏁 Script executed:
fd -t f "ActivityEventEntity.java"Repository: Devlaner/cleat
Length of output: 245
🏁 Script executed:
cat -n backend/libs/common/src/main/java/dev/cleat/common/dto/request/ActivityEventRequestDto.javaRepository: Devlaner/cleat
Length of output: 3105
🏁 Script executed:
cat -n backend/libs/common/src/main/java/dev/cleat/common/mapper/CleatMapper.javaRepository: Devlaner/cleat
Length of output: 15993
🏁 Script executed:
cat -n backend/libs/persistence/src/main/java/dev/cleat/persistence/entity/ActivityEventEntity.javaRepository: Devlaner/cleat
Length of output: 5492
Fix mapper to preserve repo and event category in ActivityEventEntity mapping.
The toActiveEventEntity() method at lines 122-130 omits mapping for repo and eventCategory, causing data loss. The entity has a category field (not eventCategory), so the correct setter is .setCategory().
Required mapper fix
public ActivityEventEntity toActiveEventEntity(ActivityEventRequestDto activityEventRequestDto) {
if (activityEventRequestDto == null) return null;
return new ActivityEventEntity()
.setType(activityEventRequestDto.getType())
.setSeverity(activityEventRequestDto.getSeverity())
.setActor(activityEventRequestDto.getActor())
.setTarget(activityEventRequestDto.getTarget())
+ .setRepo(activityEventRequestDto.getRepo())
.setMessage(activityEventRequestDto.getMessage())
+ .setCategory(activityEventRequestDto.getEventCategory());
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/libs/common/src/main/java/dev/cleat/common/dto/request/ActivityEventRequestDto.java`
at line 13, The `toActiveEventEntity()` method is missing mappings for two
fields, causing data loss during DTO to entity conversion. Update the method to
map the `repo` field from the DTO to the entity, and map the `eventCategory`
field using the `.setCategory()` setter on the entity (note that the entity
field is named `category`, not `eventCategory`). Ensure both fields are properly
set when converting ActivityEventRequestDto to ActivityEventEntity.
| private String repo; | ||
| private String message; | ||
| private OffsetDateTime createdAt; | ||
| private EventCategory eventCategory; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
EventCategory field in ActivityEventResponseDto is never populated by the mapper.
The eventCategory field (line 18) and its accessor methods (lines 101–108) are declared in the DTO, but the upstream CleatMapper.toActivityEventDto() method does not invoke setEventCategory(). This means the field will always be null in JSON responses, creating a contract mismatch between the advertised DTO shape and what the service actually returns.
Either remove the unused field from the DTO, or ensure the mapper populates it from activityEventEntity.getEventCategory() (assuming the entity has this attribute).
Also applies to: 101-108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/libs/common/src/main/java/dev/cleat/common/dto/response/ActivityEventResponseDto.java`
at line 18, The eventCategory field declared in ActivityEventResponseDto is not
being populated by the CleatMapper.toActivityEventDto() method, causing it to
always be null in responses. Either remove the eventCategory field entirely from
ActivityEventResponseDto along with its accessor methods (getEventCategory and
setEventCategory), or update the CleatMapper.toActivityEventDto() method to
invoke setEventCategory() and populate it from the
activityEventEntity.getEventCategory() method (assuming the entity class has
this attribute).
Comment on lines
+1
to
+12
| CREATE TABLE scorecard_check( | ||
| id UUID PRIMARY KEY, | ||
| name VARCHAR(255), | ||
| score INTEGER, | ||
| reason TEXT, | ||
| repo_id UUID, | ||
|
|
||
| CONSTRAINT fk_scorecard_repo | ||
| FOREIGN KEY (repo_id) | ||
| REFERENCES repo(id) | ||
| ON DELETE CASCADE); | ||
| ); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
cat -n backend/libs/persistence/src/main/resources/db/migration/V2__update_repo_and_add_tables.sqlRepository: Devlaner/cleat
Length of output: 8238
Fix SQL syntax errors blocking Flyway migration execution.
The migration file contains five critical SQL syntax errors that will prevent execution:
- Lines 11-12 and 21-22: Duplicate closing parentheses (extra
);) - Line 54: Trailing comma on final column before table close
- Line 87: Semicolon terminating foreign key constraint inside CREATE TABLE (should be part of table definition, not statement terminator)
- Line 113: Missing comma after
DEFAULT 0.0beforeusage_idcolumn
Suggested patch
CREATE TABLE scorecard_check(
@@
- ON DELETE CASCADE);
-);
+ ON DELETE CASCADE
+);
@@
CREATE TABLE repo_topics(
@@
- ON DELETE CASCADE);
-);
+ ON DELETE CASCADE
+);
@@
CREATE TABLE member (
@@
- repo_access INTEGER,
+ repo_access INTEGER
@@
);
@@
CREATE TABLE secret_finding(
@@
- FOREIGN KEY (repo)
- REFERENCES repo(id);
+ FOREIGN KEY (repo)
+ REFERENCES repo(id)
);
@@
CREATE TABLE usage_point(
@@
- cost DECIMAL(19, 4) DEFAULT 0.0
+ cost DECIMAL(19, 4) DEFAULT 0.0,
usage_id UUID,🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/libs/persistence/src/main/resources/db/migration/V2__update_repo_and_add_tables.sql`
around lines 1 - 12, The SQL migration file V2__update_repo_and_add_tables.sql
contains multiple syntax errors that will block Flyway execution. Remove the
duplicate closing parenthesis and semicolon at the end of the scorecard_check
table definition (there should be only one `);` to terminate the CREATE TABLE
statement). Additionally, audit the entire migration file for the following
issues: eliminate any trailing commas on the final column before closing
parentheses in CREATE TABLE statements, ensure semicolons only appear at the end
of complete SQL statements (not inside CREATE TABLE definitions), and verify all
column definitions have proper commas between them (check for missing commas
after column definitions like `DEFAULT 0.0` before the next column).
Comment on lines
+31
to
+42
| repo UUID, | ||
| message TEXT, | ||
| created_at TIMESTAMP WITH TIME ZONE, | ||
| category VARCHAR(50), | ||
|
|
||
| CONSTRAINT fk_activity_account | ||
| FOREIGN KEY (account_id) | ||
| REFERENCES account(id), | ||
|
|
||
| CONSTRAINT fk_activity_repo | ||
| FOREIGN KEY (repo_id) | ||
| REFERENCES repo(id) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical
🧩 Analysis chain
🏁 Script executed:
find . -type f -name "V2__update_repo_and_add_tables.sql" 2>/dev/nullRepository: Devlaner/cleat
Length of output: 246
🏁 Script executed:
cat -n ./backend/libs/persistence/src/main/resources/db/migration/V2__update_repo_and_add_tables.sqlRepository: Devlaner/cleat
Length of output: 8238
Fix activity_event FK constraint column reference.
Line 31 defines column repo, but line 41 references repo_id in the FK constraint. This will cause the migration to fail since repo_id does not exist in the table. Change the FK constraint to reference the actual column name repo:
Suggested fix
CONSTRAINT fk_activity_repo
FOREIGN KEY (repo)
REFERENCES repo(id)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/libs/persistence/src/main/resources/db/migration/V2__update_repo_and_add_tables.sql`
around lines 31 - 42, In the activity_event table migration, the column is
defined as repo but the fk_activity_repo foreign key constraint incorrectly
references repo_id which does not exist. Update the CONSTRAINT fk_activity_repo
foreign key definition to reference the actual column name repo instead of
repo_id to match the table structure and allow the migration to execute
successfully.
Comment on lines
+45
to
+54
| CREATE TABLE member ( | ||
| id UUID PRIMARY KEY, | ||
| login VARCHAR(255), | ||
| name VARCHAR(255), | ||
| role VARCHAR(50), | ||
| two_factor BOOLEAN, | ||
| last_active_at TIMESTAMP WITH TIME ZONE, | ||
| outside_collaborator BOOLEAN, | ||
| repo_access INTEGER, | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the migration file
find . -name "*V2__update_repo_and_add_tables.sql" -type fRepository: Devlaner/cleat
Length of output: 246
🏁 Script executed:
# Read the migration file to examine the member table (lines 45-54) and usage table (lines 90-97)
cat -n ./backend/libs/persistence/src/main/resources/db/migration/V2__update_repo_and_add_tables.sql | sed -n '40,100p'Repository: Devlaner/cleat
Length of output: 3618
🏁 Script executed:
# Find the MemberRepository file
find . -name "MemberRepository.java" -type fRepository: Devlaner/cleat
Length of output: 248
🏁 Script executed:
# Read the MemberRepository file
cat -n ./backend/libs/persistence/src/main/java/dev/cleat/persistence/repository/MemberRepository.javaRepository: Devlaner/cleat
Length of output: 573
🏁 Script executed:
# Find the DashboardService file
find . -name "DashboardService.java" -type fRepository: Devlaner/cleat
Length of output: 227
🏁 Script executed:
# Read the DashboardService file around line 62
cat -n ./backend/libs/domain/src/main/java/dev/cleat/domain/DashboardService.java | sed -n '55,70p'Repository: Devlaner/cleat
Length of output: 1217
Add account_id columns to member and usage tables.
MemberRepository.findAllByAccountId(...) (line 9) and usageRepository.findByAccountId(...) (line 66 in DashboardService.java) require these columns, but the member table (lines 45-54) and usage table (lines 90-97) in the migration lack them, preventing these queries from executing correctly.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/libs/persistence/src/main/resources/db/migration/V2__update_repo_and_add_tables.sql`
around lines 45 - 54, Add the missing `account_id` column to both the `member`
table (defined at lines 45-54) and the `usage` table (defined at lines 90-97) in
the migration script. These columns are required by the
`MemberRepository.findAllByAccountId()` and `usageRepository.findByAccountId()`
methods respectively. Add an `account_id` column of appropriate type (likely
UUID or similar) to each table definition, ensuring it can be used as a foreign
key reference to link members and usage records to their associated accounts.
Comment on lines
+122
to
+129
| CREATE TABLE vulnerability( | ||
| id UUID PRIMARY KEY, | ||
| account_id UUID, | ||
| packageName VARCHAR(255), | ||
| ecosystem VARCHAR(255), | ||
| current_version VARCHAR(255), | ||
| fixed_version VARCHAR(255), | ||
| cvss DOUBLE PRECISION, |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
find . -name "V2__update_repo_and_add_tables.sql" -type fRepository: Devlaner/cleat
Length of output: 246
🏁 Script executed:
find . -name "VulnerabilityEntity.java" -type fRepository: Devlaner/cleat
Length of output: 247
🏁 Script executed:
sed -n '122,135p' ./backend/libs/persistence/src/main/resources/db/migration/V2__update_repo_and_add_tables.sqlRepository: Devlaner/cleat
Length of output: 1002
🏁 Script executed:
cat -n ./backend/libs/persistence/src/main/java/dev/cleat/persistence/entity/VulnerabilityEntity.java | head -80Repository: Devlaner/cleat
Length of output: 2875
Fix column naming mismatch in vulnerability table.
Line 125 uses packageName, but VulnerabilityEntity maps to package_name via JPA annotation (line 34). This mismatch will cause ORM failures on read/write operations. Rename the SQL column to package_name to match the entity configuration and maintain consistency with other columns in the table.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@backend/libs/persistence/src/main/resources/db/migration/V2__update_repo_and_add_tables.sql`
around lines 122 - 129, In the CREATE TABLE vulnerability statement, rename the
column `packageName` to `package_name` to match the JPA entity mapping used in
VulnerabilityEntity. This change ensures consistency with the snake_case naming
convention used for other columns in the table and aligns with the ORM
configuration to prevent read/write failures.
Comment on lines
+60
to
+61
| Local dependencies (Postgres, Redis) are expected to run via Docker Compose. The | ||
| two services build into one container image each. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove duplicated “Local dependencies…” statement.
The sentence appears twice with different endings, which makes the setup guidance inconsistent.
Also applies to: 80-80
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@backend/README.md` around lines 60 - 61, The "Local dependencies (Postgres,
Redis) are expected to run via Docker Compose" statement appears twice in the
README.md file, once around line 60-61 and again around line 80, with different
endings that create inconsistency. Identify both instances of this duplicated
statement and remove one of the duplicate occurrences, keeping the version that
provides the most complete and useful information for setup guidance. This will
eliminate the redundancy and ensure consistent documentation.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this change?
Related issue
Closes #7
Area
Screenshots
If this touches the UI, drop before and after screenshots here.
Checklist
bun run typecheck && bun run buildinapps/web(for frontend changes)Summary by CodeRabbit
New Features
Documentation