[Az.Resources] Fix Remove-AzRoleDefinition confirmation prompt #29076
+33
−2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status. |
- Fixed action description showing unformatted placeholder '{0}'
- Fixed target showing empty GUID when using -Name parameter
- Added distinct action descriptions for -Id and -Name scenarios
atomassi
force-pushed
the
dev/atomassilli/fix-confirm-msg
branch
from
5c66503 to
8223cdd
Compare
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug in the Remove-AzRoleDefinition cmdlet where the confirmation prompt displayed incorrect messages when using -WhatIf or -Confirm parameters.
Changes:
- Fixed the confirmation prompt to display proper action descriptions ("Removing role definition by id/name") instead of an unformatted placeholder string containing
'{0}' - Fixed the target identifier to show the actual role name when using the
-Nameparameter instead of showing an empty GUID (00000000-0000-0000-0000-000000000000) - Removed the breaking change attribute from the cmdlet class since the breaking change announcement is now properly documented in the ChangeLog
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| RemoveAzureRoleDefinitionCommand.cs | Added logic to set proper action descriptions and targets for confirmation prompts based on whether Id or Name parameter is used; removed unused using statement and breaking change attribute |
| Resources.resx | Added two new resource strings for action descriptions: "RemovingRoleDefinitionById" and "RemovingRoleDefinitionByName" |
| Resources.Designer.cs | Auto-generated code to expose the new resource strings as strongly-typed properties |
| ChangeLog.md | Added entry documenting the fix with reference to issue #29075 |
Files not reviewed (1)
- src/Resources/Resources/Properties/Resources.Designer.cs: Language not supported
Member
|
/azp run |
Contributor
|
Azure Pipelines successfully started running 3 pipeline(s). |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #29075
Description
Remove-AzRoleDefinitiondisplays incorrect confirmation message and target when using-WhatIfor-Confirm. This PR fixes two issues:'{0}'instead of a proper action description-Nameparameter, the target was showing empty GUID00000000-0000-0000-0000-000000000000instead of the role nameBefore/After Comparison
-WhatIfParameterRemove-AzRoleDefinition -Id $(New-Guid) -WhatIfWhat if: Performing the operation "Are you sure you want to remove role definition with id '{0}'." on target "02f07549-8863-43d4-beb3-17c08a6318d6".What if: Performing the operation "Removing role definition by id" on target "7a59d7f5-7fa4-4912-ad3d-c06b1fed81b1".Remove-AzRoleDefinition -Name TestRole -WhatIfWhat if: Performing the operation "Are you sure you want to remove role definition with id '{0}'." on target "00000000-0000-0000-0000-000000000000".What if: Performing the operation "Removing role definition by name" on target "TestRole".-ConfirmParameterRemove-AzRoleDefinition -Id $(New-Guid) -ConfirmConfirm: Are you sure you want to perform this action? Performing the operation "Are you sure you want to remove role definition with id '{0}'." on target "02f07549-8863-43d4-beb3-17c08a6318d6".Confirm: Are you sure you want to perform this action? Performing the operation "Removing role definition by id" on target "7a59d7f5-7fa4-4912-ad3d-c06b1fed81b1".Remove-AzRoleDefinition -Name TestRole -ConfirmConfirm: Are you sure you want to perform this action? Performing the operation "Are you sure you want to remove role definition with id '{0}'." on target "00000000-0000-0000-0000-000000000000".Confirm: Are you sure you want to perform this action? Performing the operation "Removing role definition by name" on target "TestRole".Mandatory Checklist
Please choose the target release of Azure PowerShell. (⚠️ Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.