Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optionally set an Author on an Edition #2919

Conversation

pezholio
Copy link
Contributor

In Content Modelling, when viewing content that is dependent of a content block, we'd like to be able to see who last edited that content, so publishers can liaise with the "owner" of that content if, for example, a change to a content block will alter the meaning of the dependent content.

As the Content Block Manager is publishing app agnostic, we currently have no way of knowing who last touched a piece of content, as this information is only stored within the publishing app itself.

This PR adds a new last_edited_by field which can optionally be sent to the Publishing API. In future this will allow us to use the Embedded Content Endpoint to return information about authors (if it exists).

The next step will be to update the presenters in the various publishing apps - trialing with Whitehall first.

Copy link
Member

@yndajas yndajas left a comment

Choose a reason for hiding this comment

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

Generally looks good. A couple of things that might be good to address, and a couple of other more minor suggestions. I might also quickly run the broader idea past the team in our dev chat this afternoon


The labels/first lines of each of the comments here are following the Conventional Comments format, which I'm trying out after watching a useful DevOpsDays London talk

app/models/edition.rb Outdated Show resolved Hide resolved
app/commands/v2/put_content.rb Outdated Show resolved Hide resolved
spec/integration/put_content/new_edition_spec.rb Outdated Show resolved Hide resolved
@pezholio pezholio force-pushed the content-modelling/632-spike-can-we-get-the-user-who-have-last-updated-a-piece-of-content branch 3 times, most recently from 94eba2d to c2590b7 Compare October 17, 2024 08:51
@pezholio pezholio requested a review from yndajas October 17, 2024 09:05
Copy link
Member

@yndajas yndajas left a comment

Choose a reason for hiding this comment

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

Looks good, just a question about the field name/intended use, and a bit of leftover code that can be removed

This takes the UUID of the user who last edited the edition.
@pezholio pezholio force-pushed the content-modelling/632-spike-can-we-get-the-user-who-have-last-updated-a-piece-of-content branch from c2590b7 to eeb4a2e Compare October 17, 2024 10:29
@pezholio pezholio requested a review from yndajas October 17, 2024 10:29
This ensures this value gets set if it is present. I’ve added some
shared exampes to the new edition and previous draft integration specs,
as these seemed like the best places for them to go.
This is currently only internal to the Publishing API, so does not need
to be sent upstream.
@pezholio pezholio force-pushed the content-modelling/632-spike-can-we-get-the-user-who-have-last-updated-a-piece-of-content branch from eeb4a2e to cc2a00c Compare October 17, 2024 10:37
@pezholio pezholio enabled auto-merge October 17, 2024 10:38
@pezholio pezholio merged commit 1f7f0ba into main Oct 17, 2024
37 of 38 checks passed
@pezholio pezholio deleted the content-modelling/632-spike-can-we-get-the-user-who-have-last-updated-a-piece-of-content branch October 17, 2024 10:40
nacnudus added a commit to nacnudus/govuk-knowledge-graph-gcp that referenced this pull request Oct 22, 2024
The editions table in the upstream Publishing API database has a new
column, `last_edited_by_editor_id`, created in
alphagov/publishing-api#2919. This broke the
data pipeline, because the BigQuery schema of that table rejected the
new column.

The new column contains sensitive data, so it is only included in tables
in the `publishing-api` and `private` datasets, which most users cannot
access. It is explicitly omitted from the query that puts the data into
more accessible datasets.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants