Wording update and display correct message in Select Destination onboarding step

abrilgzz · danielluo-msft · commit bd92d0a8c0c4 · 2024-05-15T16:49:48.000-07:00
diff --git a/Service/GroupMembershipManagement/Hosts/WebApi/Services.Messages/Requests/GetGroupOnboardingStatusRequest.cs b/Service/GroupMembershipManagement/Hosts/WebApi/Services.Messages/Requests/GetGroupOnboardingStatusRequest.cs
@@ -8,5 +8,12 @@ namespace Services.Messages.Requests
     public class GetGroupOnboardingStatusRequest : RequestBase
     {
         public Guid GroupId { get; set; }
+        public string UserIdentity { get; }
+
+        public GetGroupOnboardingStatusRequest(Guid groupId, string userIdentity)
+        {
+            GroupId = groupId;
+            UserIdentity = userIdentity;
+        }
     }
 }
diff --git a/Service/GroupMembershipManagement/Hosts/WebApi/Services.WebApi/GetGroupOnboardingStatusHandler.cs b/Service/GroupMembershipManagement/Hosts/WebApi/Services.WebApi/GetGroupOnboardingStatusHandler.cs
@@ -29,6 +29,7 @@ public GetGroupOnboardingStatusHandler(ILoggingRepository loggingRepository,
         protected override async Task<GetGroupOnboardingStatusResponse> ExecuteCoreAsync(GetGroupOnboardingStatusRequest request)
         {
             var isAppIdOwner = await _graphGroupRepository.IsAppIDOwnerOfGroup(_gmmAppId, request.GroupId);
+            var isUserOwner = await _graphGroupRepository.IsEmailRecipientOwnerOfGroupAsync(request.UserIdentity, request.GroupId);
             var syncJobExists = await _syncJobRepository.GetSyncJobByObjectIdAsync(request.GroupId);
             bool isOnboarded = syncJobExists != null;
 
@@ -38,13 +39,17 @@ protected override async Task<GetGroupOnboardingStatusResponse> ExecuteCoreAsync
             {
                 response.Status = OnboardingStatus.Onboarded;
             }
-            else if (isAppIdOwner)
+            else if (!isAppIdOwner)
             {
-                response.Status = OnboardingStatus.ReadyForOnboarding;
+                response.Status = OnboardingStatus.AppIdNotOwner;
+            }
+            else if (!isUserOwner)
+            {
+                response.Status = OnboardingStatus.UserNotOwner;
             }
             else
             {
-                response.Status = OnboardingStatus.NotReadyForOnboarding;
+                response.Status = OnboardingStatus.ReadyForOnboarding;
             }
 
             return response;
diff --git a/Service/GroupMembershipManagement/Hosts/WebApi/WebApi.Tests/DestinationsControllerTests.cs b/Service/GroupMembershipManagement/Hosts/WebApi/WebApi.Tests/DestinationsControllerTests.cs
@@ -10,6 +10,8 @@
 using Models;
 using Microsoft.Extensions.Options;
 using Common.DependencyInjection;
+using System.Security.Claims;
+using WebApi.Models;
 
 namespace Services.Tests
 {
@@ -55,10 +57,12 @@ public void Initialize()
 
             _destinationController = new DestinationController(_searchDestinationsHandler, _getGroupEndpointsHandler, _getGroupOnboardingStatusHandler)
             {
-                ControllerContext = new ControllerContext
+                ControllerContext = CreateControllerContext(new List<Claim>
                 {
-                    HttpContext = _context
-                }
+                    new Claim(ClaimTypes.Name, "user@domain.com"),
+                    new Claim(ClaimTypes.Role, Roles.JOB_CREATOR),
+                    new Claim("http://schemas.microsoft.com/identity/claims/objectidentifier", Guid.NewGuid().ToString())
+                })
             };
 
             _groupTypes = new List<string>
@@ -149,6 +153,7 @@ public async Task GetGroupReadyForOnboardingStatusAsync()
             Guid groupNotOnboarded = Guid.NewGuid();
             _syncJobRepository.Setup(x => x.GetSyncJobByObjectIdAsync(It.IsAny<Guid>())).ReturnsAsync((SyncJob)null);
             _graphGroupRepository.Setup(x => x.IsAppIDOwnerOfGroup(It.IsAny<string>(), It.Is<Guid>(g => g == groupNotOnboarded))).ReturnsAsync(true);
+            _graphGroupRepository.Setup(x => x.IsEmailRecipientOwnerOfGroupAsync(It.IsAny<string>(), It.Is<Guid>(g => g == groupNotOnboarded))).ReturnsAsync(true);
 
             var response = await _destinationController.GetGroupOnboardingStatusAsync(groupNotOnboarded);
             var result = response.Result as OkObjectResult;
@@ -161,6 +166,26 @@ public async Task GetGroupReadyForOnboardingStatusAsync()
             Assert.IsNotNull(onboardingStatus);
             Assert.AreEqual(OnboardingStatus.ReadyForOnboarding, onboardingStatus);
         }
+
+        private ControllerContext CreateControllerContext(HttpContext httpContext)
+        {
+            return new ControllerContext { HttpContext = httpContext };
+        }
+
+        private ControllerContext CreateControllerContext(List<Claim> claims)
+        {
+            return new ControllerContext { HttpContext = CreateHttpContext(claims) };
+        }
+
+        private HttpContext CreateHttpContext(List<Claim> claims)
+        {
+            var identity = new ClaimsIdentity(claims, "TestAuthType");
+            var principal = new ClaimsPrincipal(identity);
+            var httpContext = new DefaultHttpContext();
+            httpContext.User = principal;
+
+            return httpContext;
+        }
     }
 }
 
