Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

{DenyReason} doesn't resolve properly in email notification body after request is denied #4409

Closed
harrv opened this issue Nov 19, 2021 · 5 comments

Comments

@harrv
Copy link

harrv commented Nov 19, 2021

Describe the bug / steps to reproduce

There are a number of tokens or replacement variables that can be used in notification templates as documented here. All of the ones that I have tried have worked, with the exception of {DenyReason}.

I only use email notification, and in my "Request Declined" email template I have the following (in the body):

JustWatch reports that it is currently available to stream on the following service(s): {DenyReason}

When I deny the request, I set the reason to: Netflix

In the email notification that denying the request triggers, the section above looks like this:

JustWatch reports that it is currently available to stream on the following service(s):

What do you expect to happen?

I expect it to look like this:

JustWatch reports that it is currently available to stream on the following service(s): Netflix

When I was looking through the code I saw that there are some places/variables where it is called DenyReason and others where it is called DeniedReason. So, I actually tried {DeniedReason} in my template, and got this:

JustWatch reports that it is currently available to stream on the following service(s): {DeniedReason}

Note that the token wasn't replaced at all in that case, unlike above where I used {DenyReason} which resolved to an empty string. So, I think I was correct to use {DenyReason} as documented but something is going wrong when you are preparing the email and replacing the tokens and you fail to retrieve and substitute the reason properly.

Screenshots

image

image

One interesting thing is that in the requests list it clearly says that the request status is "Denied" but on the Details page in the left column it contradicts that and says that the status is "Pending Approval". But in that left column it does show the correct "Denied Reason" (Netflix).
image

image

Desktop

  • OS: If you're asking about the client OS, I doubt that's the issue because I can reproduce this by denying the request using Firefox on Windows 10 and also programmatically through the API endpoint PUT /Request/movie/deny with body {"id":283,"reason":"Netflix"}. Both methods set the request status to denied, and the reason shows up properly in the UI and in the database, but in the email (that denying the request triggers) the reason is blank.

Ombi Version

  • Version 4.3.3; docker image ghcr.io/linuxserver/ombi:latest
  • Media Server: Emby Server 4.6.3
  • Database Type: MySQL; docker image ghcr.io/linuxserver/mariadb
@github-actions
Copy link

Hello, Please use the Github template to report an issue. If this is a feature request, please take a look at the readme.
Thanks,
Ombi Bot

@harrv
Copy link
Author

harrv commented Nov 19, 2021

@tidusjar Hi Jamie. I'm pretty sure I got all the necessary info in there except perhaps a relevant log snippet which I couldn't find. Which bits of the Github template can't be modified? The bot closed this. Can it be reopened or do I need to resubmit? Thanks.

@tidusjar tidusjar reopened this Nov 19, 2021
@tidusjar
Copy link
Member

It's all good. You added plenty of info so thank you for that!

@sephrat
Copy link
Contributor

sephrat commented Dec 8, 2021

I tried to reproduce it. Out of 4 tries, it worked twice and the variable {DenyReason} was empty the 2 other times.
I'm guessing the email is sent asynchronously and sometimes, the deny reason is not yet saved in the database when the reason is fetched from the database.

@tidusjar
Copy link
Member

tidusjar commented Dec 8, 2021

Yeah I think you are right, it seems that we schedule the notification before updating the database. So it's totally possible to sometimes work. The fix would be just to call the Notification API after we have saved the record

tidusjar added a commit that referenced this issue Dec 11, 2021
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

3 participants