AO3-7358 Fix 500 error when sending item_added_notification for bookmark of deleted work#5804
Conversation
…of deleted work Fix 500 error sending item_added_notification for bookmark of deleted work The mail tries to reference .title of @creation.bookmarkable which errors out when @creation.bookmarkable is deleted. Updated the mail text to explicitly handle this case
|
With my changes, the email will look like: i.e the mail body is:
Please do let me know if the original intention of the ticket was to replace the entire mail body with "Bookmark of deleted item" |
|
@Bilka2 I don't have Jira perms yet, could you please set the ticket to In-Review/In-Progress? 😄 |
Bilka2
left a comment
Bilka2
left a comment
There was a problem hiding this comment.
Could you add a test for this change?
The rest of my comments are just style nitpicks, nice work!
| <i><b><%= style_link(@creation.title.html_safe, work_url(@creation)) %></b></i> | ||
| <% else %> | ||
| <% if @creation.bookmarkable == nil %> | ||
| Bookmark of deleted item |
There was a problem hiding this comment.
Normally, we'd prefer to internationalise text. But since that would significantly complicate the pull request, it's good to keep it like this for now. Just wanted to note it for the future
That said, could you indent this and the other line inside the else one step?
| <% if @creation.is_a?(Work) %> | ||
| <i><b><%= style_link(@creation.title.html_safe, work_url(@creation)) %></b></i> | ||
| <% else %> | ||
| <% if @creation.bookmarkable == nil %> |
There was a problem hiding this comment.
Another style nitpick: Could you check unless @creation.bookmarkable instead?
|
|
||
|
|
||
| <%= @creation.is_a?(Work) ? @creation.pseuds.length > 0 ? pseuds : @creation.authors_to_sort_on : pseuds %> posted a <%= @creation.is_a?(Work) ? @creation.backdate ? "backdated " : "new " : "new " %> <%= @creation.is_a?(Work) ? "work" : "bookmark" %> to your collection, "<%= @collection.name %>" (<%= collection_url(@collection) %>: | ||
| <%= @creation.is_a?(Work) ? @creation.pseuds.length > 0 ? pseuds : @creation.authors_to_sort_on : pseuds %> posted a <%= @creation.is_a?(Work) ? @creation.backdate ? "backdated " : "new " : "new " %> <%= @creation.is_a?(Work) ? "work" : "bookmark" %> to your collection, "<%= @collection.name %>" (<%= collection_url(@collection) %>): |
There was a problem hiding this comment.
Nice work spotting that missing parenthesis!
Bilka2
changed the title
3 participants
Pull Request Checklist
as the first thing in your pull request title (e.g.
AO3-1234 Fix thing)until they are reviewed and merged before creating new pull requests.
Issue
https://otwarchive.atlassian.net/browse/AO3-7358
Purpose
Fix 500 error sending
item_added_notificationfor bookmark of deleted workThe mail tries to reference
.titleof@creation.bookmarkablewhich errors out when@creation.bookmarkableis deleted. Updated the mail text to explicitly handle this caseCredit
Smaran Jawalkar (he/him)