diff --git a/Service/GroupMembershipManagement/Hosts/WebApi/WebApi/Controllers/v1/Destination/DestinationController.cs b/Service/GroupMembershipManagement/Hosts/WebApi/WebApi/Controllers/v1/Destination/DestinationController.cs
@@ -9,6 +9,7 @@
 using Services.Contracts;
 using Services.Messages.Requests;
 using Services.Messages.Responses;
+using System.Security.Claims;
 using AzureADGroup = Models.AzureADGroup;
 
 namespace WebApi.Controllers.v1.Destination
@@ -52,8 +53,24 @@ public async Task<ActionResult<List<string>>> GetGroupEndpointsAsync(Guid groupI
         [HttpGet("groups/{groupId}/onboarding-status")]
         public async Task<ActionResult<GetGroupOnboardingStatusResponse>> GetGroupOnboardingStatusAsync(Guid groupId)
         {
-            var response = await _getGroupOnboardingStatusHandler.ExecuteAsync(new GetGroupOnboardingStatusRequest { GroupId = groupId });
-            return Ok(response.Status);
+            try
+            {
+                var user = User;
+                var claimsIdentity = User.Identity as ClaimsIdentity;
+                var userId = claimsIdentity?.Claims.FirstOrDefault(c => c.Type == "http://schemas.microsoft.com/identity/claims/objectidentifier")?.Value;
+
+                if (string.IsNullOrEmpty(userId))
+                {
+                    return new ForbidResult();
+                }
+
+                var response = await _getGroupOnboardingStatusHandler.ExecuteAsync(new GetGroupOnboardingStatusRequest (groupId, userId));
+                return Ok(response.Status);
+            }
+            catch (Exception ex)
+            {
+                return Problem(statusCode: (int)System.Net.HttpStatusCode.InternalServerError, detail: $"An error occurred: ${ex}");
+            }
         }
     }
 }
diff --git a/Service/GroupMembershipManagement/Models/OnboardingStatus.cs b/Service/GroupMembershipManagement/Models/OnboardingStatus.cs
@@ -7,6 +7,7 @@ public enum OnboardingStatus
     {
         Onboarded,
         ReadyForOnboarding,
-        NotReadyForOnboarding
+        AppIdNotOwner,
+        UserNotOwner
     }
 }
