Architechture/enum set up - #2
Merged
Merged
Conversation
…t); introduce Attachment entity for file metadata
…improve consistency in data handling partly after review from code rabbit
…erminology, and adjust MVP priorities
gitnes94
requested review from
Ericthilen,
alicewersen-rgb and
codebyNorthsteep
March 30, 2026 09:11
|
Caution Review failedPull request was closed or merged during review 📝 WalkthroughWalkthroughThis pull request introduces a significant architectural refactoring, reorganizing code from a flat structure to a feature-based modular structure ( Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🧹 Nitpick comments (2)
src/main/java/org/example/cyberwatch/model/entitys/forms/EmploymentForm.java (1)
3-5: Comment block is out of sync with current model naming/states.Line 3 uses
EmployeeForm(class isEmploymentForm), and Line 5 proposes states not present in the current status enum set. Consider aligning/removing this draft note to avoid confusion.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/cyberwatch/model/entitys/forms/EmploymentForm.java` around lines 3 - 5, The review comment references the wrong class and outdated enum states; update or remove it to match the code: either change the reference from EmployeeForm to EmploymentForm and align proposed lifecycle states with the actual status enum (or extend the enum used by EmploymentForm to include DRAFT, SUBMITTED, APPROVED, COMPLETED, REJECTED if you intend that workflow), and ensure any Javadoc or inline comment near the EmploymentForm class and its status enum uses the correct names and current state values so the comment is not misleading.src/main/java/org/example/cyberwatch/model/enums/Status.java (1)
3-3: Clean up draft comments in enum definition.Line 3 and Line 8 look like leftover draft notes; removing them will keep the enum as a clean domain contract.
Also applies to: 8-8
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/org/example/cyberwatch/model/enums/Status.java` at line 3, Remove the leftover draft comment lines inside the Status enum so the enum declaration for Status contains only the actual enum constants (DRAFT, SUBMITTED, IN_PROGRESS, RESOLVED, CLOSED, REOPENED) and any necessary JavaDoc/annotations, i.e., locate the enum named Status and delete the stray comment lines (the draft notes) so the enum is a clean domain contract without draft comments.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@dataStructure.md`:
- Around line 1-34: Update the architecture doc to correct typos and use
consistent terminology: rename "Entitys" to "Entities", "Filemetadata" to "File
metadata" (or "FileMetadata" if using code-style), and "S3-file" to "S3 file"
(or "S3 object/key" if more precise); standardize role names (e.g.,
"HR/Management/Consultant" → "HR, Management, Consultant") and entity names like
"ReportForm"/"EmploymentForm" to consistent casing, improve flow headings and
bullets for clarity, and replace rough terms with clear phrases so the doc reads
cleanly and can be copy-pasted into tickets/onboarding without ambiguous
wording.
In `@pom.xml`:
- Around line 142-146: The Lombok dependency in the pom (the <dependency> block
for org.projectlombok:lombok) uses an invalid scope value "annotationProcessor";
change the <scope> to "provided" and add <optional>true</optional> to the
dependency so Lombok is available at compile time but not transitively included.
In `@src/main/java/org/example/cyberwatch/config/DataInitializer.java`:
- Around line 25-29: The seed data saves roles into the free-form role field but
never sets the enum-backed Staff.department, leaving demo users incomplete;
update the createStaff usages (the calls passed into staffRepository.saveAll and
the similar block around createStaff for other entries) to also set the
Staff.department enum value (or extend the createStaff factory to accept a
Department enum and assign it to Staff.setDepartment) so that department is
populated consistently with the role for each seeded user.
- Around line 10-11: The DataInitializer bean is unscoped and will seed data in
all environments; annotate the DataInitializer class (the class implementing
CommandLineRunner and currently annotated with `@Component`) with a Spring
`@Profile` to exclude production (for example `@Profile`("!prod") or explicitly
`@Profile`({"dev","test"})) so the seeding only runs in non-production profiles;
update the class-level annotations accordingly and ensure tests/dev startup use
the matching profiles.
In `@src/main/java/org/example/cyberwatch/model/entitys/forms/Attachment.java`:
- Around line 33-35: Attachment.reportForm is mandatory but ReportForm lacks the
inverse collection and cascade/orphanRemoval settings; update the ReportForm
class to declare a OneToMany back-reference (mappedBy = "reportForm") named
attachments, initialize it to an empty List, and set cascade = CascadeType.ALL
and orphanRemoval = true so deleting a ReportForm will cascade deletes of
Attachment entities and avoid FK constraint failures; ensure the field name
matches the existing ReportForm reference in Attachment.reportForm.
- Around line 17-31: Attachment entity fields fileName, contentType, fileSize,
s3Key and uploadDate are nullable and must be made mandatory; update the
Attachment class to enforce non-null at both validation and DB level by adding
Bean Validation and column constraints: annotate fileName, contentType and s3Key
with `@NotNull` (and optionally `@Size`(min=1)), annotate fileSize with `@NotNull` and
a positive constraint (e.g. `@Positive`), annotate uploadDate with `@NotNull` (and
optionally `@PastOrPresent`), and update each `@Column` to include nullable = false
so the database schema is constrained; ensure imports for javax/
jakarta.validation annotations are added and that the class name Attachment and
field names (fileName, contentType, fileSize, s3Key, uploadDate) are the ones
updated.
In `@src/main/java/org/example/cyberwatch/model/entitys/staff/Staff.java`:
- Around line 16-17: The socialSecurityNumber field in class Staff is currently
stored as a plain unique column; treat it as high-sensitivity data by encrypting
it at rest and using a derived value for lookups/uniqueness. Replace the raw
String socialSecurityNumber storage with an encrypted column (e.g., use a JPA
AttributeConverter or service to encrypt/decrypt the value for persistence in
the socialSecurityNumber field) and add a separate derived field (e.g.,
socialSecurityNumberHash or socialSecurityNumberLookup) that stores a
deterministic HMAC/hash for uniqueness/lookups (compute via a secure keyed HMAC
using a server-side key). Update any persistence logic, equals/hashCode or query
methods that reference socialSecurityNumber to use the derived lookup field
instead and ensure encryption keys are managed outside the repo.
- Around line 31-34: The Staff class currently has two conflicting
classification fields: a free-form String role and an enumerated Department
department, causing possible nulls or contradictory values (e.g.,
HR/MANAGEMENT/CONSULTANT seeded as roles). Pick one source of truth in Staff:
either remove the String role and rename/use the Department enum (ensure
Department values represent organizational departments) or replace the
Department enum with a Role enum and remove department; update any seeding/DB
code and validation to use only the chosen field and adjust mappings/annotations
in Staff (e.g., `@Enumerated` on the retained enum field) so only one canonical
classification exists.
In `@src/main/java/org/example/cyberwatch/model/enums/Status.java`:
- Around line 5-11: The Status enum (Status) is persisted as strings via
`@Enumerated`(EnumType.STRING) in ReportForm.java; before renaming/removing any
literals, add a migration/backfill plan: introduce a DB migration framework
(e.g., Flyway or Liquibase), create migration scripts that map legacy string
values to new enum names (or add a new column for a transitional value), and/or
implement a JPA AttributeConverter to translate legacy values to current enum
constants so existing rows won’t throw IllegalArgumentException when loaded;
ensure migration scripts and the converter reference the Status enum literal
names used in the current code.
In `@src/main/java/org/example/cyberwatch/repository/FormRepository.java`:
- Around line 3-4: The FormRepository interface is empty and never instantiated
by Spring; either delete this unused interface or make it a proper Spring Data
repository by extending a Spring Data interface (e.g., change FormRepository to
extend JpaRepository<Form, Long> or CrudRepository<Form, ID>) and, if you plan
to add custom methods, annotate/declare it appropriately (add `@Repository` only
if you implement custom behavior beyond extending). Locate the FormRepository
type and either remove the file or update its declaration to extend the chosen
Spring Data interface and ensure the correct domain type and ID type are used.
---
Nitpick comments:
In
`@src/main/java/org/example/cyberwatch/model/entitys/forms/EmploymentForm.java`:
- Around line 3-5: The review comment references the wrong class and outdated
enum states; update or remove it to match the code: either change the reference
from EmployeeForm to EmploymentForm and align proposed lifecycle states with the
actual status enum (or extend the enum used by EmploymentForm to include DRAFT,
SUBMITTED, APPROVED, COMPLETED, REJECTED if you intend that workflow), and
ensure any Javadoc or inline comment near the EmploymentForm class and its
status enum uses the correct names and current state values so the comment is
not misleading.
In `@src/main/java/org/example/cyberwatch/model/enums/Status.java`:
- Line 3: Remove the leftover draft comment lines inside the Status enum so the
enum declaration for Status contains only the actual enum constants (DRAFT,
SUBMITTED, IN_PROGRESS, RESOLVED, CLOSED, REOPENED) and any necessary
JavaDoc/annotations, i.e., locate the enum named Status and delete the stray
comment lines (the draft notes) so the enum is a clean domain contract without
draft comments.
🪄 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
Run ID: 7dce747f-f9dc-44f0-96b5-449154ef4165
📒 Files selected for processing (16)
dataStructure.mdpom.xmlsrc/main/java/org/example/cyberwatch/config/DataInitializer.javasrc/main/java/org/example/cyberwatch/model/entitys/forms/Attachment.javasrc/main/java/org/example/cyberwatch/model/entitys/forms/EmploymentForm.javasrc/main/java/org/example/cyberwatch/model/entitys/staff/Consultant.javasrc/main/java/org/example/cyberwatch/model/entitys/staff/HR.javasrc/main/java/org/example/cyberwatch/model/entitys/staff/Management.javasrc/main/java/org/example/cyberwatch/model/entitys/staff/Staff.javasrc/main/java/org/example/cyberwatch/model/enums/Department.javasrc/main/java/org/example/cyberwatch/model/enums/IssueType.javasrc/main/java/org/example/cyberwatch/model/enums/Priority.javasrc/main/java/org/example/cyberwatch/model/enums/Status.javasrc/main/java/org/example/cyberwatch/repository/FormRepository.javasrc/main/java/org/example/cyberwatch/repository/StaffRepository.javasrc/main/java/org/example/cyberwatch/repository/TicketRepository.java
Comment on lines
+25
to
+29
| staffRepository.saveAll(List.of( | ||
| createStaff("19900101-0101", "Alice", "Andersson", "alice@cyberwatch.local", "070-1111111", "CONSULTANT"), | ||
| createStaff("19880505-0505", "Bob", "Berg", "bob@cyberwatch.local", "070-2222222", "HR"), | ||
| createStaff("19770707-0707", "Carla", "Carlsson", "carla@cyberwatch.local", "070-3333333", "MANAGEMENT") | ||
| )); |
There was a problem hiding this comment.
Seed data leaves department unset.
These rows only write the classification into the free-form role field and never populate Staff.department. Any query or authorization rule using department will see incomplete demo users.
Populate the enum-backed field too
+import org.example.cyberwatch.model.enums.Department;
import org.example.cyberwatch.model.entitys.staff.Staff;
import org.example.cyberwatch.repository.StaffRepository;
...
staffRepository.saveAll(List.of(
- createStaff("19900101-0101", "Alice", "Andersson", "alice@cyberwatch.local", "070-1111111", "CONSULTANT"),
- createStaff("19880505-0505", "Bob", "Berg", "bob@cyberwatch.local", "070-2222222", "HR"),
- createStaff("19770707-0707", "Carla", "Carlsson", "carla@cyberwatch.local", "070-3333333", "MANAGEMENT")
+ createStaff("19900101-0101", "Alice", "Andersson", "alice@cyberwatch.local", "070-1111111", Department.CONSULTANT),
+ createStaff("19880505-0505", "Bob", "Berg", "bob@cyberwatch.local", "070-2222222", Department.HR),
+ createStaff("19770707-0707", "Carla", "Carlsson", "carla@cyberwatch.local", "070-3333333", Department.MANAGEMENT)
));
}
- private Staff createStaff(String ssn, String firstName, String lastName, String email, String phone, String role) {
+ private Staff createStaff(String ssn, String firstName, String lastName, String email, String phone, Department department) {
Staff staff = new Staff();
staff.setSocialSecurityNumber(ssn);
staff.setFirstName(firstName);
staff.setLastName(lastName);
staff.setEmail(email);
staff.setPhoneNumber(phone);
- staff.setRole(role);
+ staff.setRole(department.name());
+ staff.setDepartment(department);
return staff;
}Also applies to: 32-40
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/example/cyberwatch/config/DataInitializer.java` around
lines 25 - 29, The seed data saves roles into the free-form role field but never
sets the enum-backed Staff.department, leaving demo users incomplete; update the
createStaff usages (the calls passed into staffRepository.saveAll and the
similar block around createStaff for other entries) to also set the
Staff.department enum value (or extend the createStaff factory to accept a
Department enum and assign it to Staff.setDepartment) so that department is
populated consistently with the role for each seeded user.
Comment on lines
+17
to
+31
| @Column(name = "file_name") | ||
| String fileName; | ||
|
|
||
| @Column(name = "content_type") | ||
| String contentType; | ||
|
|
||
|
|
||
| @Column(name = "file_size") | ||
| Long fileSize; | ||
|
|
||
| @Column(name = "s3_key") | ||
| String s3Key; | ||
|
|
||
| @Column(name = "upload_date") | ||
| LocalDateTime uploadDate; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd /tmp && find . -type f -name "Attachment.java" 2>/dev/null | head -5Repository: ithsjava25/project-backend-super-cool-team
Length of output: 68
🏁 Script executed:
find . -type f -path "*/entitys/forms/Attachment.java" 2>/dev/nullRepository: ithsjava25/project-backend-super-cool-team
Length of output: 160
🏁 Script executed:
find . -type f -name "Attachment.java" 2>/dev/null | head -20Repository: ithsjava25/project-backend-super-cool-team
Length of output: 160
🏁 Script executed:
cat -n ./src/main/java/org/example/cyberwatch/model/entitys/forms/Attachment.javaRepository: ithsjava25/project-backend-super-cool-team
Length of output: 2680
🏁 Script executed:
cat -n ./src/main/java/org/example/cyberwatch/model/entitys/forms/ReportForm.java | head -50Repository: ithsjava25/project-backend-super-cool-team
Length of output: 1407
🏁 Script executed:
find . -type f -name "*.sql" -o -name "*migration*" 2>/dev/null | head -10Repository: ithsjava25/project-backend-super-cool-team
Length of output: 68
🏁 Script executed:
cat -n ./src/main/java/org/example/cyberwatch/model/entitys/forms/ReportForm.javaRepository: ithsjava25/project-backend-super-cool-team
Length of output: 3108
Make attachment metadata fields non-null at the entity boundary.
The fields fileName, contentType, fileSize, s3Key, and uploadDate currently allow null values, enabling storage of incomplete attachment records that cannot be reliably rendered, downloaded, or cleaned up. Since reportForm is already constrained as mandatory (optional = false, nullable = false), these metadata fields should be equally protected.
Add validation annotations and database constraints:
Hardening
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.NotNull;
+import jakarta.validation.constraints.PositiveOrZero;
...
- `@Column`(name = "file_name")
+ `@NotBlank`
+ `@Column`(name = "file_name", nullable = false)
String fileName;
- `@Column`(name = "content_type")
+ `@NotBlank`
+ `@Column`(name = "content_type", nullable = false)
String contentType;
- `@Column`(name = "file_size")
+ `@NotNull`
+ `@PositiveOrZero`
+ `@Column`(name = "file_size", nullable = false)
Long fileSize;
- `@Column`(name = "s3_key")
+ `@NotBlank`
+ `@Column`(name = "s3_key", nullable = false)
String s3Key;
- `@Column`(name = "upload_date")
+ `@NotNull`
+ `@Column`(name = "upload_date", nullable = false)
LocalDateTime uploadDate;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/example/cyberwatch/model/entitys/forms/Attachment.java`
around lines 17 - 31, Attachment entity fields fileName, contentType, fileSize,
s3Key and uploadDate are nullable and must be made mandatory; update the
Attachment class to enforce non-null at both validation and DB level by adding
Bean Validation and column constraints: annotate fileName, contentType and s3Key
with `@NotNull` (and optionally `@Size`(min=1)), annotate fileSize with `@NotNull` and
a positive constraint (e.g. `@Positive`), annotate uploadDate with `@NotNull` (and
optionally `@PastOrPresent`), and update each `@Column` to include nullable = false
so the database schema is constrained; ensure imports for javax/
jakarta.validation annotations are added and that the class name Attachment and
field names (fileName, contentType, fileSize, s3Key, uploadDate) are the ones
updated.
Comment on lines
+33
to
+35
| @ManyToOne(fetch = FetchType.LAZY, optional = false) | ||
| @JoinColumn(name = "report_form_id", nullable = false) | ||
| ReportForm reportForm; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, find the ReportForm.java file
find . -name "ReportForm.java" -type fRepository: ithsjava25/project-backend-super-cool-team
Length of output: 160
🏁 Script executed:
# Also check the Attachment.java file to confirm the code
find . -name "Attachment.java" -type fRepository: ithsjava25/project-backend-super-cool-team
Length of output: 160
🏁 Script executed:
# Read the Attachment.java file to confirm the relationship definition
cat -n ./src/main/java/org/example/cyberwatch/model/entitys/forms/Attachment.javaRepository: ithsjava25/project-backend-super-cool-team
Length of output: 2680
🏁 Script executed:
# Read the ReportForm.java file to check for inverse relationship
cat -n ./src/main/java/org/example/cyberwatch/model/entitys/forms/ReportForm.javaRepository: ithsjava25/project-backend-super-cool-team
Length of output: 3108
Add inverse relationship with orphan removal in ReportForm.
Attachment.reportForm is mandatory, but ReportForm has no inverse @OneToMany relationship or cascade configuration. Deleting a report with attachments will fail on the foreign key constraint instead of cascading the delete.
Add the following to ReportForm:
`@OneToMany`(mappedBy = "reportForm", cascade = CascadeType.ALL, orphanRemoval = true)
private List<Attachment> attachments = new ArrayList<>();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/example/cyberwatch/model/entitys/forms/Attachment.java`
around lines 33 - 35, Attachment.reportForm is mandatory but ReportForm lacks
the inverse collection and cascade/orphanRemoval settings; update the ReportForm
class to declare a OneToMany back-reference (mappedBy = "reportForm") named
attachments, initialize it to an empty List, and set cascade = CascadeType.ALL
and orphanRemoval = true so deleting a ReportForm will cascade deletes of
Attachment entities and avoid FK constraint failures; ensure the field name
matches the existing ReportForm reference in Attachment.reportForm.
Comment on lines
+16
to
+17
| @Column(name = "social_security_number", nullable = false, unique = true) | ||
| String socialSecurityNumber; |
There was a problem hiding this comment.
Treat socialSecurityNumber as high-sensitivity data.
A plain unique column for full SSNs is a large privacy/compliance surface area. If the full value is genuinely required, store it encrypted and use a derived value for lookups/uniqueness; otherwise keep only the minimum you actually need.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/example/cyberwatch/model/entitys/staff/Staff.java` around
lines 16 - 17, The socialSecurityNumber field in class Staff is currently stored
as a plain unique column; treat it as high-sensitivity data by encrypting it at
rest and using a derived value for lookups/uniqueness. Replace the raw String
socialSecurityNumber storage with an encrypted column (e.g., use a JPA
AttributeConverter or service to encrypt/decrypt the value for persistence in
the socialSecurityNumber field) and add a separate derived field (e.g.,
socialSecurityNumberHash or socialSecurityNumberLookup) that stores a
deterministic HMAC/hash for uniqueness/lookups (compute via a secure keyed HMAC
using a server-side key). Update any persistence logic, equals/hashCode or query
methods that reference socialSecurityNumber to use the derived lookup field
instead and ensure encryption keys are managed outside the repo.
Comment on lines
+31
to
+34
| String role; | ||
|
|
||
| @Enumerated(EnumType.STRING) | ||
| Department department; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Pick one source of truth for staff classification.
role is a free-form String while department is an enum, and the values being seeded (HR, MANAGEMENT, CONSULTANT) read like roles rather than departments. Keeping both makes nulls and contradictory combinations easy to persist.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/org/example/cyberwatch/model/entitys/staff/Staff.java` around
lines 31 - 34, The Staff class currently has two conflicting classification
fields: a free-form String role and an enumerated Department department, causing
possible nulls or contradictory values (e.g., HR/MANAGEMENT/CONSULTANT seeded as
roles). Pick one source of truth in Staff: either remove the String role and
rename/use the Department enum (ensure Department values represent
organizational departments) or replace the Department enum with a Role enum and
remove department; update any seeding/DB code and validation to use only the
chosen field and adjust mappings/annotations in Staff (e.g., `@Enumerated` on the
retained enum field) so only one canonical classification exists.
This was referenced Mar 31, 2026
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
2 participants
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.
Added logic to ENUM classes and Lombok to the pom.xml file!
Summary by CodeRabbit
Release Notes
New Features
Documentation