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

DRY up new attachment code #3226

Merged
merged 1 commit into from
Jun 13, 2024
Merged

DRY up new attachment code #3226

merged 1 commit into from
Jun 13, 2024

Conversation

KludgeKML
Copy link
Contributor

  • Attachment code in Publications, Consultations, and calls for evidence was recently updated to render locally rather than using the pre-rendered HTML from whitehall
  • We move some of the duplicated code into the Content Item::Attachment concern, and delete the existing code there (which wasn't being used), and provide a utility method to simplify some of the unavoidably repeated code.
  • Coverage slightly improved (presumable due to the deleted unused method).

https://trello.com/c/tcTN1jbu/28-update-rendering-of-govspeak-attachments

⚠️ This repo is Continuously Deployed: make sure you follow the guidance ⚠️

Follow these steps if you are doing a Rails upgrade.

- Attachment code in Publications, Consultations, and
  calls for evidence was recently updated to render locally
  rather than using the pre-rendered HTML from whitehall
- We move some of the duplicated code into the Content
  Item::Attachment concern, and delete the existing code
  there (which wasn't being used), and provide a utility
  method to simplify some of the unavoidably repeated
  code.
- Coverage slightly improved (presumable due to the deleted
  unused method).
@KludgeKML KludgeKML requested a review from andysellick June 13, 2024 10:24
@govuk-ci govuk-ci temporarily deployed to government-frontend-pr-3226 June 13, 2024 10:24 Inactive
Copy link
Contributor

@andysellick andysellick left a comment

Choose a reason for hiding this comment

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

Excellent stuff 👏

@KludgeKML KludgeKML merged commit 5ce8939 into main Jun 13, 2024
12 checks passed
@KludgeKML KludgeKML deleted the dry-attachment-code branch June 13, 2024 10:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants