-
Notifications
You must be signed in to change notification settings - Fork 1
Code improvements using Java records 1 #183
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
Conversation
|
CI: Security warnings found! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR converts CategoryTemplate and ExternalNotificationEmailModel from traditional Java classes to Java records, simplifying the codebase by leveraging record's automatic generation of constructors, getters, equals, hashCode, and toString methods.
Key changes:
- Converted
CategoryTemplateandExternalNotificationEmailModelto records with compact constructors for validation - Updated all references from getter methods (e.g.,
getSubmission()) to record accessor methods (e.g.,submission()) - Added comprehensive javadoc documentation to mapper and service classes
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| CategoryTemplate.java | Converted to record with compact constructor ensuring guidanceTexts is never null |
| ExternalNotificationEmailModel.java | Converted to record with documentation |
| CategoryTemplateMapper.java | Updated method calls to use record accessors and added javadoc |
| CategoryTemplateServiceImpl.java | Updated to use record accessor methods and marked fields as final |
| EmailServiceImpl.java | Updated to use record accessor methods and added comprehensive javadoc |
| ExternalNotificationEmailMapper.java | Updated to use record accessors, marked fields as final, and added javadoc |
| CategoryTemplateMapperTest.java | Simplified test helper methods to use single-line record instantiation |
| CategoryTemplateTest.java | Replaced EqualsVerifier test with manual record behavior verification |
| CategoryTemplateServiceImplTest.java | Updated mock expectations to use record accessor methods |
| EmailServiceImplTest.java | Updated mock expectations and verifications to use record accessor methods |
| ExternalNotificationEmailMapperTest.java | Updated mock expectations to use record accessor methods |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/uk/gov/companieshouse/efs/api/categorytemplates/model/CategoryTemplate.java
Show resolved
Hide resolved
.../java/uk/gov/companieshouse/efs/api/categorytemplates/mapper/CategoryTemplateMapperTest.java
Show resolved
Hide resolved
|
CI: Security warnings found! |
1 similar comment
|
CI: Security warnings found! |
|
CI: Security warnings found! |
hepsimo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good. Just one small improvement comment
src/main/java/uk/gov/companieshouse/efs/api/categorytemplates/model/CategoryTemplate.java
Outdated
Show resolved
Hide resolved
|
CI: Security warnings found! |
|
|
CI: Security warnings found! |
hepsimo
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look good




Jira ticket: ASM-881
Brief description of the change(s)
Mini PR to cover changes to:
CategoryTemplate and related classes
ExternalNotificationEmailModel and related classes
Working example
Successful EFS submission in local environment
Test notes
Regression test
Checklist
- [ ] Added/updated logging appropriately.- [ ] Updated Docker/ECS configs.- [ ] Added all new properties to chs-configs.`Strikethrough (
~~like this~~) anything not applicable to your changes.