Fix roleassignment properties issue #4906

darshanhs90 · 2017-11-01T20:28:17Z

Related to https://icm.ad.msft.net/imp/v3/incidents/details/49435437/home
caused by dcf1326#diff-e02485bad168a96ed85b20165bc08220

Description

This checklist is used to make sure that common guidelines for a pull request are followed. You can find a more complete discussion of PowerShell cmdlet best practices here.

I have read the contribution guidelines.
If changes were made to any cmdlet, the XML help was regenerated using the platyPSHelp module.
If any large changes are made to a service, they are reflected in the respective change log.

General Guidelines

Title of the pull request is clear and informative.
There are a small number of commits, each of which have an informative message. This means that previously merged commits do not appear in the history of the PR. For more information on cleaning up the commits in your PR, see this page.
The pull request does not introduce breaking changes (unless a major version change occurs in the assembly and module).

Testing Guidelines

Pull request includes test coverage for the included changes.
PowerShell scripts used in tests should do any necessary setup as part of the test or suite setup, and should not use hard-coded values for locations or existing resources.

Cmdlet Signature Guidelines

New cmdlets that make changes or have side effects should implement ShouldProcess and have SupportShouldProcess=true specified in the cmdlet attribute. You can find more information on ShouldProcess here.
Cmdlet specifies OutputType attribute if any output is produced - if the cmdlet produces no output, it should implement a PassThru parameter.

Cmdlet Parameter Guidelines

Parameter types should not expose types from the management library - complex parameter types should be defined in the module.
Complex parameter types are discouraged - a parameter type should be simple types as often as possible. If complex types are used, they should be shallow and easily creatable from a constructor or another cmdlet.
Cmdlet parameter sets should be mutually exclusive - each parameter set must have at least one mandatory parameter not in other parameter sets.

…nto preview

cormacpayne

@darshanhs90 two minor comments

Also, please update the Resources change log to include a snippet about the changes you are making in this PR

cormacpayne · 2017-11-02T03:59:41Z

src/ResourceManager/Resources/Commands.Resources/Models.Authorization/AuthorizationClient.cs

@@ -224,8 +224,13 @@ public List<PSRoleAssignment> FilterRoleAssignments(FilterRoleAssignmentsOptions

                var tempResult = AuthorizationManagementClient.RoleAssignments.List(
                    new Rest.Azure.OData.ODataQuery<RoleAssignmentFilter>(f => f.PrincipalId == principalId));
-                result.AddRange(tempResult.FilterRoleAssignmentsOnRoleId(AuthorizationHelper.ConstructFullyQualifiedRoleDefinitionIdFromScopeAndIdAsGuid(currentSubscription, options.RoleDefinitionId))
+                if (!string.IsNullOrEmpty(options.Scope)) {


@darshanhs90 nit: please put each opening and closing braces on lines by themselves #Resolved

cormacpayne · 2017-11-02T04:00:11Z

src/ResourceManager/Resources/Commands.Resources/Models.Authorization/AuthorizationClient.cs

@@ -224,8 +224,13 @@ public List<PSRoleAssignment> FilterRoleAssignments(FilterRoleAssignmentsOptions

                var tempResult = AuthorizationManagementClient.RoleAssignments.List(
                    new Rest.Azure.OData.ODataQuery<RoleAssignmentFilter>(f => f.PrincipalId == principalId));
-                result.AddRange(tempResult.FilterRoleAssignmentsOnRoleId(AuthorizationHelper.ConstructFullyQualifiedRoleDefinitionIdFromScopeAndIdAsGuid(currentSubscription, options.RoleDefinitionId))
+                if (!string.IsNullOrEmpty(options.Scope)) {
+                    result.AddRange(tempResult.FilterRoleAssignmentsOnRoleId(AuthorizationHelper.ConstructFullyQualifiedRoleDefinitionIdFromScopeAndIdAsGuid(currentSubscription, options.RoleDefinitionId))
                    .ToPSRoleAssignments(this, ActiveDirectoryClient, options.Scope, options.ExcludeAssignmentsForDeletedPrincipals));


@darshanhs90 nit: please indent this line (and the other .ToPsRoleAssignments(...) line below that you added) #Resolved

changelog summary added

markcowl · 2017-11-02T22:37:14Z

on demand run here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/131/

markcowl · 2017-11-02T23:47:59Z

@azuresdkci add to whitelist

markcowl · 2017-11-06T16:16:28Z

@darshanhs90 It looks like one of your tests (Microsoft.Azure.Commands.Resources.Test.ScenarioTests.RoleAssignmentTests.RaPropertiesValidation) is failing - it might need to be re-recorded.

markcowl

It looks like one of your tests is failign (Microsoft.Azure.Commands.Resources.Test.ScenarioTests.RoleAssignmentTests.RaPropertiesValidation) it might need to be re-recorded

markcowl · 2017-11-14T18:48:24Z

on demand run here: https://azuresdkci.westus2.cloudapp.azure.com/view/PowerShell/job/powershell-demand/173/

darshanhs90 added 2 commits November 1, 2017 13:24

fix roleassignment issues

d0c0544

Merge branch 'preview' of https://github.com/Azure/azure-powershell i…

8a1845f

…nto preview

azuresdkci added the needs-review label Nov 1, 2017

azuresdkci assigned shahabhijeet Nov 1, 2017

update session records

8a8a53b

darshanhs90 requested a review from cormacpayne November 1, 2017 22:28

cormacpayne previously requested changes Nov 2, 2017

View reviewed changes

cormacpayne added needs-revision and removed needs-review labels Nov 2, 2017

cormacpayne assigned darshanhs90 and unassigned shahabhijeet Nov 2, 2017

update as per the comments

7b1ac09

darshanhs90 added needs-review and removed needs-revision labels Nov 2, 2017

markcowl approved these changes Nov 2, 2017

View reviewed changes

markcowl added Waiting for On Demand and removed needs-review labels Nov 2, 2017

markcowl unassigned darshanhs90 Nov 2, 2017

markcowl changed the base branch from preview to release-5.0.0 November 2, 2017 23:25

markcowl requested changes Nov 6, 2017

View reviewed changes

markcowl added needs-revision and removed Waiting for On Demand labels Nov 6, 2017

markcowl assigned darshanhs90 Nov 6, 2017

darshanhs90 added 2 commits November 13, 2017 10:08

update the session records

e0fbf5a

update test

22cff07

darshanhs90 added the needs-review label Nov 13, 2017

darshanhs90 removed the needs-revision label Nov 13, 2017

cormacpayne changed the base branch from release-5.0.0 to preview November 14, 2017 16:33

markcowl approved these changes Nov 14, 2017

View reviewed changes

markcowl added Waiting for On Demand and removed needs-review labels Nov 14, 2017

markcowl merged commit 685ceb0 into Azure:preview Nov 14, 2017

darshanhs90 mentioned this pull request Nov 25, 2017

Get-AzureRmRoleAssignment not returning names of custom roles #4564

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix roleassignment properties issue #4906

Fix roleassignment properties issue #4906

Uh oh!

darshanhs90 commented Nov 1, 2017 •

edited

Loading

Uh oh!

cormacpayne left a comment

Uh oh!

cormacpayne Nov 2, 2017 •

edited by darshanhs90

Loading

Uh oh!

cormacpayne Nov 2, 2017 •

edited by darshanhs90

Loading

Uh oh!

markcowl commented Nov 2, 2017

Uh oh!

markcowl commented Nov 2, 2017

Uh oh!

markcowl commented Nov 6, 2017

Uh oh!

markcowl left a comment

Uh oh!

markcowl commented Nov 14, 2017

Uh oh!

Uh oh!

Fix roleassignment properties issue #4906

Fix roleassignment properties issue #4906

Uh oh!

Conversation

darshanhs90 commented Nov 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

General Guidelines

Testing Guidelines

Cmdlet Signature Guidelines

Cmdlet Parameter Guidelines

Uh oh!

cormacpayne left a comment

Choose a reason for hiding this comment

Uh oh!

cormacpayne Nov 2, 2017 • edited by darshanhs90 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cormacpayne Nov 2, 2017 • edited by darshanhs90 Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

markcowl commented Nov 2, 2017

Uh oh!

markcowl commented Nov 2, 2017

Uh oh!

markcowl commented Nov 6, 2017

Uh oh!

markcowl left a comment

Choose a reason for hiding this comment

Uh oh!

markcowl commented Nov 14, 2017

Uh oh!

Uh oh!

darshanhs90 commented Nov 1, 2017 •

edited

Loading

cormacpayne Nov 2, 2017 •

edited by darshanhs90

Loading

cormacpayne Nov 2, 2017 •

edited by darshanhs90

Loading