Skip to content

Commit 7da86e2

Browse files
EnovotnyEric Novotny
andauthored
fix the timeseries group office filtering (#1709)
# Fix timeseries/group endpoint returning groups for all offices when include-assigned=false ## Problem The `/timeseries/group` endpoint was returning timeseries groups from all offices when the `include-assigned=false` query parameter was set, instead of respecting the `group-office-id` and `category-office-id` filters. ### Example Query: GET /timeseries/group?office=MVP&group-office-id=MVP&include-assigned=false&timeseries-category-like=SHEF%20Export Expected: Only groups with `office-id=MVP` Actual: Groups from MVP, SWT, LRN, SAJ, SAM and other offices were returned ## Root Cause The `TimeSeriesGroupDao.getTimeSeriesGroups()` method branches based on `includeAssigned`: - When `includeAssigned=true`: calls `getTimeSeriesGroupsWhere()` which applies office filters - When `includeAssigned=false`: calls `getTimeSeriesGroupsWithoutAssigned()` which was NOT receiving or applying the office filter parameters ## Solution Updated `getTimeSeriesGroupsWithoutAssigned()` to: 1. Accept `groupOfficeId` and `categoryOfficeId` parameters 2. Apply the same office filtering logic as `getTimeSeriesGroupsWhere()` This ensures consistent behavior regardless of the `include-assigned` parameter value. ## Changes - **cwms-data-api/src/main/java/cwms/cda/data/dao/TimeSeriesGroupDao.java** - Modified `getTimeSeriesGroupsWithoutAssigned()` signature to accept office parameters - Added office filtering conditions to the query - Updated method call to pass the parameters - **cwms-data-api/src/test/java/cwms/cda/api/TimeSeriesGroupControllerTestIT.java** - Added `test_get_all_groups_without_assigned_filters_by_office()` to validate office filtering works with `include-assigned=false` - Test leverages existing test data (SPK and CWMS offices with different groups) ## Testing - New test PASSED ✅ - Verifies that when querying with `GROUP_OFFICE_ID=SPK` and `include-assigned=false`, only groups from SPK office are returned - Existing TimeSeriesGroupControllerTestIT tests continue to pass ## Impact - Fixes endpoint behavior to match documented API - No breaking changes - only corrects the filtering to work as expected Co-authored-by: Eric Novotny <nov00002@umn.edu>
1 parent 8ffecfa commit 7da86e2

2 files changed

Lines changed: 58 additions & 12 deletions

File tree

cwms-data-api/src/main/java/cwms/cda/data/dao/TimeSeriesGroupDao.java

Lines changed: 22 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,7 @@ public List<TimeSeriesGroup> getTimeSeriesGroups(String tsOfficeId, String group
106106
if (includeAssigned) {
107107
return getTimeSeriesGroupsWhere(dsl, whereCond, tsOfficeId, groupOfficeId, categoryOfficeId);
108108
} else {
109-
return getTimeSeriesGroupsWithoutAssigned(whereCond);
109+
return getTimeSeriesGroupsWithoutAssigned(whereCond, groupOfficeId, categoryOfficeId);
110110
}
111111

112112
}
@@ -226,19 +226,29 @@ private List<TimeSeriesGroup> getTimeSeriesGroupsWhere(DSLContext dslContext, Co
226226
}
227227

228228
@NotNull
229-
private List<TimeSeriesGroup> getTimeSeriesGroupsWithoutAssigned(Condition whereCond) {
229+
private List<TimeSeriesGroup> getTimeSeriesGroupsWithoutAssigned(Condition whereCond, String groupOfficeId, String categoryOfficeId) {
230+
AV_TS_CAT_GRP catGrp = AV_TS_CAT_GRP.AV_TS_CAT_GRP;
231+
232+
Condition whereCondGrpCat = DSL.noCondition();
233+
if (categoryOfficeId != null) {
234+
whereCondGrpCat = whereCondGrpCat.and(catGrp.CAT_DB_OFFICE_ID.eq(categoryOfficeId.toUpperCase()));
235+
}
236+
if (groupOfficeId != null) {
237+
whereCondGrpCat = whereCondGrpCat.and(catGrp.GRP_DB_OFFICE_ID.eq(groupOfficeId.toUpperCase()));
238+
}
230239

231240
SelectConditionStep<Record8<String, String, String, String, String, String, String, String>> query = dsl.select(
232-
AV_TS_CAT_GRP.AV_TS_CAT_GRP.CAT_DB_OFFICE_ID,
233-
AV_TS_CAT_GRP.AV_TS_CAT_GRP.TS_CATEGORY_ID,
234-
AV_TS_CAT_GRP.AV_TS_CAT_GRP.TS_CATEGORY_DESC,
235-
AV_TS_CAT_GRP.AV_TS_CAT_GRP.GRP_DB_OFFICE_ID,
236-
AV_TS_CAT_GRP.AV_TS_CAT_GRP.TS_GROUP_ID,
237-
AV_TS_CAT_GRP.AV_TS_CAT_GRP.TS_GROUP_DESC,
238-
AV_TS_CAT_GRP.AV_TS_CAT_GRP.SHARED_TS_ALIAS_ID,
239-
AV_TS_CAT_GRP.AV_TS_CAT_GRP.SHARED_REF_TS_ID)
240-
.from(AV_TS_CAT_GRP.AV_TS_CAT_GRP)
241-
.where(whereCond);
241+
catGrp.CAT_DB_OFFICE_ID,
242+
catGrp.TS_CATEGORY_ID,
243+
catGrp.TS_CATEGORY_DESC,
244+
catGrp.GRP_DB_OFFICE_ID,
245+
catGrp.TS_GROUP_ID,
246+
catGrp.TS_GROUP_DESC,
247+
catGrp.SHARED_TS_ALIAS_ID,
248+
catGrp.SHARED_REF_TS_ID)
249+
.from(catGrp)
250+
.where(whereCond)
251+
.and(whereCondGrpCat);
242252

243253
logger.atFine().log("%s", lazy(() -> query.getSQL(ParamType.INLINED)));
244254

cwms-data-api/src/test/java/cwms/cda/api/TimeSeriesGroupControllerTestIT.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2382,4 +2382,40 @@ void test_create_with_ignore_nulls_parameter(String format) throws Exception {
23822382
// delete category and group gets done by afterEach.
23832383
}
23842384

2385+
@Test
2386+
void test_get_all_groups_without_assigned_filters_by_office() {
2387+
String officeId = user.getOperatingOffice();
2388+
2389+
// Get all groups for this office with include-assigned=false
2390+
// This tests the fix where groupOfficeId filter was not being applied when includeAssigned=false
2391+
Response response = given()
2392+
.log().ifValidationFails(LogDetail.ALL, true)
2393+
.accept("application/json")
2394+
.queryParam(GROUP_OFFICE_ID, officeId)
2395+
.queryParam("include-assigned", false)
2396+
.when()
2397+
.get("/timeseries/group")
2398+
.then()
2399+
.assertThat()
2400+
.log().ifValidationFails(LogDetail.ALL, true)
2401+
.statusCode(anyOf(is(200), is(404)))
2402+
.extract()
2403+
.response();
2404+
2405+
// If we get a 404, that's fine (no groups for this office)
2406+
// If we get a 200, verify all returned groups are from the specified office
2407+
if (response.statusCode() == 200) {
2408+
JsonPath jsonPathEval = response.jsonPath();
2409+
List<String> officeIds = jsonPathEval.get("office-id");
2410+
2411+
if (officeIds != null && !officeIds.isEmpty()) {
2412+
// All returned groups should be from the specified office
2413+
for (String returnedOfficeId : officeIds) {
2414+
assertThat("Expected all groups to be from office " + officeId + ", but found " + returnedOfficeId,
2415+
returnedOfficeId, equalTo(officeId));
2416+
}
2417+
}
2418+
}
2419+
}
2420+
23852421
}

0 commit comments

Comments
 (0)