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

Add multi-byline fields #27698

Merged
merged 1 commit into from
Jan 10, 2025
Merged

Add multi-byline fields #27698

merged 1 commit into from
Jan 10, 2025

Conversation

simonbyford
Copy link
Contributor

@simonbyford simonbyford commented Jan 7, 2025

What does this change?

This adds support for the new fields included in Multi-byline fields.

These fields have been available content-api-client since v33.0.0.

dotcom-rendering is already able to render these fields, since guardian/dotcom-rendering#12496

Screenshots

Before After
Screenshot 2025-01-09 at 12 14 53 Screenshot 2025-01-09 at 12 14 45

@SiAdcock
Copy link
Contributor

SiAdcock commented Jan 8, 2025

@simonbyford I think we'll need to bump content-api-scala-client in facia-scala-client, and then bump facia-scala-client here, to get the build to pass.

Who knew dependency cascades weren't just a JavaScript problem 😄

I've raised a PR: guardian/facia-scala-client#341

@simonbyford simonbyford force-pushed the add-multi-byline-fields branch 2 times, most recently from 37c7cc2 to 0d7a688 Compare January 8, 2025 13:58
@rtyley
Copy link
Member

rtyley commented Jan 9, 2025

Ah hello! Sorry I missed this PR, I think I may have duplicated your work a bit as I've opened #27705 - actually that PR is only bumping CAPI & FAPI client versions, not adding the additonal code you've added around reading the multi-author bylines in PageElement.scala.

We might want to stack this PR on top of #27705, what do you think?

@simonbyford
Copy link
Contributor Author

Ah hello! Sorry I missed this PR, I think I may have duplicated your work a bit as I've opened #27705 - actually that PR is only bumping CAPI & FAPI client versions, not adding the additonal code you've added around reading the multi-author bylines in PageElement.scala.

We might want to stack this PR on top of #27705, what do you think?

I don't mind! Happy to keep them separate if yours is likely to be merged soon? Then I can just rebase

@simonbyford simonbyford force-pushed the add-multi-byline-fields branch from 0d7a688 to ab56383 Compare January 9, 2025 12:25
@simonbyford simonbyford merged commit a9ed44d into main Jan 10, 2025
6 checks passed
@simonbyford simonbyford deleted the add-multi-byline-fields branch January 10, 2025 08:18
@prout-bot
Copy link
Collaborator

Seen on FRONTS-PROD, ADMIN-PROD (merged by @simonbyford 1 hour, 42 minutes and 13 seconds ago)

@SiAdcock SiAdcock linked an issue Jan 10, 2025 that may be closed by this pull request
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.

Upgrade CAPI client in frontend
4 participants