-
Notifications
You must be signed in to change notification settings - Fork 0
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
Low rate email to admin #6
Conversation
backend/src/LeanCode.AppRating/Configuration/AdminConfiguration.cs
Outdated
Show resolved
Hide resolved
backend/src/LeanCode.AppRating/MassTransintRegistrationConfigurationExtensions.cs
Outdated
Show resolved
Hide resolved
backend/src/LeanCode.AppRating/Processes/SendEmailOnLowRateSubmitted.cs
Outdated
Show resolved
Hide resolved
backend/src/LeanCode.AppRating/Processes/SendEmailOnLowRateSubmitted.cs
Outdated
Show resolved
Hide resolved
backend/src/LeanCode.AppRating/Processes/SendEmailOnLowRateSubmitted.cs
Outdated
Show resolved
Hide resolved
@@ -62,6 +71,8 @@ public Task ExecuteAsync(HttpContext context, SubmitAppRating command) | |||
) | |||
); | |||
|
|||
publishEndpoint.Publish(new RateSubmitted<TUserId>(userId, command.Rating)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be await
ed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My bad. You're right. It should. will fix
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is why having Async
suffix is useful, one does not need to remember if every single method out there returns an awaitable or not 🙈
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't Roslyn catch this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe I configured something incorrectly. Will fix
backend/src/LeanCode.AppRating/Processes/SendEmailOnLowRateSubmitted.cs
Outdated
Show resolved
Hide resolved
backend/src/LeanCode.AppRating/EmailViewModels/LowRateSubmittedEmail.cs
Outdated
Show resolved
Hide resolved
backend/src/LeanCode.AppRating/Processes/SendEmailOnLowRateSubmitted.cs
Outdated
Show resolved
Hide resolved
backend/src/LeanCode.AppRating/Processes/SendEmailOnLowRateSubmitted.cs
Outdated
Show resolved
Hide resolved
backend/src/LeanCode.AppRating/Processes/SendEmailOnLowRateSubmitted.cs
Outdated
Show resolved
Hide resolved
feb250b
to
d931080
Compare
d931080
to
dd87fb8
Compare
string LowRatingEmailSubjectKey, | ||
string FromEmail, | ||
string[] ToEmails, | ||
string[] BccEmails |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this will ever be non-empty. :D
} | ||
} | ||
|
||
public class AppRatingConsumerDefinition<TConsumer> : ConsumerDefinition<TConsumer> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder, on one hand, we allow configuring too much (e.g. BCC, or default locale/culture), on the other, we set some things in stone and disallow any changes (consumer configuration). Where is the "ideal" line between ease of use and usability when it comes to configuring things?
No description provided.