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

Added a new stub for attempting to delete a missing asset #1248

Merged
merged 1 commit into from Apr 11, 2024

Conversation

ryanb-gds
Copy link
Contributor

@ryanb-gds ryanb-gds commented Apr 5, 2024

This is needed for testing how publishing applications react to attempting to delete a missing asset. At the moment some of them are not handling not found errors, meaning that the state of the publishing application remains out of sync with asset manager. This causes other logical issues such as being unable to discard a draft in Specialist Publisher because there is an attachment that can't be deleted.

This commit also adds some test coverage for other asset deletion stubs.

test/test_helper.rb Outdated Show resolved Hide resolved
Copy link
Member

@brucebolt brucebolt left a comment

Choose a reason for hiding this comment

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

Code looks good to me. Could you add an entry to the Unreleased section of the CHANGELOG, since you'll need to release a new version of this gem (in another PR) to make the stub available to other applications.

This is needed for testing how publishing applications react to attempting to delete a missing asset. At the moment they are not handling not found errors, meaning that the state of the publishing application remains out of sync with asset manager and causing other logical issues.

This commit also adds some test coverage for other asset deletion stubs and makes byebug available in the testing runtime.
@ryanb-gds
Copy link
Contributor Author

Code looks good to me. Could you add an entry to the Unreleased section of the CHANGELOG, since you'll need to release a new version of this gem (in another PR) to make the stub available to other applications.

There doesn't appear to be any unreleased code currently, so I've added the section at the top, hope that's appropriate. Would this constitute a patch or minor release? I'm inclined to suggest the latter as it seems to me it's offering a new feature

@callumknights
Copy link
Contributor

I'm happy with the code here as well. I'm inclined to agree with you that this is probably a minor release, I initially misread the PR description and thought this was a bug fix but that doesn't appear to be the case.

@ryanb-gds ryanb-gds merged commit af3f7f7 into main Apr 11, 2024
44 checks passed
@ryanb-gds ryanb-gds deleted the add-delete-asset-404-stub branch April 11, 2024 09:23
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.

None yet

4 participants