-
Notifications
You must be signed in to change notification settings - Fork 7
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
Publication chain for Feast fronts #1565
Conversation
8f04791
to
cc29750
Compare
9c0db8b
to
2c92681
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Testing this afternoon – a few comments on the code as I do that!
Tested in CODE, I can see publish messages on the queue 🎉 Before we merge: there's an odd error when editing collections in Editions App issues: S3 is spitting out errors at preview, giving 'request too big'. I've deployed |
S3 errors now appear to be fixed |
9129460
to
26d41ec
Compare
…se them during tests
Co-authored-by: Jonathon Herbert <jonathon.herbert@guardian.co.uk>
…says it does not support proofing
…now where to publish it
5e4196c
to
d79db1b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested proofing and publishing again against Editions in CODE, LGTM 👍
Note Requires https://github.com/guardian/editorial-tools-platform/pull/770 as well
What's changed?
Previously, the Editions workflow only targeted one product - the Editions app. This meant outputting "standard" issue fronts into S3 from where they are picked up by the Editions app backend.
Now, we are extending the UI to use the same process for the Feast app. This has a different requirement; the front must have its format converted into the form that the Feast app expects and it needs to be sent over to the Feast app backend via SNS.
Another difference is in "proofing". This is an integral part of the Editions publication process, but is not (currently) supported by the Feast app. In-app support is necessary for it to work. An extra flag has therefore been added to the Edition object,
requiresProofing
, which tells both frontend and backend if the Proofing stage is required.true
(always the case for Editions app; the logic here sets the flag totrue
if the template is defined in the list of Editions app templates) then the UI shows theProof
button, proofing is required to publish and the Publish action publishes the last proofed version.false
(currently true for non-Editions app templates) then the UI does not show theProof
button and the proofing process is short-circuited to directly output the current state of the Edition.This PR makes the relevant changes, by extracting the generic parts of the publication chain into Traits, which are then extended by concrete implementations for both Editions and Feast.
How will this data be used?
See guardian/recipes-backend#36
How to test in CODE
All Recipes
andMeat-Free
frontsCheck
to ensure that it's all ready to goPublish
. Make a mental note of the time. You should see the status isStarted
(currently this won't change as the feedback loop is not present)feast-facia-test
. This is subscribed to the SNS topic that the app writes toSend and Receive Messages
Poll for messages
Message
field. Something like this:Note the Feast-style formatting, article internal IDs in place of recipe IDs
Implementation notes
EditionsPublishing
class (containing generic publication code) has been renamed to simplyPublishing
EditionsPublishing
used to directly referenceEditionsBucket
instances for pushing to Preview and Live locations.EditionsBucket
now inherits from aPublicationTarget
trait, in common with the newFeastPublicationTarget
implementationPublicationTargetWithTransform
layer is also put in, that represents any potential publication target which needs to have the data converted from the Fronts format into 'something' elseHeld over for future PRs
String
(which has been JSON-encoded by the caller). Changing this is out of scope for this PR, however.Checklist
General
Client
404 accessing https://fronts.code.dev-gutools.co.uk/undefined
. This is also present on main branch so leaving it out of scope here.