docs(experiments): spec for the experiment creator's name in the API (#37304) - #37510
docs(experiments): spec for the experiment creator's name in the API (#37304)#37510oidacra wants to merge 1 commit into
Conversation
Spec for exposing the experiment creator's display name so the Experiments portlet listing can render a Created By column without resolving user IDs itself (consumer: #37307). Additive and non-breaking: a createdByUserName field joins the existing createdBy, which keeps its key, its value and its role as the permission owner. The field is added at the Experiment level, so all 14 endpoints that return an Experiment carry it. lastModifiedBy is deliberately out of scope. Grounded against the code rather than the issue text: - the issue lists DELETE /v1/experiments/{id} as carrying an Experiment; it returns a confirmation message. Two endpoints that do return one were omitted (DELETE .../goals/primary, DELETE .../targetingConditions/{id}). The spec's endpoint table supersedes the issue list (A7). - user lookups are already cache-backed via UserCache (A1), which is what the no-N-lookups requirement rests on. - a @Schema on an Immutables abstract accessor does reach openapi.yaml (verified via AbstractTimestampsView), so no annotation pass over ExperimentsResource is needed (A3). - the immutables mechanism is left to the plan phase and boxed in by four requirements: eager derivation would fire on non-serializing paths, and a memoized lazy value would outlive a rename (A2). 23 FRs, 7 SCs, 3 user stories, 8 assumptions.
|
Claude finished @oidacra's task in 1m 42s —— View job Spec review: creator name on the Experiments API
This is a spec-only PR, so I reviewed the spec's factual accuracy against the code it commits to. The endpoint table, the New Issues
Verified accurate (no action needed)
Overall the spec is thorough and the additive approach is sound. The single-space finding is non-blocking but should be corrected before |
Spec-first PR for #37304, using the single-PR flow: the specification lands as the initial commit and the implementation will land in this same PR.
Closes #37304. Visual review page (summarizes; the spec decides): https://claude.ai/code/artifact/fdc2cd0a-bf41-4a1f-9b77-3c3ae9e47e85
What the spec defines
Add an additive
createdByUserNameto the Experiment serialization, so every response carrying an Experiment reports the creator's display name instead of only an opaque user id.createdBykeeps its key, its value and its role as the permission owner behindgetOwner(), so nothing that consumes the current payload changes. Consumer: #37307 (the portlet listing's Created By column).PATCH,_archive,_start,_end,scheduled/{id}/_cancel, the four variant operations, and the twoDELETEendpoints for the primary goal and a targeting condition.User.getFullName()returns""there. A lookup failure never turns a successful experiment read into a failed request.openapi.yaml. A full@Operation/@ApiResponsepass overExperimentsResource(which today carries only@Tag) is explicitly out of scope.lastModifiedByis deliberately excluded — same id-not-name problem, separate follow-up.23 functional requirements, 7 success criteria, 3 user stories.
Three findings that shaped the spec
The Immutables mechanism is left to the plan phase, deliberately.
@Value.Derivedcomputes atbuild(), so the user lookup would fire on every Experiment construction — including the DB transformer, the push-publish dependency walk and running-experiment selection during page render, none of which serialize anything.@Value.Lazymemoizes inside the instance, and Experiment instances are themselves cached inExperimentsCache, so a memoized name would outlive a rename. Neither is settable from JSON. The spec states four constraints (FR-014, FR-015, FR-016, FR-023) and leaves the choice to/speckit-plan, which has to check its pick against all four.User lookups are already cache-backed.
UserFactoryLiferayImpl.loadUserByIdreadsUserCachebefore the database, so a listing whose experiments share a creator costs one read. Kept as a requirement anyway: a listing spanning many distinct creators must not degenerate into one lookup per row.A
@Schemaon an Immutables abstract accessor does reachopenapi.yaml— verified againstAbstractTimestampsView, whose per-accessor descriptions and examples appear verbatim under the generatedTimestampsViewschema. That is what makes annotating the model sufficient.Corrections to the issue body
The issue's endpoint list is close but not exact, so the spec carries a 14-endpoint table that supersedes it. None of this changes the approach — the field is added once and every experiment-carrying response inherits it.
DELETE /v1/experiments/{experimentId}is listed among the endpoints carrying an Experiment; it returns the confirmation string"Experiment deleted".DELETE /v1/experiments/{experimentId}/goals/primaryandDELETE /v1/experiments/{experimentId}/targetingConditions/{id}.PATCH, notPUT.Checklist
MainSuite*)createdByandgetOwner()are untouched, so permission behaviour is unchanged. The field exposes a creator's display name only to users who can already read the experiment.