Skip to content

MX-263: add admin CRUD endpoints for office services and geolocation#77

Merged
IOhacker merged 1 commit into
openMF:developfrom
DeathGun44:MX-263-office-admin-crud-endpoints
May 28, 2026
Merged

MX-263: add admin CRUD endpoints for office services and geolocation#77
IOhacker merged 1 commit into
openMF:developfrom
DeathGun44:MX-263-office-admin-crud-endpoints

Conversation

@DeathGun44
Copy link
Copy Markdown
Contributor

@DeathGun44 DeathGun44 commented May 27, 2026

Summary by CodeRabbit

  • New Features

    • v2 REST endpoints to create, read, update, and delete office services and office geolocation (with permission checks and validation).
  • Data

    • New immutable payloads for office service and office geolocation (including latitude/longitude).
  • Tests

    • Comprehensive unit tests covering service and geolocation flows, validations, and error cases.
  • Chores

    • CI workflow updated to tolerate missing integration coverage artifacts.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

📝 Walkthrough

Walkthrough

Adds office services and geolocation: DTOs, read/write service interfaces and JDBC implementations (with validation/upsert), a /v2/offices JAX-RS resource for CRUD, unit tests, and a CI workflow tolerance for missing integration coverage artifacts.

Changes

Office Extension Services and Geolocation

Layer / File(s) Summary
Data Transfer Objects
src/main/java/org/apache/fineract/savings/office/data/OfficeServiceData.java, src/main/java/org/apache/fineract/savings/office/data/OfficeGeolocationData.java
Immutable DTOs with factory methods and getters for office services (id, officeId, serviceName, serviceExternalId, workingHours) and geolocation (id, officeId, latitude, longitude).
Read Platform Service
src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformService.java, src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformServiceImpl.java
Read-only service interface and JDBC implementation providing retrieval of office services and office geolocation with authenticated access, parameterized queries, null-safe single-row handling, and RowMappers.
Write Platform Service
src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformService.java, src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImpl.java
Write interface and transactional JDBC implementation for create/update/delete of services and save/delete geolocation. Implements JSON parsing, selective UPDATE construction, office existence checks, coordinate presence/range validation, geolocation upsert, and returns CommandProcessingResult.
REST API Resource
src/main/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResource.java
JAX-RS resource under /v2/offices exposing endpoints for office services and geolocation (GET/POST/PUT/DELETE), enforcing permissions, delegating to platform services, and serializing responses via DefaultToApiJsonSerializer.
REST API Tests
src/test/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResourceTest.java
JUnit 5 + Mockito tests that mock security context, services, and serializers to verify API resource correctly delegates to read/write services and returns serialized results for services and geolocation endpoints.
Write Service Tests
src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java
JUnit 5 + Mockito tests covering create (including non-existent office -> OfficeNotFoundException), update (selective field updates, zero-rows -> PlatformDataIntegrityException, skip empty updates), delete (zero-rows -> PlatformDataIntegrityException), geolocation save (validation errors and success returning resource id), and delete (verifies DELETE SQL).

CI Coverage Artifact

Layer / File(s) Summary
Maven CI workflow: tolerate missing integration coverage artifact
.github/workflows/maven-build.yml
Upload step now uses if-no-files-found: ignore for JaCoCo integration artifact and the coverage gate download step uses continue-on-error: true.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

⏱️ 30-60 Min Review

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 53.97% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main changes: adding admin CRUD endpoints for office services and geolocation management.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🧹 Nitpick comments (4)
src/main/java/org/apache/fineract/savings/office/data/OfficeGeolocationData.java (1)

49-63: ⚡ Quick win

Document the public getter methods with Javadoc.

These public API methods are currently undocumented.

As per coding guidelines "Public methods and classes MUST have Javadoc."

🤖 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
`@src/main/java/org/apache/fineract/savings/office/data/OfficeGeolocationData.java`
around lines 49 - 63, Add Javadoc comments to the public getter methods in
OfficeGeolocationData: getId, getOfficeId, getLatitude, and getLongitude; for
each method include a one-line description of what the getter returns and a
`@return` tag describing the returned value (e.g., "the geolocation record id",
"the associated office id", "the latitude value", "the longitude value") and
indicate nullability if applicable. Ensure the Javadoc is placed immediately
above each method declaration and follows the project's Javadoc style.
src/main/java/org/apache/fineract/savings/office/data/OfficeServiceData.java (1)

58-75: ⚡ Quick win

Add Javadoc for the public getter methods.

The public getters are missing Javadoc, which breaks the repository documentation standard for Java public APIs.