diff --git a/UI/web-app/src/components/SelectDestination/SelectDestination.base.tsx b/UI/web-app/src/components/SelectDestination/SelectDestination.base.tsx
@@ -75,9 +75,15 @@ export const SelectDestinationBase: React.FunctionComponent<ISelectDestinationPr
   };
 
   const addGroupOwnerLink: string = `https://portal.azure.com/#view/Microsoft_AAD_IAM/GroupDetailsMenuBlade/~/Owners/groupId/${selectedDestination?.id}/menuId/`
-  const ownershipWarning = onboardingStatus === OnboardingStatus.NotReadyForOnboarding ? (
+  const appIdNotOwnerWarning = onboardingStatus === OnboardingStatus.AppIdNotOwner ? (
     <div className={classNames.ownershipWarning}>
-      {strings.ManageMembership.labels.ownershipWarning} <a href={addGroupOwnerLink}>{strings.ManageMembership.labels.clickHere}</a>.
+      {strings.ManageMembership.labels.appIdNotOwnerWarning} <a href={addGroupOwnerLink}>{strings.ManageMembership.labels.clickHere}</a>.
+    </div>
+  ) : null;
+
+  const userNotOwnerWarning = onboardingStatus === OnboardingStatus.UserNotOwner ? (
+    <div className={classNames.ownershipWarning}>
+      {strings.ManageMembership.labels.userNotOwnerWarning}
     </div>
   ) : null;
 
@@ -202,7 +208,8 @@ export const SelectDestinationBase: React.FunctionComponent<ISelectDestinationPr
                       Yammer
                     </ActionButton>
                   )}
-                  {ownershipWarning}
+                  {appIdNotOwnerWarning}
+                  {userNotOwnerWarning}
                   {alreadyOnboardedWarning}
                 </div>
               ) : null}
