-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
CWDB-225 - Implemented Measurement Controller #890
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, just a few suggestions and points of clarification
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/main/java/cwms/cda/api/MeasurementPatchController.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/test/java/cwms/cda/api/MeasurementControllerTestIT.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/test/java/cwms/cda/api/MeasurementControllerTestIT.java
Show resolved
Hide resolved
cwms-data-api/src/test/java/cwms/cda/api/MeasurementControllerTestIT.java
Outdated
Show resolved
Hide resolved
cf63abe
to
502182a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some suggestions and a question
cwms-data-api/src/test/java/cwms/cda/api/MeasurementControllerTestIT.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/test/java/cwms/cda/api/MeasurementControllerTestIT.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/test/java/cwms/cda/api/MeasurementControllerTestIT.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/test/java/cwms/cda/api/MeasurementControllerTestIT.java
Outdated
Show resolved
Hide resolved
…easurements to cleanup routine in IT. Code format refactors.
f88aea1
to
a84e771
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some failing integration tests still
…ements for bulk update
@MikeNeilson - With Measurements we wanted to support a bulk store, and there is no rename, so update is really just a check-for-existing and then store... The test db doesn't have display units for the office (SPK) needed for bulk retrieving existing measurements efficiently (there is jooq code in the MeasurementDao that retrieves in bulk from the meas view, with the given meas numbers), which causes the tests to fail, without configuring the at_display_units table manually for SPK . If configured, the existing check does work as expected. EDIT: after further discussion we decided its best to just not support an update at all for measurements, as it doesn't make much sense. No PATCH, just POST. |
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
…ed via a single-element-array. Updated to support all optional query parameters for retrieval.
cwms-data-api/src/main/java/cwms/cda/api/MeasurementController.java
Outdated
Show resolved
Hide resolved
…s. Made descriptions of date/timezones consistent.
…of null begin/end.
No description provided.