Hcmpre 2832 attendance revamp backend changes #1928
Conversation
…dance-Revamp-Backend-Changes
WalkthroughThese changes introduce a new "tag" field for attendance attendees, enabling grouping and filtering by tag. The update includes schema migration, model and API enhancements, query and repository updates, enrichment and validation logic, and a new endpoint for updating attendee tags. Support for filtering by tenant ID is also added. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant APIController
participant AttendeeService
participant Validator
participant Repository
participant Enrichment
participant Producer
Client->>APIController: POST /attendee/v1/_updateTag (AttendeeUpdateTagRequest)
APIController->>AttendeeService: updateAttendeeTag(request)
AttendeeService->>Validator: validateAttendeeUpdateTagRequestParameters(request)
AttendeeService->>Repository: getAttendeesByIds(ids, tenantId)
AttendeeService->>Validator: validateAllAttendeesExist(request.attendees, dbRecords)
AttendeeService->>Enrichment: enrichAttendeesForTagUpdate(request, dbRecords)
AttendeeService->>Producer: push updated request to topic
AttendeeService->>APIController: return updated request
APIController->>Client: AttendeeUpdateTagResponse
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 15
🔭 Outside diff range comments (1)
backend/attendance/src/main/resources/attendance-service-persister.yml (1)
359-372: 🧹 Nitpick (assertive)UPDATE statement: keep
additionaldetails&tagordering symmetric with INSERTIn the UPDATE clause
tagis set beforeadditionaldetails, whereas in the INSERT it comes afterlastmodifiedtime.
SQL doesn’t care about column order, but the mismatch increases cognitive load and can trip up anyone doing a blanket copy-paste later.- deenrollment_date=?, tag=? ,additionaldetails=?, + deenrollment_date=?, additionaldetails=?, tag=?,Maintaining a consistent sequence across CRUD statements reduces maintenance errors.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (14)
backend/attendance/src/main/java/org/egov/enrichment/AttendeeEnrichmentService.java(3 hunks)backend/attendance/src/main/java/org/egov/repository/querybuilder/AttendeeQueryBuilder.java(3 hunks)backend/attendance/src/main/java/org/egov/repository/rowmapper/AttendeeRowMapper.java(2 hunks)backend/attendance/src/main/java/org/egov/service/AttendanceRegisterService.java(1 hunks)backend/attendance/src/main/java/org/egov/service/AttendeeService.java(2 hunks)backend/attendance/src/main/java/org/egov/validator/AttendeeServiceValidator.java(3 hunks)backend/attendance/src/main/java/org/egov/web/controllers/AttendeeApiController.java(2 hunks)backend/attendance/src/main/java/org/egov/web/models/AttendanceRegisterSearchCriteria.java(1 hunks)backend/attendance/src/main/java/org/egov/web/models/AttendeeSearchCriteria.java(1 hunks)backend/attendance/src/main/java/org/egov/web/models/AttendeeUpdateTagRequest.java(1 hunks)backend/attendance/src/main/java/org/egov/web/models/AttendeeUpdateTagResponse.java(1 hunks)backend/attendance/src/main/java/org/egov/web/models/IndividualEntry.java(1 hunks)backend/attendance/src/main/resources/attendance-service-persister.yml(4 hunks)backend/attendance/src/main/resources/db/migration/main/V20250605193600__alter_attendance_attendee_table.sql(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
backend/attendance/src/main/java/org/egov/web/models/AttendeeUpdateTagRequest.java (1)
backend/attendance/src/main/java/org/egov/web/models/AttendeeUpdateTagResponse.java (1)
Getter(10-23)
backend/attendance/src/main/java/org/egov/web/models/AttendeeUpdateTagResponse.java (1)
backend/attendance/src/main/java/org/egov/web/models/AttendeeUpdateTagRequest.java (1)
Getter(12-25)
backend/attendance/src/main/java/org/egov/enrichment/AttendeeEnrichmentService.java (1)
backend/attendance/src/main/java/org/egov/service/AttendeeService.java (1)
Service(23-249)
🔇 Additional comments (8)
backend/attendance/src/main/java/org/egov/repository/rowmapper/AttendeeRowMapper.java (2)
39-39: Extracttagsafely and handle missing values
The newrs.getString("tag")call may yieldnull. Confirm downstream logic handles anulltag gracefully or add a default/unwrapped check if needed.
63-63: MaptagintoIndividualEntrybuilder
The.tag(tag)call correctly propagates the new field. Looks fine.backend/attendance/src/main/java/org/egov/web/models/AttendanceRegisterSearchCriteria.java (1)
76-77: New filter fieldtagsadded
This aligns with tag-based filtering. Ensure API documentation clarifies whether multiple tags are combined via AND/OR semantics.backend/attendance/src/main/java/org/egov/service/AttendeeService.java (1)
78-81: Kafka topic name hard-coded via config – verify idempotencyUpdates are pushed straight to the
"updateAttendeeTopic". Tag changes might be sent multiple times if the client retries.
Consider adding a request hash / idempotency key in the message or rely on a database-sideversionfield to avoid double updates.backend/attendance/src/main/resources/attendance-service-persister.yml (1)
94-123: Verify placeholder ↔ JSON-map count & column order for the newtagfieldThe INSERT clause now contains 12 placeholders while the surrounding
jsonMapssection also lists 12 paths. Good.
However, a silent mismatch between the SQL column order and the order ofjsonPaths is a classic persister gotcha that shows up only at runtime.Double-check that:
tagis physically the last column in both the INSERT column list and thejsonMapslist (it is right now – keep it that way).- The migration script created the column as NULLABLE; otherwise existing rows will break on future updates that omit the field.
If both are satisfied, nothing else to do.
backend/attendance/src/main/java/org/egov/enrichment/AttendeeEnrichmentService.java (2)
99-100: Good catch copyingtagduring delete enrichmentPropagating the existing
tagvalue prevents it from being lost during a de-enrolment flow.
57-67: 🧹 Nitpick (assertive)Minor: avoid O(n²) lookup
You already build a
uuidToAttendeeMap; leverage it to skip the linear search inside the for-loop by removing the redundant second loop. No functional bug, just a micro-optimisation for large batches.-for (IndividualEntry attendee : attendeesListFromRequest) { - IndividualEntry dbAttendee = uuidToAttendeeMap.get(attendee.getId()); - ... -} +attendeesListFromRequest.forEach(attendee -> { + IndividualEntry dbAttendee = uuidToAttendeeMap.get(attendee.getId()); + ... +});Likely an incorrect or invalid review comment.
backend/attendance/src/main/java/org/egov/validator/AttendeeServiceValidator.java (1)
105-121:validateAllAttendeesExistis O(n) with Sets – niceEfficient implementation; clear and scalable.
| // This field is used to group individuals into teams. | ||
| @JsonProperty("tag") | ||
| private String tag = null; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Add validation constraint for tag length
Since the DB enforces VARCHAR(64), consider annotating tag with JSR-303 (e.g., @Size(max=64)) or custom validation to prevent request-time oversize values.
🤖 Prompt for AI Agents
In backend/attendance/src/main/java/org/egov/web/models/IndividualEntry.java
around lines 46 to 48, the `tag` field lacks a validation constraint for its
length, which should not exceed 64 characters as per the database VARCHAR(64)
limit. Add a JSR-303 annotation `@Size(max=64)` above the `tag` field
declaration to enforce this constraint at the request validation stage and
prevent oversize values from being processed.
| ALTER TABLE eg_wms_attendance_attendee ADD COLUMN tag character varying(64); | ||
| CREATE INDEX IF NOT EXISTS index_eg_wms_attendance_attendee_tag ON eg_wms_attendance_attendee (tag); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider composite index for multi-tenant queries
Indexing tag alone optimizes tag lookups, but since queries also filter by tenantId, a composite index on (tenantid, tag) could significantly improve performance in a multi-tenant setup.
🤖 Prompt for AI Agents
In
backend/attendance/src/main/resources/db/migration/main/V20250605193600__alter_attendance_attendee_table.sql
at lines 1 to 2, the current index is only on the tag column, but queries also
filter by tenantId. Modify the index to be a composite index on (tenantid, tag)
to optimize multi-tenant query performance. Replace the existing single-column
index with a composite index including tenantid as the first column and tag as
the second.
| @JsonProperty("tags") | ||
| private List<String> tags; | ||
|
|
||
| @JsonProperty("tenantId") | ||
| private String tenantId; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enforce mandatory tenantId and validate tags
New tenantId is critical for scoped searches—add @NotNull or @NotBlank. Also consider @Size on tags elements to match DB constraints.
🤖 Prompt for AI Agents
In
backend/attendance/src/main/java/org/egov/web/models/AttendeeSearchCriteria.java
between lines 37 and 42, add the @NotNull or @NotBlank annotation to the
tenantId field to enforce it as mandatory. Additionally, apply a @Size
annotation on the tags list elements to ensure they conform to database
constraints, such as limiting the length of each tag string. This will enforce
validation rules for both tenantId and tags during input processing.
| @JsonProperty("RequestInfo") | ||
| private RequestInfo requestInfo; | ||
|
|
||
| @JsonProperty("attendees") | ||
| @Valid | ||
| private List<IndividualEntry> attendees; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add mandatory-field validation annotations
requestInfo and the attendees list are both required for the endpoint to work, yet there are no bean-validation constraints ensuring they are present or the list is non-empty. A malformed request will reach the service layer before failing, producing cryptic errors.
@JsonProperty("RequestInfo")
- private RequestInfo requestInfo;
+ @NotNull
+ private RequestInfo requestInfo;
@JsonProperty("attendees")
@Valid
- private List<IndividualEntry> attendees;
+ @NotEmpty
+ private List<IndividualEntry> attendees;📝 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.
| @JsonProperty("RequestInfo") | |
| private RequestInfo requestInfo; | |
| @JsonProperty("attendees") | |
| @Valid | |
| private List<IndividualEntry> attendees; | |
| @JsonProperty("RequestInfo") | |
| @NotNull | |
| private RequestInfo requestInfo; | |
| @JsonProperty("attendees") | |
| @Valid | |
| @NotEmpty | |
| private List<IndividualEntry> attendees; |
🤖 Prompt for AI Agents
In
backend/attendance/src/main/java/org/egov/web/models/AttendeeUpdateTagRequest.java
between lines 19 and 24, the fields requestInfo and attendees lack validation
annotations to enforce their presence and non-emptiness. Add @NotNull to
requestInfo and both @NotNull and @NotEmpty to the attendees list to ensure
these mandatory fields are validated at the API boundary, preventing malformed
requests from proceeding.
| private List<IndividualEntry> fetchAllAttendeesAssociatedToRegisterIds(List<String> registerIdsToSearch, AttendanceRegisterSearchCriteria searchCriteria) { | ||
| AttendeeSearchCriteria attendeeSearchCriteria = null; | ||
| if(searchCriteria.getAttendeeId() != null){ | ||
| attendeeSearchCriteria = AttendeeSearchCriteria.builder().registerIds(registerIdsToSearch).individualIds(Collections.singletonList(searchCriteria.getAttendeeId())).build(); | ||
| attendeeSearchCriteria = AttendeeSearchCriteria.builder().registerIds(registerIdsToSearch).individualIds(Collections.singletonList(searchCriteria.getAttendeeId())).tags(searchCriteria.getTags() != null && !searchCriteria.getTags().isEmpty() ? searchCriteria.getTags() : null) | ||
| .tenantId(searchCriteria.getTenantId()).build(); | ||
| } else { | ||
| attendeeSearchCriteria = AttendeeSearchCriteria.builder().registerIds(registerIdsToSearch).build(); | ||
| attendeeSearchCriteria = AttendeeSearchCriteria.builder().registerIds(registerIdsToSearch).tags(searchCriteria.getTags() != null && !searchCriteria.getTags().isEmpty() ? searchCriteria.getTags() : null) | ||
| .tenantId(searchCriteria.getTenantId()).build(); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Duplicate tag-handling logic & unnecessary ternary
The same tags null-check/assignment is repeated in both branches. The builder can already cope with null, so the ternary is redundant and the duplication noisy.
- if(searchCriteria.getAttendeeId() != null){
- attendeeSearchCriteria = AttendeeSearchCriteria.builder()
- .registerIds(registerIdsToSearch)
- .individualIds(Collections.singletonList(searchCriteria.getAttendeeId()))
- .tags(searchCriteria.getTags() != null && !searchCriteria.getTags().isEmpty() ? searchCriteria.getTags() : null)
- .tenantId(searchCriteria.getTenantId())
- .build();
- } else {
- attendeeSearchCriteria = AttendeeSearchCriteria.builder()
- .registerIds(registerIdsToSearch)
- .tags(searchCriteria.getTags() != null && !searchCriteria.getTags().isEmpty() ? searchCriteria.getTags() : null)
- .tenantId(searchCriteria.getTenantId())
- .build();
- }
+ AttendeeSearchCriteria.AttendeeSearchCriteriaBuilder builder =
+ AttendeeSearchCriteria.builder()
+ .registerIds(registerIdsToSearch)
+ .tags(searchCriteria.getTags())
+ .tenantId(searchCriteria.getTenantId());
+
+ if (searchCriteria.getAttendeeId() != null) {
+ builder.individualIds(Collections.singletonList(searchCriteria.getAttendeeId()));
+ }
+
+ attendeeSearchCriteria = builder.build();🤖 Prompt for AI Agents
In
backend/attendance/src/main/java/org/egov/service/AttendanceRegisterService.java
around lines 351 to 359, the tags null-check and ternary operation are
duplicated in both branches of the if-else. Remove the ternary and the
duplication by assigning the tags field directly from searchCriteria.getTags()
without checking for null or emptiness, since the builder can handle null
values. This simplifies the code and eliminates redundant logic.
| @Getter | ||
| @Setter | ||
| @AllArgsConstructor | ||
| @NoArgsConstructor | ||
| @Builder | ||
| public class AttendeeUpdateTagResponse { | ||
|
|
||
| @JsonProperty("ResponseInfo") | ||
| private ResponseInfo responseInfo; | ||
|
|
||
| @JsonProperty("attendees") | ||
| @Valid | ||
| private List<IndividualEntry> attendees; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider omitting nulls in the API response
The DTO is perfect functionally. Adding Jackson’s @JsonInclude(JsonInclude.Include.NON_NULL) at class level trims empty fields, keeping the wire contract lean.
import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonInclude;
...
@Builder
+@JsonInclude(JsonInclude.Include.NON_NULL)
public class AttendeeUpdateTagResponse {🤖 Prompt for AI Agents
In
backend/attendance/src/main/java/org/egov/web/models/AttendeeUpdateTagResponse.java
around lines 10 to 23, the class lacks the Jackson annotation to omit null
fields in JSON output. Add the annotation
@JsonInclude(JsonInclude.Include.NON_NULL) at the class level to ensure that
null fields are not included in the serialized API response, making the response
payload more concise.
| /** | ||
| * Enriches the attendees for tag update by preserving original fields from the database | ||
| * and updating audit details. | ||
| * | ||
| * @param attendeeUpdateTagRequest The request containing attendees to be updated. | ||
| * @param attendeesFromDB The list of attendees fetched from the database. | ||
| */ | ||
| public void enrichAttendeesForTagUpdate (AttendeeUpdateTagRequest attendeeUpdateTagRequest, List<IndividualEntry> attendeesFromDB) { | ||
|
|
||
| RequestInfo requestInfo = attendeeUpdateTagRequest.getRequestInfo(); | ||
| List<IndividualEntry> attendeesListFromRequest = attendeeUpdateTagRequest.getAttendees(); | ||
|
|
||
| // Create a map for quick lookup of attendees by their UUID | ||
| Map<String, IndividualEntry> uuidToAttendeeMap = attendeesFromDB.stream() | ||
| .collect(Collectors.toMap(IndividualEntry::getId, Function.identity())); | ||
|
|
||
| // Validate that all attendees in the request exist in the database | ||
| for (IndividualEntry attendee : attendeesListFromRequest) { | ||
| IndividualEntry dbAttendee = uuidToAttendeeMap.get(attendee.getId()); | ||
|
|
||
| if (dbAttendee == null) { | ||
| throw new CustomException("INVALID_ID", "Attendee not found: " + attendee.getId()); | ||
| } | ||
|
|
||
| // Preserve original fields from DB | ||
| attendee.setRegisterId(dbAttendee.getRegisterId()); | ||
| attendee.setIndividualId(dbAttendee.getIndividualId()); | ||
| attendee.setTenantId(dbAttendee.getTenantId()); | ||
| attendee.setEnrollmentDate(dbAttendee.getEnrollmentDate()); | ||
| attendee.setDenrollmentDate(dbAttendee.getDenrollmentDate()); | ||
| attendee.setAdditionalDetails(dbAttendee.getAdditionalDetails()); | ||
|
|
||
| // Enrich audit details (lastModifiedBy and lastModifiedTime) | ||
| AuditDetails auditDetails = attendanceServiceUtil.getAuditDetails( | ||
| requestInfo.getUserInfo().getUuid(), | ||
| dbAttendee.getAuditDetails(), | ||
| false // isCreate = false since this is an update | ||
| ); | ||
| // Set the enriched audit details back to the attendee | ||
| attendee.setAuditDetails(auditDetails); | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Null-safety guard around requestInfo
requestInfo.getUserInfo().getUuid() will throw NPE if either requestInfo or userInfo is unexpectedly null. Validators should protect us, yet this service is also callable from tests or future code paths.
A defensive early-exit keeps the service robust:
- RequestInfo requestInfo = attendeeUpdateTagRequest.getRequestInfo();
+RequestInfo requestInfo = attendeeUpdateTagRequest.getRequestInfo();
+if (requestInfo == null || requestInfo.getUserInfo() == null
+ || StringUtils.isBlank(requestInfo.getUserInfo().getUuid())) {
+ throw new CustomException("REQUEST_INFO_MISSING",
+ "RequestInfo or User UUID is absent while enriching attendee tag update");
+}📝 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.
| /** | |
| * Enriches the attendees for tag update by preserving original fields from the database | |
| * and updating audit details. | |
| * | |
| * @param attendeeUpdateTagRequest The request containing attendees to be updated. | |
| * @param attendeesFromDB The list of attendees fetched from the database. | |
| */ | |
| public void enrichAttendeesForTagUpdate (AttendeeUpdateTagRequest attendeeUpdateTagRequest, List<IndividualEntry> attendeesFromDB) { | |
| RequestInfo requestInfo = attendeeUpdateTagRequest.getRequestInfo(); | |
| List<IndividualEntry> attendeesListFromRequest = attendeeUpdateTagRequest.getAttendees(); | |
| // Create a map for quick lookup of attendees by their UUID | |
| Map<String, IndividualEntry> uuidToAttendeeMap = attendeesFromDB.stream() | |
| .collect(Collectors.toMap(IndividualEntry::getId, Function.identity())); | |
| // Validate that all attendees in the request exist in the database | |
| for (IndividualEntry attendee : attendeesListFromRequest) { | |
| IndividualEntry dbAttendee = uuidToAttendeeMap.get(attendee.getId()); | |
| if (dbAttendee == null) { | |
| throw new CustomException("INVALID_ID", "Attendee not found: " + attendee.getId()); | |
| } | |
| // Preserve original fields from DB | |
| attendee.setRegisterId(dbAttendee.getRegisterId()); | |
| attendee.setIndividualId(dbAttendee.getIndividualId()); | |
| attendee.setTenantId(dbAttendee.getTenantId()); | |
| attendee.setEnrollmentDate(dbAttendee.getEnrollmentDate()); | |
| attendee.setDenrollmentDate(dbAttendee.getDenrollmentDate()); | |
| attendee.setAdditionalDetails(dbAttendee.getAdditionalDetails()); | |
| // Enrich audit details (lastModifiedBy and lastModifiedTime) | |
| AuditDetails auditDetails = attendanceServiceUtil.getAuditDetails( | |
| requestInfo.getUserInfo().getUuid(), | |
| dbAttendee.getAuditDetails(), | |
| false // isCreate = false since this is an update | |
| ); | |
| // Set the enriched audit details back to the attendee | |
| attendee.setAuditDetails(auditDetails); | |
| } | |
| } | |
| /** | |
| * Enriches the attendees for tag update by preserving original fields from the database | |
| * and updating audit details. | |
| * | |
| * @param attendeeUpdateTagRequest The request containing attendees to be updated. | |
| * @param attendeesFromDB The list of attendees fetched from the database. | |
| */ | |
| public void enrichAttendeesForTagUpdate(AttendeeUpdateTagRequest attendeeUpdateTagRequest, List<IndividualEntry> attendeesFromDB) { | |
| RequestInfo requestInfo = attendeeUpdateTagRequest.getRequestInfo(); | |
| if (requestInfo == null || requestInfo.getUserInfo() == null | |
| || StringUtils.isBlank(requestInfo.getUserInfo().getUuid())) { | |
| throw new CustomException("REQUEST_INFO_MISSING", | |
| "RequestInfo or User UUID is absent while enriching attendee tag update"); | |
| } | |
| List<IndividualEntry> attendeesListFromRequest = attendeeUpdateTagRequest.getAttendees(); | |
| // Create a map for quick lookup of attendees by their UUID | |
| Map<String, IndividualEntry> uuidToAttendeeMap = attendeesFromDB.stream() | |
| .collect(Collectors.toMap(IndividualEntry::getId, Function.identity())); | |
| // Validate that all attendees in the request exist in the database | |
| for (IndividualEntry attendee : attendeesListFromRequest) { | |
| IndividualEntry dbAttendee = uuidToAttendeeMap.get(attendee.getId()); | |
| if (dbAttendee == null) { | |
| throw new CustomException("INVALID_ID", "Attendee not found: " + attendee.getId()); | |
| } | |
| // Preserve original fields from DB | |
| attendee.setRegisterId(dbAttendee.getRegisterId()); | |
| attendee.setIndividualId(dbAttendee.getIndividualId()); | |
| attendee.setTenantId(dbAttendee.getTenantId()); | |
| attendee.setEnrollmentDate(dbAttendee.getEnrollmentDate()); | |
| attendee.setDenrollmentDate(dbAttendee.getDenrollmentDate()); | |
| attendee.setAdditionalDetails(dbAttendee.getAdditionalDetails()); | |
| // Enrich audit details (lastModifiedBy and lastModifiedTime) | |
| AuditDetails auditDetails = attendanceServiceUtil.getAuditDetails( | |
| requestInfo.getUserInfo().getUuid(), | |
| dbAttendee.getAuditDetails(), | |
| false // isCreate = false since this is an update | |
| ); | |
| attendee.setAuditDetails(auditDetails); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
backend/attendance/src/main/java/org/egov/enrichment/AttendeeEnrichmentService.java
between lines 45 and 86, add a null-safety check for requestInfo and its
userInfo before accessing getUuid(). If either requestInfo or userInfo is null,
exit the method early or handle the case gracefully to avoid
NullPointerException. This ensures the method remains robust even if called from
tests or unexpected code paths.
| /** | ||
| * Validates the MDMS data and request info for updating attendee tags. | ||
| * | ||
| * @param attendeeUpdateTagRequest The request object containing the list of attendees to be updated. | ||
| */ | ||
| public void validateMDMSAndRequestInfoForUpdateTagAttendee(AttendeeUpdateTagRequest attendeeUpdateTagRequest) { | ||
|
|
||
| RequestInfo requestInfo = attendeeUpdateTagRequest.getRequestInfo(); | ||
| List<IndividualEntry> attendeeListFromRequest = attendeeUpdateTagRequest.getAttendees(); | ||
| Map<String, String> errorMap = new HashMap<>(); | ||
| String tenantId = attendeeListFromRequest.get(0).getTenantId(); | ||
| Object mdmsData = mdmsUtils.mDMSCall(requestInfo, tenantId); | ||
| //check tenant Id | ||
| log.info("validate tenantId with MDMS"); | ||
| validateMDMSData(tenantId, mdmsData, errorMap); | ||
| //validate request-info | ||
| log.info("validate request info coming from api request"); | ||
| validateRequestInfo(requestInfo, errorMap); | ||
| if (!errorMap.isEmpty()) | ||
| throw new CustomException(errorMap); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Potential race between MDMS lookup and RequestInfo validation
You hit MDMS before verifying requestInfo/userInfo non-null, whereas the create/delete flows validate first. Aligning the order avoids an avoidable NPE on transient null RequestInfos.
🤖 Prompt for AI Agents
In
backend/attendance/src/main/java/org/egov/validator/AttendeeServiceValidator.java
around lines 363 to 383, the method calls mdmsUtils.mDMSCall before validating
that requestInfo and its userInfo are non-null, which can cause a
NullPointerException. To fix this, first validate the requestInfo object for
null or invalid values by calling validateRequestInfo before making the MDMS
call. This aligns the validation order with other flows and prevents potential
NPEs.
| /** | ||
| * Validates the tenant IDs in the AttendeeUpdateTagRequest object. | ||
| * | ||
| * @param attendeeUpdateTagRequest The request object containing the list of attendees to be updated. | ||
| * @param tenantId The tenant ID to validate against. | ||
| */ | ||
| public void validateTenantIds(AttendeeUpdateTagRequest attendeeUpdateTagRequest, String tenantId) { | ||
| List<IndividualEntry> attendeeList = attendeeUpdateTagRequest.getAttendees(); | ||
| //validate if all attendee in the list have the same tenant id | ||
| for (IndividualEntry attendee : attendeeList) { | ||
| if (!attendee.getTenantId().equals(tenantId)) { | ||
| log.error("All attendees dont have the same tenant id in attendee request"); | ||
| throw new CustomException("TENANT_ID", "All Attendees to be enrolled or de enrolled must have the same tenant id. Please raise new request for different tenant id"); | ||
| } | ||
| } | ||
|
|
||
| } |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
DRY: three overloaded validateTenantIds have identical bodies
The 3 overloads (create, delete, update-tag) differ only in parameter type yet duplicate code. Extract a private generic helper accepting List<IndividualEntry> to reduce repetition.
🤖 Prompt for AI Agents
In
backend/attendance/src/main/java/org/egov/validator/AttendeeServiceValidator.java
around lines 193 to 209, there are three overloaded validateTenantIds methods
with identical logic but different parameter types. To fix this, create a
private helper method that accepts a List<IndividualEntry> and contains the
tenant ID validation logic. Then refactor the three public validateTenantIds
methods to extract the attendee list from their respective parameters and call
this new helper method, eliminating code duplication.
| public void validateAttendeeUpdateTagRequestParameters(AttendeeUpdateTagRequest attendeeUpdateTagRequest) { | ||
| List<IndividualEntry> attendeeList = attendeeUpdateTagRequest.getAttendees(); | ||
| Map<String, String> errorMap = new HashMap<>(); | ||
| // Check if the attendee list is empty or null | ||
| if (attendeeList == null || attendeeList.isEmpty()) { | ||
| log.error("ATTENDEE Object is empty in attendee request"); | ||
| throw new CustomException("ATTENDEE", "ATTENDEE Object is empty in attendee request"); | ||
| } | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consolidate repeated error-map population
The same string literals (ID, TENANT_ID, TAG) are hard-coded in multiple blocks. Extracting them into constants (or re-using those in AttendanceServiceConstants) minimises typo risk and eases i18n.
🤖 Prompt for AI Agents
In
backend/attendance/src/main/java/org/egov/validator/AttendeeServiceValidator.java
around lines 58 to 66, the string literals "ID", "TENANT_ID", and "TAG" are
hard-coded multiple times when populating the errorMap. To fix this, replace
these repeated string literals with constants defined in
AttendanceServiceConstants or create new constants if they don't exist. This
will reduce the risk of typos and make future internationalization easier.
|
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
backend/attendance/src/main/java/org/egov/service/AttendanceRegisterService.java (1)
390-400: 🧹 Nitpick (assertive)Repeated ternary for tags – same duplication noted earlier
The builder handles
null, so the? : nullternary (lines 393 & 399) is unnecessary noise that was already flagged in a previous review.- .tags((fallbackTags != null && !fallbackTags.isEmpty()) ? fallbackTags : null) + .tags(fallbackTags)Applies to both branches. Removes duplication and makes the intent clearer.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
backend/attendance/src/main/java/org/egov/service/AttendanceRegisterService.java(1 hunks)backend/attendance/src/main/java/org/egov/web/models/AttendanceRegisterSearchCriteria.java(1 hunks)
| // If includeTaggedAttendees is true and attendeeId is provided, try resolving tags | ||
| if (attendeeId != null && includeTagged) { | ||
| List<IndividualEntry> baseAttendee = attendeeRepository.getAttendees( | ||
| AttendeeSearchCriteria.builder() | ||
| .individualIds(Collections.singletonList(attendeeId)) | ||
| .registerIds(registerIdsToSearch) | ||
| .tenantId(tenantId) | ||
| .build() | ||
| ); | ||
|
|
||
| resolvedTags = baseAttendee.stream() | ||
| .map(IndividualEntry::getTag) | ||
| .filter(Objects::nonNull) | ||
| .distinct() | ||
| .collect(Collectors.toList()); | ||
|
|
||
| if (resolvedTags.isEmpty()) { | ||
| log.warn("includeTaggedAttendees is true but no tags found for attendeeId {}", attendeeId); | ||
| } | ||
| } |
There was a problem hiding this comment.
Null-safety gap when getAttendees() returns null
attendeeRepository.getAttendees(...) is assumed to return a non-null list, but the contract is not enforced here.
If the repository implementation ever returns null, the subsequent stream() call will explode with an NPE, taking the whole request down.
- List<IndividualEntry> baseAttendee = attendeeRepository.getAttendees(
+ List<IndividualEntry> baseAttendee = Optional
+ .ofNullable(attendeeRepository.getAttendees(
AttendeeSearchCriteria.builder()
.individualIds(Collections.singletonList(attendeeId))
.registerIds(registerIdsToSearch)
.tenantId(tenantId)
.build()
- );
+ )).orElse(Collections.emptyList());Fail fast or default to an empty list – either way the service stays resilient.
📝 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.
| // If includeTaggedAttendees is true and attendeeId is provided, try resolving tags | |
| if (attendeeId != null && includeTagged) { | |
| List<IndividualEntry> baseAttendee = attendeeRepository.getAttendees( | |
| AttendeeSearchCriteria.builder() | |
| .individualIds(Collections.singletonList(attendeeId)) | |
| .registerIds(registerIdsToSearch) | |
| .tenantId(tenantId) | |
| .build() | |
| ); | |
| resolvedTags = baseAttendee.stream() | |
| .map(IndividualEntry::getTag) | |
| .filter(Objects::nonNull) | |
| .distinct() | |
| .collect(Collectors.toList()); | |
| if (resolvedTags.isEmpty()) { | |
| log.warn("includeTaggedAttendees is true but no tags found for attendeeId {}", attendeeId); | |
| } | |
| } | |
| // If includeTaggedAttendees is true and attendeeId is provided, try resolving tags | |
| if (attendeeId != null && includeTagged) { | |
| List<IndividualEntry> baseAttendee = Optional | |
| .ofNullable(attendeeRepository.getAttendees( | |
| AttendeeSearchCriteria.builder() | |
| .individualIds(Collections.singletonList(attendeeId)) | |
| .registerIds(registerIdsToSearch) | |
| .tenantId(tenantId) | |
| .build() | |
| )) | |
| .orElse(Collections.emptyList()); | |
| resolvedTags = baseAttendee.stream() | |
| .map(IndividualEntry::getTag) | |
| .filter(Objects::nonNull) | |
| .distinct() | |
| .collect(Collectors.toList()); | |
| if (resolvedTags.isEmpty()) { | |
| log.warn("includeTaggedAttendees is true but no tags found for attendeeId {}", attendeeId); | |
| } | |
| } |
🤖 Prompt for AI Agents
In
backend/attendance/src/main/java/org/egov/service/AttendanceRegisterService.java
around lines 361 to 380, the call to attendeeRepository.getAttendees(...) may
return null, causing a NullPointerException when calling stream() on the result.
To fix this, add a null check after retrieving baseAttendee and either fail fast
with a clear exception or default baseAttendee to an empty list before
streaming. This ensures the service remains resilient and avoids runtime
crashes.
| @JsonProperty("tags") | ||
| private List<String> tags; | ||
|
|
||
| @JsonProperty("includeTaggedAttendees") | ||
| private Boolean includeTaggedAttendees; | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Consider sensible defaults & doc for the new filter flags
tags and includeTaggedAttendees were added without Javadoc or default handling:
includeTaggedAttendeesbeingnullleaves callers to guess the default behaviour.
– Defaulting toBoolean.FALSEin the builder (or via Lombok@Builder.Default) makes the contract explicit.- Adding a short comment/Javadoc clarifies how
tagsand the flag interact.
- private Boolean includeTaggedAttendees;
+ @Builder.Default
+ private Boolean includeTaggedAttendees = Boolean.FALSE; // default: only the requested attendeeNot critical but improves API ergonomics.
📝 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.
| @JsonProperty("tags") | |
| private List<String> tags; | |
| @JsonProperty("includeTaggedAttendees") | |
| private Boolean includeTaggedAttendees; | |
| @JsonProperty("tags") | |
| private List<String> tags; | |
| @JsonProperty("includeTaggedAttendees") | |
| @Builder.Default | |
| private Boolean includeTaggedAttendees = Boolean.FALSE; // default: only the requested attendee |
🤖 Prompt for AI Agents
In
backend/attendance/src/main/java/org/egov/web/models/AttendanceRegisterSearchCriteria.java
around lines 76 to 81, add Javadoc comments to both the 'tags' and
'includeTaggedAttendees' fields explaining their purpose and interaction. Also,
set a default value of Boolean.FALSE for 'includeTaggedAttendees' either by
initializing it directly or using Lombok's @Builder.Default annotation to make
the default behavior explicit and improve API clarity.
Summary by CodeRabbit