mikebridge opened a new pull request, #42711:
URL: https://github.com/apache/superset/pull/42711
### SUMMARY
The version-history endpoints' `changed_by` attribution carried `username`,
while the sibling activity payload (`ActivityChangedBySchema`, same file) has
always excluded it by design, with a unit test pinning the exclusion:
`username` is a login identifier, not display data. Exposing it in one payload
of the pair was an inconsistency that becomes a breaking change to fix the
moment the API ships — flagged in the versioning capstone review as "cheap now,
breaking later."
This PR trims the SELECT and the projection to `id` +
`first_name`/`last_name`, aligns `VersionChangedBySchema` with
`ActivityChangedBySchema`, and pins both directions:
- the projection excludes `username` even if the SELECT widens again
- a schema-parity test asserts the two attribution schemas keep the
identical field set, so clients can share one type and a field added to one
must be consciously added to (or rejected from) both
No known consumer reads it — the version-history frontend types the field
but never renders it, and no test asserted its presence.
The version endpoints are part of the not-yet-released versioning feature
set (behind `VERSION_HISTORY`, default off), so this is a pre-release contract
correction, not a breaking change to a shipped API.
Disclosure: this change was developed with AI assistance (Claude), on behalf
of and reviewed by @mikebridge.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — API payload shape only; nothing rendered it.
### TESTING INSTRUCTIONS
- `pytest tests/unit_tests/versioning/test_queries.py` — projection excludes
`username` even when present in the row; `changed_by` still `None` for
user-less commits; schema-parity assertion.
- `pytest tests/integration_tests/versioning/
tests/integration_tests/dashboards/version_restore_tests.py
tests/integration_tests/charts/version_restore_tests.py
tests/integration_tests/datasets/version_restore_tests.py` — payload consumers
green.
- Manual: `GET /api/v1/dashboard/{uuid}/versions/` with `VERSION_HISTORY`
enabled — `changed_by` is `{id, first_name, last_name}`.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [x] Required feature flags: `VERSION_HISTORY` (endpoints affected are
gated by it; default off)
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