As per coding guidelines "Public methods and classes MUST have Javadoc."

🤖 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 `@src/main/java/org/apache/fineract/savings/office/data/OfficeServiceData.java`
around lines 58 - 75, Add Javadoc comments to each public getter in
OfficeServiceData (getId, getOfficeId, getServiceName, getServiceExternalId,
getWorkingHours): for each method add a brief one-line description of what the
getter returns and a `@return` tag describing the returned value (including
nullability if applicable), ensuring the style matches other classes' Javadoc in
the project.
src/test/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResourceTest.java (1)

58-160: ⚡ Quick win

Strengthen test assertions and add edge-case coverage.

Most tests only assert non-null output. Assert exact serialized payloads and verify serializer invocation, then add edge cases (e.g., not-found responses and invalid payloads) to catch contract regressions.

As per coding guidelines "Verify both happy path and edge cases."

🤖 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
`@src/test/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResourceTest.java`
around lines 58 - 160, Current tests only assert non-null results; update each
test (retrieveOfficeServices, createOfficeService, updateOfficeService,
deleteOfficeService, retrieveOfficeGeolocation, saveOfficeGeolocation,
deleteOfficeGeolocation) to assert the exact serialized payload (use
assertEquals with the stubbed serviceSerializer/geolocationSerializer return
values) and add verify(...) calls that the appropriate serializer
(serviceSerializer or geolocationSerializer) was invoked with the expected data;
additionally add new unit tests for edge cases: service/read methods returning
null or throwing a NotFound/EntityNotFound scenario and write methods receiving
invalid JSON (expecting validation errors or command processing failures) to
ensure the resource handles these paths and that writeService/readService
interactions are verified.
src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java (1)

9-11: ⚡ Quick win

Use AssertJ assertions to match test standards.

Replace JUnit assertion style with AssertJ for consistency with project test conventions.

