Add Update Report Definition support#159
Add Update Report Definition support#159smcgowan-smartsheet wants to merge 8 commits intosmartsheet:mainlinefrom
Conversation
Review completed for commit 7757db4. The documentation has been corrected to reflect PUT instead of PATCH. All previous issues have been addressed:
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
smcgowan-smartsheet
changed the title
Review completed. The implementation looks solid and follows established patterns in the codebase. No issues were identified that require changes. The PR successfully adds support for the PATCH /reports/{id}/definition endpoint with:
Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues. |
src/main/java/com/smartsheet/api/models/ReportGroupingCriterion.java
Outdated
Show resolved
Hide resolved
remove read-only param as not in use
| /** | ||
| * Represents the AUTO_NUMBER system column type. | ||
| */ | ||
| AUTO_NUMBER, |
There was a problem hiding this comment.
Not supported by reports, should be removed from all SDKs
| * | ||
| * @return true if nulls forced to bottom, false otherwise | ||
| */ | ||
| public Boolean getForceNullsToBottom() { |
There was a problem hiding this comment.
This property will be removed
remove invalid options
add update filters query param support
change aggregationCriteria to summarizingCriteria to match spec add returns report definition
changed to PUT from PATCH no longer returning changed definition no longer needs query param
![roomote-v0[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| /** | ||
| * <p>Updates a report's definition (filters, grouping, summarize, and sorting).</p> | ||
| * | ||
| * <p>It mirrors to the following Smartsheet REST API method: PATCH /reports/{id}/definition</p> |
There was a problem hiding this comment.
The documentation states this method mirrors "PATCH /reports/{id}/definition", but the implementation in ReportResourcesImpl.java (line 314) and the tests (TestUpdateReportDefinition.java line 92) both expect PUT. This should be updated to "PUT /reports/{id}/definition" to match the actual implementation.
| * <p>It mirrors to the following Smartsheet REST API method: PATCH /reports/{id}/definition</p> | |
| * <p>It mirrors to the following Smartsheet REST API method: PUT /reports/{id}/definition</p> |
Fix it with Roo Code or mention @roomote and request a fix.
3 participants
Pull Request
Summary
Add Support for PATCH /reports/{id}/definition endpoints - Update Report Definition
Added Wiremock tests for the new endpoint
Checklist
./gradlew clean build