Add StatusResponse class to handle successful response messages #1167

rohaan-rma · 2025-07-01T17:53:32Z

Saving the last 5 tasks for a separate PR (This one is too long already)

MikeNeilson · 2025-07-01T18:14:35Z

cwms-data-api/src/test/java/cwms/cda/data/dto/StatusResponseTest.java

+    @Test
+    void createStatusResponse_allFieldsProvided_Success() {
+        // all three fields provided
+        StatusResponse item = new StatusResponse("TestOffice", "TestMessage", "TestIdentifier123");


Use "real" data.... At least for the office anyways. I get a real "message" could be a bit tricky.

Done, used more realistic test data names.

MikeNeilson · 2025-07-01T18:16:28Z

cwms-data-api/src/test/resources/cwms/cda/data/dto/status_response.json

@@ -0,0 +1,5 @@
+{


Do we actually want the officeID, there's a "CwmsBaseDTO" that CwmsDTO Derives from that intentionally doesn't include the office-id field.

I'm trying to include some relevant data other than just the response message. I assumed an office-id would be helpful to have, but I'm open to getting rid of it if we think it's unnecessary. Perhaps there's some more specific data that would be better to include instead.

if you look at CdaError it contains a Map field for additional data.
... might also be worth seeing if that just needs a better name, like CdaResponse, in instead of error.

Yes, I've been looking at the CdaError class. I do think I can rework/use that class to become a general message response class. Although the incidentIdentifier field in that class would be a little useless for successful responses since there won't be any error logging.

I guess this begs the question of whether or not we want to create a new class for general messages or whether or not we want to reuse the CdaError class and put some information in the details map. I think either way is probably fine.

Also, I rewrote some messages in the TurbineController class as an example of how the current StatusResponse class would be used.

Now that I've worked further, I believe the two most important fields to report are the message itself and a name or ID of what was changed (created, updated, or deleted).

While it was possible to rework the CdaError class to be more generalized, I don't think that this basic status response class needs a details map or an incident identifier for error logs. Having separate classes for errors and successful responses also seems to make more sense.

Also, since there are a lot of files to update, it may be better to get this PR merged first and then I can make separate PRs to update the rest of the codebase. This might make things easier on reviewers as well.

…Api docs

cwms-data-api/src/main/java/cwms/cda/data/dto/StatusResponse.java

cwms-data-api/src/main/java/cwms/cda/api/TurbineController.java

…age, update tests and TurbineController accordingly

…e work still necessary)

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

cwms-data-api/src/main/java/cwms/cda/api/StreamLocationController.java

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

cwms-data-api/src/main/java/cwms/cda/api/watersupply/WaterContractTypeCreateController.java

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

cwms-data-api/src/main/java/cwms/cda/api/PropertyController.java

MikeNeilson

Please do not add additional controller changes to this PR. It's already rather long.

MikeNeilson · 2025-07-07T19:50:03Z

Please do not add additional controller changes to this PR. It's already rather long.

For clarity I mean updates to the new scheme, fixes and additional change may be expected.

MikeNeilson · 2025-07-07T19:51:36Z

cwms-data-api/src/main/java/cwms/cda/data/dto/StatusResponse.java

+@FormattableWith(contentType = Formats.JSONV1, formatter = JsonV1.class, aliases = {Formats.DEFAULT, Formats.JSON})
+@JsonInclude(JsonInclude.Include.NON_NULL)
+@JsonNaming(PropertyNamingStrategies.KebabCaseStrategy.class)
+public final class StatusResponse extends CwmsDTO{


why was this set back to CwmsDTO from CwmsDTOBase?

We looked over everything again, and decided that the office-id field is useful to include since almost everything is office specific. There was really no additional overhead to requiring the office-id field.

Also, every controller has an office-id with easy access. I checked all the instances of where I'm going to be implementing StatusResponse and I couldn't find a case where I wouldn't be able to grab an office-id.

cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java

cwms-data-api/src/main/java/cwms/cda/api/location/kind/OutletController.java

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

…eChanges, and StreamReach Controllers. Update Tests accordingly.

…, TurbineChanges, and StreamReach Controllers. Update Tests accordingly." This reverts commit 7a9513b.

… delete response bodies

cwms-data-api/src/main/java/cwms/cda/api/StreamLocationController.java

cwms-data-api/src/main/java/cwms/cda/api/LookupTypeController.java

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

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

zack-rma

Changes look good

Rohaan Sandhu and others added 4 commits June 25, 2025 14:31

Add basic version of StatusResponse class

8d6f645

Add serialization/deserialization unit tests for StatusResponse

6a96c9d

Get rid of harcoded vars in StatusResponse

83ca1e8

Merge branch 'develop' into bugfix/invalid-JSON-mismatch

c388048

rohaan-rma changed the title ~~Add StatusResponse class to handle successful JSON messages~~ Add StatusResponse class to handle successful response messages Jul 1, 2025

MikeNeilson reviewed Jul 1, 2025

View reviewed changes

Rohaan Sandhu added 2 commits July 1, 2025 13:17

Make tests more realistic

277eea0

Add possible StatusResponse usage in TurbineController, also fix Open…

5589725

…Api docs

zack-rma reviewed Jul 1, 2025

View reviewed changes

cwms-data-api/src/main/java/cwms/cda/data/dto/StatusResponse.java Outdated Show resolved Hide resolved

cwms-data-api/src/main/java/cwms/cda/api/TurbineController.java Outdated Show resolved Hide resolved

cwms-data-api/src/main/java/cwms/cda/api/TurbineController.java Show resolved Hide resolved

Rohaan Sandhu added 3 commits July 1, 2025 14:45

Make StatusResponse extend CwmsDTOBase, change response field to mess…

ad9fc06

…age, update tests and TurbineController accordingly

Add StatusResponse class to LevelsController and update basic IT (mor…

795b6fb

…e work still necessary)

Add tests for TurbineController and finish tests for LevelsController

d193300

zack-rma reviewed Jul 2, 2025

View reviewed changes

cwms-data-api/src/test/java/cwms/cda/api/LevelsControllerTestIT.java Outdated Show resolved Hide resolved

Rohaan Sandhu and others added 8 commits July 2, 2025 12:56

Clean code: use levelId for testing identifier

c65985e

Use levelId in tests for identifier

5aebacc

Add StatusResponse to LocationController and update tests

4cf8606

Add StatusResponse to EmbankmentController and IT

670c856

Add StatusResponse to StreamLocation

bf83fa6

Add office-id field back to StatusResponse

887a5b6

Update Controllers and tests to include office-id

81a558e

Merge branch 'develop' into bugfix/invalid-JSON-mismatch

514878f

adamkorynta self-requested a review July 3, 2025 18:56

zack-rma reviewed Jul 3, 2025

View reviewed changes

cwms-data-api/src/main/java/cwms/cda/api/StreamLocationController.java Outdated Show resolved Hide resolved

cwms-data-api/src/test/java/cwms/cda/api/LevelsControllerTestIT.java Outdated Show resolved Hide resolved

rohaan-rma added 7 commits July 3, 2025 13:30

Reformat tests and remove useless import

1064f01

Add StatusResponse to StreamController

209c1f9

Add StatusResponse to BasinController

6afa97f

Add StatusResponse to LookupTypeController

e70a119

Add StatusRespoonse to WaterContractCRUDControllers

94fd73c

Add StatusResponse to WaterContractTypeControllers

c4e40ba

Add StatusResponse to WaterUserControllers

09f5293

Add StatusResponse to LockController

07d14a0

zack-rma suggested changes Jul 7, 2025

View reviewed changes

cwms-data-api/src/main/java/cwms/cda/api/watersupply/WaterContractTypeCreateController.java Outdated Show resolved Hide resolved

cwms-data-api/src/test/java/cwms/cda/api/WaterContractControllerTestIT.java Outdated Show resolved Hide resolved

rohaan-rma added 2 commits July 7, 2025 11:22

Add StatusResponse to PropertyController

d6cc8f4

Add StatusResponse to MeasurementController

232c9b4

zack-rma reviewed Jul 7, 2025

View reviewed changes

cwms-data-api/src/main/java/cwms/cda/api/PropertyController.java Outdated Show resolved Hide resolved

rohaan-rma added 2 commits July 7, 2025 11:49

Add StatusResponse to OutletController

8e21e9d

Remove unnecessary import and fix update message

db3779e

MikeNeilson reviewed Jul 7, 2025

View reviewed changes

zack-rma suggested changes Jul 7, 2025

View reviewed changes

rohaan-rma added 7 commits July 7, 2025 13:35

Use String constants in tests

73d932f

Fix HTTP Status inconsistencies

56d34ab

Fix bug in StreamLocationController

e54be2c

Add StatusResponse to VirtualOutlet, AccountingCreate, Rating, Turbin…

7a9513b

…eChanges, and StreamReach Controllers. Update Tests accordingly.

Revert "Add StatusResponse to VirtualOutlet, AccountingCreate, Rating…

9f861b7

…, TurbineChanges, and StreamReach Controllers. Update Tests accordingly." This reverts commit 7a9513b.

Change Status codes from 204 -> 200, (NO_CONTENT to OK), to allow for…

d4ff05d

… delete response bodies

Fix a couple bugs and formatting/import issues

a4606e0

rohaan-rma marked this pull request as ready for review July 8, 2025 18:28

rohaan-rma added 2 commits July 8, 2025 11:34

Fix status code mismatch

7b05143

Merge branch 'develop' into bugfix/invalid-JSON-mismatch

5aecc94

zack-rma suggested changes Jul 8, 2025

View reviewed changes

Fix format issues and update response messages

f91a399

rohaan-rma requested a review from zack-rma July 9, 2025 17:00

zack-rma reviewed Jul 9, 2025

View reviewed changes

rohaan-rma requested a review from MikeNeilson July 9, 2025 23:02

rohaan-rma added 2 commits July 10, 2025 11:44

Merge branch 'develop' into bugfix/invalid-JSON-mismatch

04f779f

Fix broken Integration Tests to update to new Status codes

2c905ba

adamkorynta approved these changes Jul 11, 2025

View reviewed changes

MikeNeilson merged commit 4f5a6f1 into USACE:develop Jul 11, 2025
5 checks passed

Add StatusResponse class to handle successful response messages #1167

Add StatusResponse class to handle successful response messages #1167

Uh oh!

Conversation

rohaan-rma commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

MikeNeilson left a comment

Choose a reason for hiding this comment

Uh oh!

MikeNeilson commented Jul 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zack-rma left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

rohaan-rma commented Jul 1, 2025 •

edited

Loading