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

#VP-2081 add member id to identity result #416

Merged

Conversation

kostyrin
Copy link
Contributor

No description provided.

@t13ka t13ka requested review from akak1977, tatarincev and yecli May 15, 2020 02:47

namespace VirtoCommerce.Storefront.Model.Security
{
public class MemberIdIdentityResult : IdentityResult
Copy link
Contributor

Choose a reason for hiding this comment

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

I suggest defining a new type of UserActionIdentityResult instead of deriving from IdentityResult that will contain all properties from IdentityResult to preserving backward compatibility plus in addition with the new User property.

@@ -181,9 +191,9 @@ public async Task<ActionResult<IdentityResult>> RegisterUser([FromBody] Organiza
[HttpPost("invitation")]
[ValidateAntiForgeryToken]
[ProducesResponseType(401)]
public async Task<ActionResult<IdentityResult>> CreateUserInvitation([FromBody] UsersInvitation invitation)
public async Task<ActionResult<UserActionIdentityResult>> CreateUserInvitation([FromBody] UsersInvitation invitation)
Copy link
Contributor

@vc-ci vc-ci May 22, 2020

Choose a reason for hiding this comment

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

MINOR Refactor this method to reduce its Cognitive Complexity from 24 to the 15 allowed. rule

@@ -77,10 +77,10 @@ public async Task<ActionResult<User>> GetUserById(string userId)
[HttpDelete("{userId}")]
[Authorize(SecurityConstants.Permissions.CanDeleteUsers)]
[ValidateAntiForgeryToken]
public async Task<ActionResult<IdentityResult>> DeleteUser([FromRoute] string userId)
public async Task<ActionResult<UserActionIdentityResult>> DeleteUser([FromRoute] string userId)
{
//TODO: Authorization check
Copy link
Contributor

Choose a reason for hiding this comment

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

INFO Complete the task associated to this 'TODO' comment. rule

@@ -311,10 +321,10 @@ public async Task<ActionResult<UserSearchResult>> SearchOrganizationUsersAsync([
[HttpPost("{userId}/lock")]
[Authorize(SecurityConstants.Permissions.CanEditUsers)]
[ValidateAntiForgeryToken]
public async Task<ActionResult<IdentityResult>> LockUser([FromRoute]string userId)
public async Task<ActionResult<UserActionIdentityResult>> LockUser([FromRoute]string userId)
{
//TODO: Add authorization checks
Copy link
Contributor

Choose a reason for hiding this comment

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

INFO Complete the task associated to this 'TODO' comment. rule

@@ -334,10 +344,10 @@ public async Task<ActionResult<IdentityResult>> LockUser([FromRoute]string userI
[HttpPost("{userId}/unlock")]
[Authorize(SecurityConstants.Permissions.CanEditUsers)]
[ValidateAntiForgeryToken]
public async Task<ActionResult<IdentityResult>> UnlockUser([FromRoute] string userId)
public async Task<ActionResult<UserActionIdentityResult>> UnlockUser([FromRoute] string userId)
{
//TODO: Add authorization checks
Copy link
Contributor

Choose a reason for hiding this comment

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

INFO Complete the task associated to this 'TODO' comment. rule

akak1977
akak1977 previously approved these changes May 25, 2020
@lnetrebskii lnetrebskii changed the base branch from dev-3.0.0 to dev May 26, 2020 06:40
@lnetrebskii lnetrebskii dismissed akak1977’s stale review May 26, 2020 06:40

The base branch was changed.

@t13ka
Copy link
Contributor

t13ka commented May 29, 2020

When this pull request is going to be released?

akak1977
akak1977 previously approved these changes May 29, 2020
tatarincev
tatarincev previously approved these changes Jun 3, 2020
@akak1977 akak1977 dismissed stale reviews from tatarincev and themself via 89914be June 3, 2020 11:53
@akak1977 akak1977 force-pushed the feature/3.0.0/VP-2081-add-member-id-to-identity-result branch from 9d9b070 to 89914be Compare June 3, 2020 11:53
@akak1977 akak1977 merged commit 60fe4aa into dev Jun 3, 2020
@akak1977 akak1977 deleted the feature/3.0.0/VP-2081-add-member-id-to-identity-result branch June 3, 2020 11:54
@vc-ci
Copy link
Contributor

vc-ci commented Jun 3, 2020

SonarQube analysis reported 8 issues

  • MAJOR 1 major
  • MINOR 3 minor
  • INFO 4 info

Watch the comments in this conversation to review them.

4 extra issues

Note: The following issues were found on lines that were not modified in the pull request. Because these issues can't be reported as line comments, they are summarized here:

  1. MAJOR ApiAccountController.cs#L76: Remove this commented out code. rule
  2. MINOR ApiAccountController.cs#L35: Constructor has 8 parameters, which is greater than the 7 authorized. rule
  3. MINOR ApiAccountController.cs#L395: Refactor this method to reduce its Cognitive Complexity from 18 to the 15 allowed. rule
  4. INFO ApiAccountController.cs#L397: Complete the task associated to this 'TODO' comment. rule

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.

5 participants