Skip to content

Normalize share comments response shape#73

Merged
ravirajsinh45 merged 1 commit into
Techiebutler:mainfrom
GChaucer:fix/share-comments-array-response
May 22, 2026
Merged

Normalize share comments response shape#73
ravirajsinh45 merged 1 commit into
Techiebutler:mainfrom
GChaucer:fix/share-comments-array-response

Conversation

@GChaucer

Copy link
Copy Markdown
Contributor

Summary

  • Normalize GET /share/{token}/comments so the no-target fallback returns a bare array.
  • Simplify the single-asset share page comments response type now that the endpoint returns a consistent array shape.
  • Add backend regression coverage for asset-share, folder/project-share query asset, and no-target fallback paths.

Closes #72

Testing

  • . .venv/bin/activate && pytest apps/api/tests/test_share_comments.py
  • . .venv/bin/activate && pytest apps/api/tests/test_share_comments.py apps/api/tests/test_smoke.py
  • git diff --check

Notes

  • Installed API test dependencies in a local virtualenv.
  • Frontend lint was not run because npm ci --ignore-scripts fails due package-lock/package.json drift, and pnpm install --frozen-lockfile --ignore-scripts fails because pnpm-lock.yaml is out of sync with package.json.

@ravirajsinh45 ravirajsinh45 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM — clean implementation of the normalization from #72. Backend → array, frontend simplified, tests cover all three paths (asset-share, folder-share via ?asset_id, no-target empty). Mock pattern matches the rest of the suite.

@ravirajsinh45 ravirajsinh45 merged commit dfa0ab1 into Techiebutler:main May 22, 2026
4 checks passed
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.

Normalize /share/{token}/comments response shape

2 participants