As per coding guidelines, src/test/java/**: Use JUnit 5, Mockito, and AssertJ.

🤖 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
`@src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java`
around lines 9 - 11, The tests in OfficeExtensionWritePlatformServiceImplTest
currently use JUnit assertions (assertEquals, assertNotNull, assertThrows);
update them to use AssertJ style for consistency: replace assertEquals(expected,
actual) with assertThat(actual).isEqualTo(expected), assertNotNull(obj) with
assertThat(obj).isNotNull(), and assertThrows(…, () -> ...) with
assertThatThrownBy(() ->
...).isInstanceOf(TheException.class).withMessageContaining(...) as appropriate;
update all occurrences in the test class and import static
org.assertj.core.api.Assertions.assertThat and assertThatThrownBy instead of
JUnit assertion statics.
🤖 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
`@src/main/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResource.java`:
- Around line 130-137: The controller currently ignores the route's officeId for
mutations: updateOfficeService (and likewise deleteOfficeService) only passes
serviceId and jsonBody to writeService, allowing cross-office updates; change
the controller methods to pass the path officeId into the write layer (call
writeService.updateOfficeService(officeId, serviceId, jsonBody) and
writeService.deleteOfficeService(officeId, serviceId)), then update the
corresponding write-layer methods to accept the officeId parameter and enforce
the DB update/delete using WHERE id = ? AND office_id = ? (returning/throwing a
not-found/forbidden error if no rows are affected) so operations are scoped to
the given office.

In
`@src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformServiceImpl.java`:
- Around line 53-63: The method retrieveOfficeService in
OfficeExtensionReadPlatformServiceImpl currently returns null when no row is
found; change its contract to return Optional<OfficeServiceData> instead and
update its implementation to catch EmptyResultDataAccessException and return
Optional.empty(), otherwise return Optional.of(result) (use the existing
OfficeServiceRowMapper to map the row). Update the method signature in the
interface and all callers to handle Optional<OfficeServiceData> so no nulls are
propagated.

In
`@src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImpl.java`:
- Around line 147-156: The latitude and longitude variables in
OfficeExtensionWritePlatformServiceImpl are used directly without null/range
validation before being added to MapSqlParameterSource; add validation right
after extracting latitude and longitude (before params.addValue) to reject nulls
and enforce -90<=latitude<=90 and -180<=longitude<=180, and throw the
appropriate validation exception (e.g., PlatformApiDataValidationException or
using DataValidatorBuilder) with clear messages when values are missing or out
of range so invalid coordinates are not persisted.
- Around line 126-138: In OfficeExtensionWritePlatformServiceImpl, verify the
JDBC update counts for both the dynamic update (the
namedJdbcTemplate.update(sql.toString(), params) call used when applying
changes) and the deleteOfficeService method (the jdbcTemplate.update("DELETE
FROM m_selfservice_office_service WHERE id = ?", serviceId) call); capture the
returned int affectedRows, and if it is 0 throw a domain-specific not-found
exception (e.g., OfficeNotFoundException or a ResourceNotFoundException)
including the serviceId so callers receive a proper 404-like error instead of a
silent success.
- Around line 158-174: Replace the separate existence check and conditional
updates in OfficeExtensionWritePlatformServiceImpl with a single atomic DB
upsert using a DB-native upsert statement via namedJdbcTemplate.update (e.g.,
for Postgres use "INSERT INTO m_selfservice_office_geolocation (office_id,
latitude, longitude) VALUES (:officeId, :latitude, :longitude) ON CONFLICT
(office_id) DO UPDATE SET latitude = EXCLUDED.latitude, longitude =
EXCLUDED.longitude" or for MySQL use "INSERT ... ON DUPLICATE KEY UPDATE ..."),
pass the existing params and officeId, and ensure there is a unique constraint
on office_id so the upsert is reliable and eliminates the race window created by
jdbcTemplate.queryForObject -> conditional update/insert.

In
`@src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java`:
- Around line 141-154: The test updateOfficeService_noChanges_skipsUpdate
currently only asserts a non-null result but doesn't verify that no DB update
was executed; add a negative interaction assertion after calling
service.updateOfficeService(SERVICE_ID, json) to ensure the component that
performs SQL updates (e.g., the mocked jdbcTemplate or repository used by
OfficeExtensionWritePlatformServiceImpl) was not invoked—for example use
verify(jdbcTemplate, never()).update(anyString(), any()) or
verifyNoInteractions(officeRepository) / verify(officeRepository,
never()).save(...) depending on which mock is wired into service, so the test
protects the "skip update" contract.

---

Nitpick comments:
In
`@src/main/java/org/apache/fineract/savings/office/data/OfficeGeolocationData.java`:
- Around line 49-63: Add Javadoc comments to the public getter methods in
OfficeGeolocationData: getId, getOfficeId, getLatitude, and getLongitude; for
each method include a one-line description of what the getter returns and a
`@return` tag describing the returned value (e.g., "the geolocation record id",
"the associated office id", "the latitude value", "the longitude value") and
indicate nullability if applicable. Ensure the Javadoc is placed immediately
above each method declaration and follows the project's Javadoc style.

In
`@src/main/java/org/apache/fineract/savings/office/data/OfficeServiceData.java`:
- Around line 58-75: Add Javadoc comments to each public getter in
OfficeServiceData (getId, getOfficeId, getServiceName, getServiceExternalId,
getWorkingHours): for each method add a brief one-line description of what the
getter returns and a `@return` tag describing the returned value (including
nullability if applicable), ensuring the style matches other classes' Javadoc in
the project.

In
`@src/test/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResourceTest.java`:
- Around line 58-160: Current tests only assert non-null results; update each
test (retrieveOfficeServices, createOfficeService, updateOfficeService,
deleteOfficeService, retrieveOfficeGeolocation, saveOfficeGeolocation,
deleteOfficeGeolocation) to assert the exact serialized payload (use
assertEquals with the stubbed serviceSerializer/geolocationSerializer return
values) and add verify(...) calls that the appropriate serializer
(serviceSerializer or geolocationSerializer) was invoked with the expected data;
additionally add new unit tests for edge cases: service/read methods returning
null or throwing a NotFound/EntityNotFound scenario and write methods receiving
invalid JSON (expecting validation errors or command processing failures) to
ensure the resource handles these paths and that writeService/readService
interactions are verified.

In
`@src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java`:
- Around line 9-11: The tests in OfficeExtensionWritePlatformServiceImplTest
currently use JUnit assertions (assertEquals, assertNotNull, assertThrows);
update them to use AssertJ style for consistency: replace assertEquals(expected,
actual) with assertThat(actual).isEqualTo(expected), assertNotNull(obj) with
assertThat(obj).isNotNull(), and assertThrows(…, () -> ...) with
assertThatThrownBy(() ->
...).isInstanceOf(TheException.class).withMessageContaining(...) as appropriate;
update all occurrences in the test class and import static
org.assertj.core.api.Assertions.assertThat and assertThatThrownBy instead of
JUnit assertion statics.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: dc30235e-457f-433f-b58b-6fdae4fd62ac

📥 Commits

Reviewing files that changed from the base of the PR and between 477ae7f and 9930ed2.

📒 Files selected for processing (9)
  • src/main/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResource.java
  • src/main/java/org/apache/fineract/savings/office/data/OfficeGeolocationData.java
  • src/main/java/org/apache/fineract/savings/office/data/OfficeServiceData.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformService.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformServiceImpl.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformService.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImpl.java
  • src/test/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResourceTest.java
  • src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java

Comment on lines +53 to +63
public OfficeServiceData retrieveOfficeService(final Long serviceId) {
this.context.authenticatedUser();
try {
return this.jdbcTemplate.queryForObject(
"SELECT s.id, s.office_id, s.service_name, s.service_external_id, s.working_hours"
+ " FROM m_selfservice_office_service s WHERE s.id = ?",
new OfficeServiceRowMapper(),
serviceId);
} catch (final EmptyResultDataAccessException e) {
return null;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | 🏗️ Heavy lift

Avoid null returns for not-found reads; use explicit contract semantics.

Line 62 and Line 77 return null on missing rows, which makes callers prone to NPEs and leaves the service contract ambiguous (especially for retrieveOfficeService). Prefer Optional<T> (or a domain not-found path) consistently across interface and implementation.

As per coding guidelines "Prefer returning Optional or sealed result types instead of null."

Also applies to: 68-78

🤖 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
`@src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformServiceImpl.java`
around lines 53 - 63, The method retrieveOfficeService in
OfficeExtensionReadPlatformServiceImpl currently returns null when no row is
found; change its contract to return Optional<OfficeServiceData> instead and
update its implementation to catch EmptyResultDataAccessException and return
Optional.empty(), otherwise return Optional.of(result) (use the existing
OfficeServiceRowMapper to map the row). Update the method signature in the
interface and all callers to handle Optional<OfficeServiceData> so no nulls are
propagated.

@DeathGun44 DeathGun44 force-pushed the MX-263-office-admin-crud-endpoints branch from 9930ed2 to 4489a35 Compare May 27, 2026 10:30
@DeathGun44
Copy link
Copy Markdown
Contributor Author

@coderabbitai review

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 27, 2026

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (2)
src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformService.java (1)

40-46: 🏗️ Heavy lift

Replace nullable geolocation return with Optional<OfficeGeolocationData>.

The method contract currently permits null, which conflicts with the project standard and pushes null-check burden to callers.

As per coding guidelines "Prefer returning Optional or sealed result types instead of null."

🤖 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
`@src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformService.java`
around lines 40 - 46, The method retrieveOfficeGeolocation currently returns a
nullable OfficeGeolocationData; change its signature to return
java.util.Optional<OfficeGeolocationData> (i.e., Optional<OfficeGeolocationData>
retrieveOfficeGeolocation(Long officeId)), update any implementing classes of
OfficeExtensionReadPlatformService to wrap returned values with
Optional.ofNullable(...) or Optional.empty(), and add the necessary import for
java.util.Optional; also update callers to handle Optional instead of null
(e.g., use .orElse, .isPresent, or .ifPresent as appropriate).
src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java (1)

178-185: ⚡ Quick win

Strengthen the delete-success test assertions.

In addition to assertNotNull, assert returned identifiers and verify the expected scoped SQL call to make this test protect behavior, not just execution.

🤖 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
`@src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java`
around lines 178 - 185, Update the test
deleteOfficeService_existingService_succeeds to assert the actual identifiers on
the returned CommandProcessingResult (e.g., verify result contains the expected
SERVICE_ID and OFFICE_ID/resource id fields) instead of only assertNotNull, and
add a verification that jdbcTemplate.update was invoked with the expected SQL
scope and parameters by verifying the call to jdbcTemplate.update used the
scoped DELETE SQL string and the eq(SERVICE_ID), eq(OFFICE_ID) arguments; locate
assertions around service.deleteOfficeService(OFFICE_ID, SERVICE_ID) and the
mock setup of jdbcTemplate.update to implement these checks.
🤖 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
`@src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImpl.java`:
- Around line 69-79: Validate the parsed JSON fields before adding them to
params: after parsing with fromJsonHelper.parse(...) and extracting serviceName,
serviceExternalId and workingHours (in OfficeExtensionWritePlatformServiceImpl
and similarly in the block covering lines 111-137), enforce non-blank checks and
max-length/format constraints (e.g., trim and require serviceName non-empty,
serviceExternalId matching allowed pattern/length, workingHours within allowed
length/format); if validation fails, throw the appropriate platform validation
exception (e.g., PlatformApiDataValidationException or IllegalArgumentException)
with clear messages, and only then call params.addValue(...) to persist
validated values.

---

Nitpick comments:
In
`@src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformService.java`:
- Around line 40-46: The method retrieveOfficeGeolocation currently returns a
nullable OfficeGeolocationData; change its signature to return
java.util.Optional<OfficeGeolocationData> (i.e., Optional<OfficeGeolocationData>
retrieveOfficeGeolocation(Long officeId)), update any implementing classes of
OfficeExtensionReadPlatformService to wrap returned values with
Optional.ofNullable(...) or Optional.empty(), and add the necessary import for
java.util.Optional; also update callers to handle Optional instead of null
(e.g., use .orElse, .isPresent, or .ifPresent as appropriate).

In
`@src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java`:
- Around line 178-185: Update the test
deleteOfficeService_existingService_succeeds to assert the actual identifiers on
the returned CommandProcessingResult (e.g., verify result contains the expected
SERVICE_ID and OFFICE_ID/resource id fields) instead of only assertNotNull, and
add a verification that jdbcTemplate.update was invoked with the expected SQL
scope and parameters by verifying the call to jdbcTemplate.update used the
scoped DELETE SQL string and the eq(SERVICE_ID), eq(OFFICE_ID) arguments; locate
assertions around service.deleteOfficeService(OFFICE_ID, SERVICE_ID) and the
mock setup of jdbcTemplate.update to implement these checks.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: e569b89d-924f-4724-a1f9-db86ba54c4da

📥 Commits

Reviewing files that changed from the base of the PR and between 9930ed2 and 4489a35.

📒 Files selected for processing (9)
  • src/main/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResource.java
  • src/main/java/org/apache/fineract/savings/office/data/OfficeGeolocationData.java
  • src/main/java/org/apache/fineract/savings/office/data/OfficeServiceData.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformService.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformServiceImpl.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformService.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImpl.java
  • src/test/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResourceTest.java
  • src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java
🚧 Files skipped from review as they are similar to previous changes (5)
  • src/main/java/org/apache/fineract/savings/office/data/OfficeGeolocationData.java
  • src/main/java/org/apache/fineract/savings/office/data/OfficeServiceData.java
  • src/test/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResourceTest.java
  • src/main/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResource.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformServiceImpl.java

Signed-off-by: DeathGun44 <krishnamewara841@gmail.com>
@DeathGun44 DeathGun44 force-pushed the MX-263-office-admin-crud-endpoints branch from 4489a35 to 5c62ae7 Compare May 28, 2026 16:41
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 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 @.github/workflows/maven-build.yml:
- Line 83: The JaCoCo coverage gate currently checks only
${project.build.directory}/jacoco.exec and tolerates missing IT data; update the
jacoco:check@enforce-coverage configuration to reference jacoco-merged.exec (or
add a separate jacoco-it check) so merged IT coverage is enforced (adjust the
maven profile or goal that sets the file path from jacoco.exec to
jacoco-merged.exec), and modify the workflow steps "Upload Integration Test
Coverage Data" and "Download Integration Test Coverage Data" to stop ignoring
missing files (remove if-no-files-found: ignore and continue-on-error: true) and
instead explicitly fail when jacoco-it.exec is absent on branches where IT
coverage is required (use a conditional like only run tolerance for pull_request
but fail on branch pushes), ensuring the enforcement step
(jacoco:check@enforce-coverage) runs against the merged/IT exec file.
🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: f4a6b8ce-6d37-4f19-ae83-04a54ffd1f23

📥 Commits

Reviewing files that changed from the base of the PR and between 4489a35 and 5c62ae7.

📒 Files selected for processing (10)
  • .github/workflows/maven-build.yml
  • src/main/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResource.java
  • src/main/java/org/apache/fineract/savings/office/data/OfficeGeolocationData.java
  • src/main/java/org/apache/fineract/savings/office/data/OfficeServiceData.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformService.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformServiceImpl.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformService.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImpl.java
  • src/test/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResourceTest.java
  • src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java
✅ Files skipped from review due to trivial changes (2)
  • src/main/java/org/apache/fineract/savings/office/data/OfficeGeolocationData.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformService.java
🚧 Files skipped from review as they are similar to previous changes (7)
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformServiceImpl.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionReadPlatformService.java
  • src/test/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResourceTest.java
  • src/main/java/org/apache/fineract/savings/office/api/OfficeExtensionApiResource.java
  • src/main/java/org/apache/fineract/savings/office/data/OfficeServiceData.java
  • src/test/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImplTest.java
  • src/main/java/org/apache/fineract/savings/office/service/OfficeExtensionWritePlatformServiceImpl.java

Comment thread .github/workflows/maven-build.yml
@IOhacker IOhacker merged commit 1458c4d into openMF:develop May 28, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants