samchinmaya opened a new pull request, #6621: URL: https://github.com/apache/fineract/pull/6621
## Description JIRA: [FINERACT-2911](https://issues.apache.org/jira/browse/FINERACT-2911) Since the meeting commands moved to the new command processing (FINERACT-2548), recording attendance for a **center** meeting always fails with `403 Client with identifier null is not in center {id}`, even for clients who are in the center. A missing `clientsAttendance` or an attendance entry without `clientId` also gives a 500 instead of a validation error. Cause: - `MeetingWriteServiceImpl.getClientsAttendance()` checks center membership with `group.isChildClient(attendance.getId())`. `attendance.getId()` is the id of the attendance record, which is always `null` when creating a meeting. The legacy `MeetingWritePlatformServiceJpaRepositoryImpl` used the client id here. The group check's exception and the center error message also used `getId()`, so both errors said "identifier null". - `getClientsAttendance()` loops over the list without a null check. - `MeetingsApiResource` has no `@Valid` on the request parameters, so the existing `@NotNull` constraints on `MeetingCreateRequest` and `MeetingAttendanceData` are never checked. Even a request without `calendarId` gives a 500. Change: - `getClientsAttendance()` uses `attendance.getClientId()` for the group and center checks and their error messages, and treats a missing list as no attendance. The duplicated `isCenter` check is removed. - `@Valid` on the `createMeeting` and `updateMeetingAttendance` request parameters (same pattern as `NotesApiResource` / `StaffApiResource`), and on the attendance lists in `MeetingCreateRequest` / `MeetingAttendanceUpdateRequest`, so entries are validated too. The existing validation messages are used. I didn't add it to `updateMeeting`, because `MeetingUpdateRequest` requires `calendarId` and that would change what PUT accepts. - New `MeetingWriteServiceImplTest`: center member accepted, non-member rejected with the real client id (center and group), missing list gives no attendance. Verification: - `MeetingWriteServiceImplTest` passes (4/4), and all 4 fail with the main-code change reverted. - Real API, same requests against `develop` and against a build of this branch (fresh database): | Request | develop | this branch | |---|---|---| | Center meeting, client in the center | 403 "identifier null" | 200, attendance saved | | Center meeting, client not in the center | 403 "identifier null" | 403 "Client with identifier 2 is not in center 1" | | Group meeting, client not in the group | 403 "identifier null" | 403 "Client with identifier 1 is not in Group with identifier 3" | | No `clientsAttendance` | 500 | 200 | | Attendance entry without `clientId` | 500 | 400 "The parameter 'clientId' is mandatory" | | No `calendarId` | 500 | 400 "The parameter 'calendarId' is mandatory" | | Valid group meeting (control) | 200 | 200 | Not changed here: `attendanceType` must now be an object (`{"id": 1}`) instead of a number, as asked in the ticket; that's waiting for an answer. Updating attendance (`MeetingAttendanceWriteServiceImpl`) has separate problems, reported in [FINERACT-2917](https://issues.apache.org/jira/browse/FINERACT-2917): it loads the meeting by the group/center id instead of the meeting id, and new attendees aren't saved. With this fix, center attendance updates no longer stop at the membership check, so they reach that bug too. I'll fix FINERACT-2917 in a follow-up PR. ## Checklist Please make sure these boxes are checked before submitting your pull request - thanks! - [x] Write the commit message as per [our guidelines](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#pull-requests) - [x] Acknowledge that we will not review PRs that are not passing the build _("green")_ - it is your responsibility to get a proposed PR to pass the build, not primarily the project's maintainers. - [x] Create/update [unit or integration tests](https://fineract.apache.org/docs/current/#_testing) for verifying the changes made. - [x] Follow our [coding conventions](https://cwiki.apache.org/confluence/display/FINERACT/Coding+Conventions). - [ ] Add required Swagger annotation and update API documentation at fineract-provider/src/main/resources/static/legacy-docs/apiLive.htm with details of any API changes - [x] [This PR must not be a "code dump"](https://cwiki.apache.org/confluence/display/FINERACT/Pull+Request+Size+Limit). Large changes can be made in a branch, with assistance. Ask for help on the [developer mailing list](https://fineract.apache.org/#contribute). - [x] If merging this PR resolves a JIRA issue, I will mark that issue as resolved and set "Fix Version/s" appropriately. - [x] I followed the [AI Policy](https://github.com/apache/fineract/blob/develop/CONTRIBUTING.md#ai-policy). Your assigned reviewer(s) will follow our [guidelines for code reviews](https://cwiki.apache.org/confluence/display/FINERACT/Code+Review+Guide). -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
