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

Fix bug with update_lead_image method #8434

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

davidgisbey
Copy link
Contributor

Description

The 'and return if' isn't returning when the image_display_option is no_image or organisation_image.

This means that when an image is created and the lead_image isn't set the association is created.

It still wouldn't be pushed downstream to Publishing API as the presenter handles passing the image based on the image_display_option, but we shouldn't have lead images sitting in the db when there isn't a lead image.

This updates the method to explicitly call return within the conditional which resolves the issue.

Trello card

https://trello.com/c/5vCiIxeQ/1064-lead-image-handling-backend-work

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

Follow these steps if you are doing a Rails upgrade.

The 'and return if' isn't returning when the image_display_option is
no_image or organisation_image.

This means that when an image is created and the lead_image isn't set
the assoication is created.

It still wouldn't be pushed downstream to Publishing API, but we
shouldn't have lead images sitting in the db when there isn't a
lead image.

This updates the method to explicitly call return within the conditional
which resolves the issue.
Copy link
Contributor

@beccapearce beccapearce left a comment

Choose a reason for hiding this comment

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

nice one!

@davidgisbey davidgisbey merged commit 0005c56 into main Oct 31, 2023
@davidgisbey davidgisbey deleted the fix-bug-with-update-lead-image-method branch October 31, 2023 09:33
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.

2 participants