Skip to content

Update error message for content already exists case.#799

Merged
rafaelfranca merged 1 commit into
rails:mainfrom
jpgeek:improve_error_message_for_inject_failure
Feb 23, 2023
Merged

Update error message for content already exists case.#799
rafaelfranca merged 1 commit into
rails:mainfrom
jpgeek:improve_error_message_for_inject_failure

Conversation

@jpgeek

@jpgeek jpgeek commented Nov 12, 2022

Copy link
Copy Markdown
Contributor

🌈 The replace! method fails if the flag is not found OR if the text is already present. The error message only says that the flag has not been found. This is incorrect/misleading in the case where it has actually failed because the content is already present. This is pr just adds that to the error message.

Alternatively, could add a separate error message, change the return value from replace! to indicate the type of error but this seems like overkill.

@jpgeek

jpgeek commented Dec 10, 2022

Copy link
Copy Markdown
Contributor Author

I am also happy to branch the flow to a separate error message for the two cases along with a separate test if that is better.

@rafaelfranca rafaelfranca merged commit 376e141 into rails:main Feb 23, 2023
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