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.

Sign up for GitHub

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 user renewals for users without former plast member role #2800

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fix user renewals for users without former plast member role #2800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45ca731
remove role checks
vsynieok Sep 6, 2022
bfeecf0
remove obsolete tests
vsynieok Sep 6, 2022
446d4db
Merge branch 'master' into fix-user-renewal
vsynieok Sep 6, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions EPlast/EPlast.BLL/Services/UserProfiles/UserRenewalService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -113,11 +113,12 @@ public async Task<CityMembersDto> RenewFormerMemberUserAsync(UserRenewalDto user
var cityId = userRenewal.CityId;
var user = await _userManager.FindByIdAsync(userId);

if (!await _userManager.IsInRoleAsync(user, Roles.FormerPlastMember))
throw new ArgumentException("User is not Former-Member", nameof(userRenewal));
if (await _userManager.IsInRoleAsync(user, Roles.FormerPlastMember))
Copy link
Contributor

Choose a reason for hiding this comment

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

throw наверняка подразумевал, что начиная с 122 строки дальше код выполняться не будет.
Твои изменения безопасны?

{
await _userManager.RemoveFromRoleAsync(user, Roles.FormerPlastMember);
await _userManager.AddToRoleAsync(user, Roles.RegisteredUser);
}

await _userManager.RemoveFromRoleAsync(user, Roles.FormerPlastMember);
await _userManager.AddToRoleAsync(user, Roles.RegisteredUser);
await ResolveUserMembershipDatesAsync(userId);
var newUser = await _cityParticipantsService.AddFollowerAsync(cityId, userId);
await ChangeUserRenewalAsync(userRenewal);
Expand Down
28 changes: 0 additions & 28 deletions EPlast/EPlast.Tests/Controllers/UserRenewalControllerTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,34 +231,6 @@ public async Task RenewUser_ReturnsBadRequestResult()
Assert.IsInstanceOf<BadRequestObjectResult>(result);
}

[Test]
public async Task RenewUser_ReturnsBadRequestResultWrongTargetRole()
{
//Arrange
_mockUserManager
.Setup(u => u.FindByIdAsync(It.IsAny<string>()))
.ReturnsAsync(new User());
_mockUserManager
.Setup(u => u.IsInRoleAsync(It.IsAny<User>(), It.IsAny<string>()))
.ReturnsAsync(false);
_mockUserRenewalService
.Setup(r => r.IsValidAdminAsync(It.IsAny<User>(), It.IsAny<int>()))
.ReturnsAsync(true);
_mockUserRenewalService
.Setup(r => r.RenewFormerMemberUserAsync(It.IsAny<UserRenewalDto>()))
.ReturnsAsync(new CityMembersDto());
_mockUserRenewalService
.Setup(r => r.SendRenewalConfirmationEmailAsync(It.IsAny<string>(), It.IsAny<int>()));

//Act
var result = await _userRenewalController.RenewUser(userRenewalDTO);

//Assert
_mockUserRenewalService.Verify();
Assert.IsNotNull(result);
Assert.IsInstanceOf<BadRequestObjectResult>(result);
}

[Test]
public async Task RenewUser_ReturnsBadRequestResultUnfortunateRenewal()
{
Expand Down
16 changes: 0 additions & 16 deletions EPlast/EPlast.Tests/Services/UserProfiles/UserRenewalServiceTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -278,22 +278,6 @@ public async Task RenewFormerMemberUser_RenewsFormerMemberAsync()
Assert.IsInstanceOf<CityMembersDto>(result);
}

[Test]
public void RenewFormerMemberUser_ThrowsArgumentException()
{
//Arrange
_mockUserManager
.Setup(u => u.FindByIdAsync(It.IsAny<string>()))
.ReturnsAsync(new User());
_mockUserManager
.Setup(u => u.IsInRoleAsync(It.IsAny<User>(), It.IsAny<string>()))
.ReturnsAsync(false);

//Assert
Assert.ThrowsAsync<ArgumentException>(
async ()=>{await _userRenewalService.RenewFormerMemberUserAsync(userRenewalDTO);});
}

[Test]
public async Task ResolveMembershipDates_ResolvesDatesAsync()
{
Expand Down
2 changes: 1 addition & 1 deletion EPlast/EPlast.WebApi/Controllers/UserRenewalController.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ public async Task<IActionResult> RenewUser(UserRenewalDto userRenewal)
var admin = await _userManager.GetUserAsync(User);
var user = await _userManager.FindByIdAsync(userId);

if (!ModelState.IsValid || user == null || !await _userManager.IsInRoleAsync(user, Roles.FormerPlastMember))
if (!ModelState.IsValid || user == null)
return BadRequest("Unable to apply the renewal");

if (!await _userRenewalService.IsValidAdminAsync(admin, cityId))
Expand Down