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

controllers/krate/delete: Send deletion notification email after successful deletion #10265

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Turbo87
Copy link
Member

@Turbo87 Turbo87 commented Dec 23, 2024

This PR addresses part two of #9352 (comment) by sending a notification email to the owner when a crate is getting deleted.

@Turbo87 Turbo87 added C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works A-backend ⚙️ labels Dec 23, 2024
@Turbo87 Turbo87 requested a review from a team December 23, 2024 13:34
@Turbo87 Turbo87 force-pushed the crate-deletion-email branch from a3a7a8a to 1e50d69 Compare December 25, 2024 18:54
format!(
"Hi {},

this is a confirmation email for the deletion of your \"{}\" crate.
Copy link
Contributor

Choose a reason for hiding this comment

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

Wording tweak (feel free to ignore it if you want, but in that case let's at least capitalise the first letter):

Suggested change
this is a confirmation email for the deletion of your \"{}\" crate.
Your \"{}\" crate has been deleted, per your request.

Comment on lines +121 to +132
let email_future = async {
if let Some(recipient) = user.email(&mut conn).await? {
let email = CrateDeletionEmail {
user: &user.gh_login,
krate: &crate_name,
};

app.emails.send(&recipient, email).await?
}

Ok::<_, anyhow::Error>(())
};
Copy link
Contributor

Choose a reason for hiding this comment

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

Curiosity, rather than any actual change required: was there a reason for doing this as an async block instead of an async fn?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-backend ⚙️ C-enhancement ✨ Category: Adding new behavior or a change to the way an existing feature works
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants