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

Fix for issue #1844 - Email sent when removing package owner #2878

Conversation

AlexBHarley
Copy link
Contributor

This PR fixes issue #1844 by sending an email to the package owner when being removed from a package.

I have taken some liberties as I wasn't sure how to phrase the message - feedback on the code and the message would be much appreciated.

@dnfclas
Copy link

dnfclas commented Feb 9, 2016

Hi @AlexBHarley, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, DNFBOT;

_packageService.RemovePackageOwner(package, user);

var currentUser = _userService.FindByUsername(HttpContext.User.Identity.Name);
_messageService.SendPackageOwnerRemovedNotice(currentUser, user, package);
Copy link
Contributor

Choose a reason for hiding this comment

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

You may want to check currentUser for null

@maartenba
Copy link
Contributor

Looks good! Added a few remarks.

Also can you try squashing your commits into one?
(tip: git rebase -i HEAD~3 and a git push --force to the current branch will solve this :))

@AlexBHarley AlexBHarley force-pushed the email-sent-to-package-owner-when-removed-alexbharley branch from 59a4f77 to 42a2e20 Compare February 9, 2016 12:32
@AlexBHarley
Copy link
Contributor Author

Fixed those few things - not sure if my check for null is correct. And squashed into a single commit.

maartenba added a commit that referenced this pull request Feb 10, 2016
…-when-removed-alexbharley

Fix for issue #1844 - Email sent when removing package owner
@maartenba maartenba merged commit 31f62d5 into NuGet:dev Feb 10, 2016
@maartenba
Copy link
Contributor

Awesome! Good to go.

@maartenba maartenba mentioned this pull request Feb 10, 2016
6 tasks
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.

3 participants