fix: remind user remove aadApp/update action #13331
Conversation
nliu-ms
commented
Mar 4, 2025
nliu-ms
commented
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #13331 +/- ##
==========================================
+ Coverage 90.30% 90.31% +0.01%
==========================================
Files 609 610 +1
Lines 36221 36367 +146
Branches 7002 7084 +82
==========================================
+ Hits 32710 32846 +136
- Misses 1517 1524 +7
- Partials 1994 1997 +3
|
nliu-ms
requested review from
blackchoey,
wenytang-ms,
KennethBWSong,
timngmsft,
sffamily,
therealjohn and
supkasar
as code owners
| @@ -217,12 +221,18 @@ export class CreateAadAppDriver implements StepDriver { | |||
| if ( | |||
| error.response!.status === 403 && | |||
| message.includes(constants.insufficientPermissionErrorMessage) && | |||
| !isTestToolEnabledProject(context.projectPath) | |||
| !isTestToolEnabledProject(context.projectPath) && | |||
| process.env.TEAMSFX_ENV == environmentNameManager.getLocalEnvName() | |||
There was a problem hiding this comment.
minor: why only enable the new feature for local environment now?
There was a problem hiding this comment.
Because this feature is targeting for F5 success rate, so I'd like to limit the impact.
| logMessageKeys.needManualRemoveAadUpdate, | ||
| teamsappYamlPath | ||
| ); | ||
| await context.ui!.showMessage("warn", message, true); |
There was a problem hiding this comment.
should we try to update the manifest first before show the warning? user may already has permission to update the app
There was a problem hiding this comment.
By the way, have we reached the UX team to review the experience? We may reach them for feedback if haven't. Asking users to remove a step directly seems a little weird. We probably need to tell them why they need to remove that step and what they need to do before remove.
There was a problem hiding this comment.
should we try to update the manifest first before show the warning? user may already has permission to update the app
We go into this function, because create API respond 403 error with insufficient permission. Which means user don't have permission to update as well.