diff --git a/UI/web-app/src/models/GroupOnboardingStatus.ts b/UI/web-app/src/models/GroupOnboardingStatus.ts
@@ -8,5 +8,6 @@ export type GroupOnboardingStatus = {
 export enum OnboardingStatus {
     Onboarded,
     ReadyForOnboarding,
-    NotReadyForOnboarding
+    AppIdNotOwner,
+    UserNotOwner
 }
diff --git a/UI/web-app/src/pages/JobDetails/JobDetails.base.tsx b/UI/web-app/src/pages/JobDetails/JobDetails.base.tsx
@@ -252,7 +252,7 @@ const MembershipDetails: React.FunctionComponent<IContentProps> = (
     <div className={classNames.card}>
       <div>
         <Text className={classNames.title} block>
-          {strings.JobDetails.labels.pageTitle}
+          {strings.JobDetails.labels.pageTitle} - {props.job.targetGroupName}
         </Text>
       </div>
       {/* <div> // Hidden until feature is enabled
diff --git a/UI/web-app/src/services/localization/IStrings.ts b/UI/web-app/src/services/localization/IStrings.ts
@@ -279,7 +279,8 @@ export type IStrings = {
       noResultsFound: string;
       appsUsed: string;
       outlookWarning: string;
-      ownershipWarning: string;
+      appIdNotOwnerWarning: string;
+      userNotOwnerWarning: string;
       step2title: string;
       step2description: string;
       advancedQuery: string;
diff --git a/UI/web-app/src/services/localization/i18n/locales/en/translations.ts b/UI/web-app/src/services/localization/i18n/locales/en/translations.ts
@@ -156,7 +156,7 @@ export const strings: IStrings = {
       lastModifiedby: 'User who made the last change to this job.',
       startDate: 'Date of the onboarding of this job into GMM.',
       endDate: 'Date of the last run of this job.',
-      type: 'Sync type.',
+      type: 'The destination\'s type. Currently GMM only manages the membership of Group destinations. In the future it will also support Teams Channel membership.',
       id: 'Object ID of the destination group.',
       lastRun: 'Time of the last run of this job.',
       nextRun: 'Next run time for this job.',
@@ -182,7 +182,7 @@ export const strings: IStrings = {
     editButton: 'Edit',
   },
   JobsList: {
-    listOfMemberships: 'List of memberships',
+    listOfMemberships: 'Managed groups',
     ShimmeredDetailsList: {
       toggleSelection: 'Toggle selection',
       toggleAllSelection: 'Toggle selection for all items',
@@ -284,7 +284,8 @@ export const strings: IStrings = {
       noResultsFound: 'No results found',
       appsUsed: 'This group uses the following apps:',
       outlookWarning: 'There are important settings that should be considered before sending email to this Outlook group. Follow the instructions on your organization.',
-      ownershipWarning: 'Warning: GMM is not the owner of this group! It will not be able to manage membership for this group until you add it.',
+      appIdNotOwnerWarning: 'Warning: GMM is not the owner of this group! It will not be able to manage membership for this group until you add it.',
+      userNotOwnerWarning: 'Warning: You are not the owner of this group! You can only manage memberships with GMM for groups you own.',
       step2title: 'Step 2: Run Configuration',
       step2description: '',
       advancedQuery: 'Advanced Query',
diff --git a/UI/web-app/src/services/localization/i18n/locales/es/translations.ts b/UI/web-app/src/services/localization/i18n/locales/es/translations.ts
@@ -158,7 +158,7 @@ export const strings: IStrings = {
       lastModifiedby: 'Usuario quien hizo el último cambio a este grupo.',
       startDate: 'Fecha en la que este grupo fue integrado a GMM.',
       endDate: 'Fecha de la última sincronización de este grupo.',
-      type: 'Tipo de sincronización.',
+      type: 'El tipo de destino. Actualmente, GMM solo administra la membresía de los destinos de grupo. En el futuro, también admitirá la membresía de los canales de Teams.',
       id: 'ID de objecto del grupo destino.',
       lastRun: 'Fecha de la última sincronización de este grupo.',
       nextRun: 'Fecha de la próxima sincronización de este grupo.',
@@ -185,7 +185,7 @@ export const strings: IStrings = {
     editButton: 'Editar',
   },
   JobsList: {
-    listOfMemberships: 'Lista de membresías',
+    listOfMemberships: 'Grupos administrados',
     ShimmeredDetailsList: {
       toggleSelection: 'Alternar selección',
       toggleAllSelection: 'Alternar selecciones para todo',
@@ -288,7 +288,8 @@ export const strings: IStrings = {
       noResultsFound: 'No se encontraron resultados',
       appsUsed: 'Este grupo utiliza las siguientes aplicaciones.',
       outlookWarning: 'Hay configuraciones importantes a considerar antes de enviar correo a este grupo de Outlook. Sigue las instrucciones de tu organización.',
-      ownershipWarning: 'Atención: GMM no es dueño de este grupo Por favor agrégalo como propietrario antes de continuar.',
+      appIdNotOwnerWarning: 'Atención: GMM no es dueño de este grupo Por favor agrégalo como propietrario antes de continuar.',
+      userNotOwnerWarning: 'Atención: No eres dueño de este grupo. Solo los propietarios pueden administrar la membresía.',
       step2title: 'Paso 2: Configuración de ejecución ',
       step2description: '',
       advancedQuery: 'Consulta avanzada',
diff --git a/UI/web-app/src/store/jobs.api.tsx b/UI/web-app/src/store/jobs.api.tsx
@@ -49,10 +49,12 @@ export const fetchJobs = createAsyncThunk<Page<Job>, PagingOptions | undefined,
         index['estimatedNextRunTime'] =
           hoursAgo > index['period'] || index['status'] === SyncStatus.CustomerPaused
             ? ''
-            : moment.utc(index['estimatedNextRunTime']).local().format('MM/DD/YYYY') +
-              ' ' +
-              hoursLeft.toString() +
-              ' hrs left';
+            : isPast || hoursLeft === 0
+              ? 'in < 1 hr'
+              : moment.utc(index['estimatedNextRunTime']).local().format('MM/DD/YYYY') +
+                ' in ' +
+                hoursLeft.toString() +
+                ' hrs';
 
         index['enabledOrNot'] =
           index['status'] === SyncStatus.Idle || index['status'] === SyncStatus.InProgress ? true : false;

Original file line number	Diff line number	Diff line change
`@@ -8,5 +8,12 @@ namespace Services.Messages.Requests`
`8`	`8`	`public class GetGroupOnboardingStatusRequest : RequestBase`
`9`	`9`	`{`
`10`	`10`	`public Guid GroupId { get; set; }`
	`11`	`+ public string UserIdentity { get; }`
	`12`	`+`
	`13`	`+ public GetGroupOnboardingStatusRequest(Guid groupId, string userIdentity)`
	`14`	`+ {`
	`15`	`+ GroupId = groupId;`
	`16`	`+ UserIdentity = userIdentity;`
	`17`	`+ }`
`11`	`18`	`}`
`12`	`19`	`}`
Original file line number	Diff line number	Diff line change
`@@ -29,6 +29,7 @@ public GetGroupOnboardingStatusHandler(ILoggingRepository loggingRepository,`
`29`	`29`	`protected override async Task<GetGroupOnboardingStatusResponse> ExecuteCoreAsync(GetGroupOnboardingStatusRequest request)`
`30`	`30`	`{`
`31`	`31`	`var isAppIdOwner = await _graphGroupRepository.IsAppIDOwnerOfGroup(_gmmAppId, request.GroupId);`
	`32`	`+ var isUserOwner = await _graphGroupRepository.IsEmailRecipientOwnerOfGroupAsync(request.UserIdentity, request.GroupId);`
`32`	`33`	`var syncJobExists = await _syncJobRepository.GetSyncJobByObjectIdAsync(request.GroupId);`
`33`	`34`	`bool isOnboarded = syncJobExists != null;`
`34`	`35`
`@@ -38,13 +39,17 @@ protected override async Task<GetGroupOnboardingStatusResponse> ExecuteCoreAsync`
`38`	`39`	`{`
`39`	`40`	`response.Status = OnboardingStatus.Onboarded;`
`40`	`41`	`}`
`41`		`- else if (isAppIdOwner)`
	`42`	`+ else if (!isAppIdOwner)`
`42`	`43`	`{`
`43`		`- response.Status = OnboardingStatus.ReadyForOnboarding;`
	`44`	`+ response.Status = OnboardingStatus.AppIdNotOwner;`
	`45`	`+ }`
	`46`	`+ else if (!isUserOwner)`
	`47`	`+ {`
	`48`	`+ response.Status = OnboardingStatus.UserNotOwner;`
`44`	`49`	`}`
`45`	`50`	`else`
`46`	`51`	`{`
`47`		`- response.Status = OnboardingStatus.NotReadyForOnboarding;`
	`52`	`+ response.Status = OnboardingStatus.ReadyForOnboarding;`
`48`	`53`	`}`
`49`	`54`
`50`	`55`	`return response;`
Original file line number	Diff line number	Diff line change
`@@ -7,6 +7,7 @@ public enum OnboardingStatus`
`7`	`7`	`{`
`8`	`8`	`Onboarded,`
`9`	`9`	`ReadyForOnboarding,`
`10`		`- NotReadyForOnboarding`
	`10`	`+ AppIdNotOwner,`
	`11`	`+ UserNotOwner`
`11`	`12`	`}`
`12`	`13`	`}`
Original file line number	Diff line number	Diff line change
`@@ -8,5 +8,6 @@ export type GroupOnboardingStatus = {`
`8`	`8`	`export enum OnboardingStatus {`
`9`	`9`	`Onboarded,`
`10`	`10`	`ReadyForOnboarding,`
`11`		`- NotReadyForOnboarding`
	`11`	`+ AppIdNotOwner,`
	`12`	`+ UserNotOwner`
`12`	`13`	`}`