Skip to content

feat: add support for add and remove report scope#124

Open
smcgowan-smartsheet wants to merge 11 commits intosmartsheet:mainlinefrom
smcgowan-smartsheet:mainline
Open

feat: add support for add and remove report scope#124
smcgowan-smartsheet wants to merge 11 commits intosmartsheet:mainlinefrom
smcgowan-smartsheet:mainline

Conversation

@smcgowan-smartsheet
Copy link

@smcgowan-smartsheet smcgowan-smartsheet commented Feb 10, 2026

Pull Request

Description

Adding

  • Support for POST /2.0/reports/{reportId}/scope endpoint
  • Support for DELETE /2.0/reports/{ReportId}/scope endpoint
  • Added wiremock integration tests for POST /2.0/reports/{reportId}/scope endpoint
  • Added wiremock integration tests for DELETE /2.0/reports/{reportId}/scope endpoint

Type of Change

  • Bug fix
  • New feature
  • Documentation update
  • Code refactoring
  • Other (please describe):

What Changes Were Made

Adding support for two additional endpoints which are to be added to the public API

  • POST /2.0/reports/{reportId}/scope
  • DELETE /2.0/reports/{ReportId}/scope

Why These Changes Were Made

Ensuring SDKs maintain parity with the public API

Testing

Added wiremock testing for the endpoints

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have updated the relevant files in docs-source/ (see Documentation guidelines)
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

@roomote
Copy link
Contributor

roomote bot commented Feb 10, 2026

Rooviewer Clock   See task

Re-reviewed at commit f1f5f69. Fixed 2 issues (asset_type getter and to_json whitespace). Still 3 outstanding type hint issues in report_scope_inclusion.py.

  • Remove incorrect type hint from asset_id property getter (line 42)
  • Remove incorrect type hint from asset_id property setter (line 46)
  • Fix asset_type property getter to return EnumeratedValue object instead of .value (lines 49-51)
  • Remove overly restrictive type hint from asset_type property setter (line 54)
  • Remove extra whitespace in to_json method signature (line 60)
Previous reviews

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

@smcgowan-smartsheet smcgowan-smartsheet marked this pull request as ready for review February 10, 2026 12:00
@roomote
Copy link
Contributor

roomote bot commented Feb 10, 2026

Rooviewer Clock   See task

Reviewed at commit c42e77c. All previously flagged issues from earlier reviews have been resolved. Found one minor issue in the current changes:

Additional recommendations (require changes to files not modified in this PR):

  • Consider adding from .report_scope_inclusion import ReportScopeInclusion to smartsheet/models/__init__.py for consistent model imports
  • Consider adding from .report_asset_type import ReportAssetType to smartsheet/models/enums/__init__.py for consistent enum imports

Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants