group membership

Author: rvowles

adks/e2e-sdk/app/steps/dashboard.ts

@@ -0,0 +1,17 @@
+import {Then, When} from "@cucumber/cucumber";
+import {SdkWorld} from "../support/world";
+import {expect} from "chai";
+
+When('I ask for the feature dashboard', async function() {
+  const world = this as SdkWorld;
+  expect(world.application.environments.length, `Application ${world.application.name} has no environments`).to.be.gt(0);
+  const data = await world.featureApi.findAllFeatureAndFeatureValuesForEnvironmentsByApplication(world.application.id, world.application.environments.map(e => e.id));
+  expect(data.status).to.eq(200);
+  world.dashboard = data.data;
+});
+
+Then('I should see an empty list of features and {int} environment', async function (envCount: number) {
+  const world = this as SdkWorld;
+  expect(world.dashboard.environments.length).to.eq(1);
+  expect(world.dashboard.features.length).to.eq(0);
+});

adks/e2e-sdk/app/steps/group.ts

@@ -22,7 +22,8 @@ When('I get the portfolio admin group', async function() {
   const groups = await world.superuser.groupApi.findGroups(world.portfolio.id);
   const adminGroup = groups.data.find(g => g.admin);
   expect(adminGroup, `${JSON.stringify(groups.data)} - could not find admin group!`).to.not.be.undefined;
-  world.group = adminGroup;
+  const fullGroup = await world.superuser.groupApi.getGroup(adminGroup.id, false, true);
+  world.group = fullGroup.data;
 });
 
 When('I create a new group with application roles {string}', async function(roles: string) {
@@ -41,6 +42,34 @@ When('I create a new group with application roles {string}', async function(role
   world.group = response.data;
 });
 
+When('I assign the superuser to the group', async function() {
+  const world = this as SdkWorld;
+  const userResponse = await world.superuser.personApi.getPerson("self");
+  const userId = (userResponse.data as Person).id.id;
+  const response = await world.superuser.groupApi.addPersonToGroup(world.group.id,
+    userId, true);
+  expect(response.status).to.eq(200);
+  expect(response.data.members.find( p => p.id.id === userId)).to.not.be.undefined;
+});
+
+Then('I can delete the supergroup from the group', async function() {
+  const world = this as SdkWorld;
+  const userResponse = await world.superuser.personApi.getPerson("self");
+  const userId = (userResponse.data as Person).id.id;
+  const response = await world.superuser.groupApi.deletePersonFromGroup(world.group.id, userId, true);
+  expect(response.status).to.eq(200);
+  expect(response.data.members.find( p => p.id.id === userId)).to.be.undefined;
+});
+
+Then(/^the (superuser|user) is marked in the group as a (superuser|user)$/, async function(userSource: string, userType: string) {
+  const world = this as SdkWorld;
+  const user = (userSource === 'superuser') ? world.superuser : world.user;
+  expect(user.me).to.not.be.undefined;
+  expect(world.group).to.not.be.undefined;
+  const suserValue = userType === 'superuser';
+  expect(world.group.sMembers.find(p => p.superuser === suserValue && p.person.id === user.personId))
+});
+
 Then('I assign the new user to the new group', async function() {
   const world = this as SdkWorld;
   expect(world.user).to.not.be.undefined;

adks/e2e-sdk/app/steps/setup.ts

@@ -1,14 +1,11 @@
 import {expect} from 'chai';
 import {Given, Then, When} from '@cucumber/cucumber';
 import {
-  Application, CreateApplication,
+  CreateApplication,
   CreatePortfolio,
   Environment,
   PortfolioServiceApi,
-  RoleType,
-  ServiceAccount,
   ServiceAccountPermission,
-  ServiceAccountServiceApi,
   UpdateEnvironment
 } from '../apis/mr-service';
 import {makeid, sleep} from '../support/random';

adks/e2e-sdk/app/support/world.ts

@@ -1,6 +1,6 @@
 import {setDefaultTimeout, setWorldConstructor, World} from '@cucumber/cucumber';
 import {
-  Application,
+  Application, ApplicationFeatureValues,
   ApplicationRolloutStrategyServiceApi,
   ApplicationServiceApi,
   AuthServiceApi,
@@ -69,6 +69,7 @@ export class ApiUser {
   public readonly applicationStrategyApi: ApplicationRolloutStrategyServiceApi;
   public readonly featureFilterApi: FeatureFilterServiceApi;
   public readonly apiKey: string;
+  public me: Person;
 
   public serviceAccounts: Array<ServiceAccount> = [];
 
@@ -80,6 +81,7 @@ export class ApiUser {
 
     this.portfolioApi = new PortfolioServiceApi(this.adminApiConfig);
     this.personApi = new PersonServiceApi(this.adminApiConfig);
+
     this.applicationApi = new ApplicationServiceApi(this.adminApiConfig);
     this.environmentApi = new EnvironmentServiceApi(this.adminApiConfig);
     this.environment2Api = new Environment2ServiceApi(this.adminApiConfig);
@@ -97,7 +99,21 @@ export class ApiUser {
     this.applicationStrategyApi = new ApplicationRolloutStrategyServiceApi(this.adminApiConfig);
     this.featureHistoryApi = new FeatureHistoryServiceApi(this.adminApiConfig);
     this.featureFilterApi = new FeatureFilterServiceApi(this.adminApiConfig);
+
+    if (apiKey) {
+      this.refreshPerson();
+    }
+  }
+
+  public refreshPerson() {
+    this.personApi.getPerson('self').then((person) => {
+      this.me = person.data;
+    }).catch((_) => {
+
+    });
   }
+
+  public get personId() { return this.me.id.id; }
 }
 
 export class SdkWorld extends World {
@@ -129,6 +145,7 @@ export class SdkWorld extends World {
   public readonly superuser: ApiUser;
   public user: ApiUser | undefined;
   public currentUser: ApiUser;
+  public dashboard: ApplicationFeatureValues | undefined;
 
   constructor(props: any) {
     super(props);
@@ -269,6 +286,8 @@ export class SdkWorld extends World {
 
   public set apiKey(val: TokenizedPerson) {
     this.adminApiConfig.accessToken = val.accessToken;
+    this.superuser.refreshPerson();
+    this.superuser.me = val.person;
     this.person = val.person;
     apiKey = val.accessToken;
     logger.info('Successfully logged in');

adks/e2e-sdk/features/permission-check.feature

@@ -0,0 +1,19 @@
+@allvariants @streamingvariants
+Feature: Certain permission combinations should work as expected
+
+  @portfolio-group
+  Scenario: The group should be able to identify who is a superuser and who is not
+    Given I create a new portfolio
+    And I get the portfolio admin group
+    And I create a new user
+    And I assign the new user to the new group
+    And I get the portfolio admin group
+    And the superuser is marked in the group as a superuser
+    And the user is marked in the group as a user
+
+  @delete-superuser-from-group
+  Scenario: I add the superuser to the portfolio admin group and then remove them
+    Given I create a new portfolio
+    And I get the portfolio admin group
+    When I assign the superuser to the group
+    Then I can delete the supergroup from the group

admin-frontend/open_admin_app/lib/routes/manage_group_route.dart

@@ -110,11 +110,13 @@ class ManageGroupRouteState extends State<ManageGroupRoute> {
               stream: bloc.groupLoaded,
               builder: (context, snapshot) {
                 if (snapshot.hasData) {
+
+                  var group = snapshot.data!;
                   return Column(
                     crossAxisAlignment: CrossAxisAlignment.start,
                     children: <Widget>[
                       if (bloc.mrClient
-                          .isPortfolioOrSuperAdmin(snapshot.data!.portfolioId))
+                          .isPortfolioOrSuperAdmin(group.portfolioId))
                         Padding(
                           padding: const EdgeInsets.all(8.0),
                           child: FilledButton.icon(
@@ -125,7 +127,7 @@ class ManageGroupRouteState extends State<ManageGroupRoute> {
                                 .addOverlay((BuildContext context) {
                               return AddMembersDialogWidget(
                                 bloc: bloc,
-                                group: snapshot.data!,
+                                group: group,
                               );
                             }),
                           ),
@@ -143,22 +145,22 @@ class ManageGroupRouteState extends State<ManageGroupRoute> {
                                   label: Text(
                                       AppLocalizations.of(context)!.columnName),
                                   onSort: (columnIndex, ascending) {
-                                    onSortColumn(snapshot.data!.members,
+                                    onSortColumn(group.sMembers,
                                         columnIndex, ascending);
                                   }),
                               DataColumn(
                                 label: Text(
                                     AppLocalizations.of(context)!.columnEmail),
                                 onSort: (columnIndex, ascending) {
-                                  onSortColumn(snapshot.data!.members,
+                                  onSortColumn(group.sMembers,
                                       columnIndex, ascending);
                                 },
                               ),
                               DataColumn(
                                 label: Text(AppLocalizations.of(context)!
                                     .columnMemberType),
                                 onSort: (columnIndex, ascending) {
-                                  onSortColumn(snapshot.data!.members,
+                                  onSortColumn(group.sMembers,
                                       columnIndex, ascending);
                                 },
                               ),
@@ -171,24 +173,22 @@ class ManageGroupRouteState extends State<ManageGroupRoute> {
                                   onSort: (i, a) => {}),
                             ],
                             rows: [
-                              for (Person member in snapshot.data!.members)
+                              for (GroupPerson member in group.sMembers)
                                 DataRow(cells: [
                                   DataCell(
-                                    Text(member.name ?? ''),
+                                    Text(member.person.name ?? ''),
                                   ),
                                   DataCell(Text(
-                                      member.personType == PersonType.person
-                                          ? member.email!
+                                      member.person.type == PersonType.person
+                                          ? member.person.email!
                                           : "")),
                                   DataCell(Text(
-                                      member.personType == PersonType.person
+                                      member.person.type == PersonType.person
                                           ? AppLocalizations.of(context)!
                                               .memberTypeUser
                                           : AppLocalizations.of(context)!
                                               .memberTypeServiceAccount)),
-                                  DataCell(bloc.mrClient
-                                          .isPortfolioOrSuperAdmin(
-                                              snapshot.data!.portfolioId)
+                                  DataCell(!member.superuser
                                       ? Tooltip(
                                           message: AppLocalizations.of(context)!
                                               .removeFromGroup,
@@ -197,14 +197,14 @@ class ManageGroupRouteState extends State<ManageGroupRoute> {
                                             onPressed: () async {
                                               try {
                                                 await bloc.removeFromGroup(
-                                                    snapshot.data!, member);
+                                                    group, member.person.id);
                                                 if (context.mounted) {
                                                   bloc.mrClient.addSnackbar(
                                                       Text(AppLocalizations.of(
                                                               context)!
                                                           .memberRemovedFromGroup(
-                                                              member.name ?? '',
-                                                              snapshot.data!
+                                                              member.person.name,
+                                                              group
                                                                   .name)));
                                                 }
                                               } catch (e, s) {
@@ -230,39 +230,39 @@ class ManageGroupRouteState extends State<ManageGroupRoute> {
     );
   }
 
-  void onSortColumn(List<Person> people, int columnIndex, bool ascending) {
+  void onSortColumn(List<GroupPerson> people, int columnIndex, bool ascending) {
     setState(() {
       if (columnIndex == 0) {
         if (ascending) {
           people.sort((a, b) {
-            return a.name!.toLowerCase().compareTo(b.name!.toLowerCase());
+            return a.person.name.toLowerCase().compareTo(b.person.name!.toLowerCase());
           });
         } else {
           people.sort((a, b) {
-            return b.name!.toLowerCase().compareTo(a.name!.toLowerCase());
+            return b.person.name.toLowerCase().compareTo(a.person.name.toLowerCase());
           });
         }
       }
       if (columnIndex == 1) {
         if (ascending) {
           people.sort((a, b) =>
-              a.email!.toLowerCase().compareTo(b.email!.toLowerCase()));
+              a.person.email!.toLowerCase().compareTo(b.person.email!.toLowerCase()));
         } else {
           people.sort((a, b) =>
-              b.email!.toLowerCase().compareTo(a.email!.toLowerCase()));
+              b.person.email!.toLowerCase().compareTo(a.person.email!.toLowerCase()));
         }
       }
       if (columnIndex == 2) {
         if (ascending) {
-          people.sort((a, b) => a.personType
+          people.sort((a, b) => a.person.type
               .toString()
               .toLowerCase()
-              .compareTo(b.personType.toString().toLowerCase()));
+              .compareTo(b.person.type.toString().toLowerCase()));
         } else {
-          people.sort((a, b) => b.personType
+          people.sort((a, b) => b.person.type
               .toString()
               .toLowerCase()
-              .compareTo(a.personType.toString().toLowerCase()));
+              .compareTo(a.person.type.toString().toLowerCase()));
         }
       }
       if (sortColumnIndex == columnIndex) {
@@ -430,10 +430,8 @@ class _AddMembersDialogWidgetState extends State<AddMembersDialogWidget> {
               keepCase: true,
               onPressed: () async {
                 final group = widget.group;
-                group.members = List.from(group.members)..addAll(membersToAdd);
-                // remove duplicates
-                group.members = group.members.toSet().toList();
-                final success = await widget.bloc.updateGroup(group);
+
+                final success = await widget.bloc.addUsersToGroup(group, membersToAdd);
                 if (success) {
                   widget.bloc.mrClient.removeOverlay();
                   if (context.mounted) {

admin-frontend/open_admin_app/lib/widgets/group/group_bloc.dart

@@ -56,7 +56,7 @@ class GroupBloc implements Bloc {
     if (groupId != null && groupId.length > 1) {
       try {
         final fetchedGroup =
-            await _groupServiceApi.getGroup(groupId, includeMembers: true);
+            await _groupServiceApi.getGroup(groupId, includeMembersV2: true);
         // publish it out...
         group = fetchedGroup;
         _groupSource.add(fetchedGroup);
@@ -69,7 +69,7 @@ class GroupBloc implements Bloc {
   Future<void> deleteGroup(String groupId, bool includeMembers) async {
     try {
       await _groupServiceApi.deleteGroup(groupId,
-          includeMembers: includeMembers);
+          includeMembersV2: includeMembers);
       group = null;
       this.groupId = null;
       _groupSource.add(null);
@@ -79,24 +79,42 @@ class GroupBloc implements Bloc {
     }
   }
 
-  Future<void> removeFromGroup(Group group, Person person) async {
+  Future<void> removeFromGroup(Group group, String personId) async {
     var data = await _groupServiceApi
-        .deletePersonFromGroup(group.id, person.id!.id, includeMembers: true);
+        .deletePersonFromGroup(group.id, personId, includeMembersV2: true);
     if (!_groupSource.isClosed) {
       _groupSource.add(data);
     }
   }
 
-  Future<bool> updateGroup(Group groupToUpdate, {String? name}) async {
+  Future<bool> addUsersToGroup(Group group, List<Person> persons) async {
     try {
-      if (name != null) {
-        groupToUpdate.name = name;
-      }
+      final fetchedGroup = await _groupServiceApi.addPersonsToGroup(group.id, personId: persons.map((p) => p.id!.id).toList(), includeMembersV2: true);
+      group = fetchedGroup;
+      _groupSource.add(fetchedGroup);
+      return true;
+    } catch (e, s) {
+      await mrClient.dialogError(e, s,
+          messageTitle: 'Failed to add users to group',
+          messageBody:
+          'Failed to update group because of a duplicate or other conflict.');
+      return false;
+    }
+  }
+
+  Future<bool> updateGroupName(Group groupToUpdate, String name) async {
+    try {
+      groupToUpdate.name = name;
+
       final newGroup = await _groupServiceApi.updateGroupOnPortfolio(
           mrClient.currentPortfolio!.id, groupToUpdate,
-          includeMembers: true, updateMembers: true);
+          includeMembersV2: true, updateMembers: false);
+
+      // tell the portfolio groups to update as the name has changed.
       await getGroups(focusGroup: groupToUpdate);
+
       group = newGroup;
+      _groupSource.add(newGroup);
       groupId = groupToUpdate.id;
       return true;
     } catch (e, s) {

admin-frontend/open_admin_app/lib/widgets/group/group_update_widget.dart

@@ -100,7 +100,7 @@ class _GroupUpdateDialogWidgetState extends State<GroupUpdateDialogWidget> {
 
   Future<void> _callUpdateGroup(String name) {
     final groupName = name.trim();
-    return widget.group == null ? widget.bloc.createGroup(groupName) : widget.bloc.updateGroup(widget.group!, name: groupName);
+    return widget.group == null ? widget.bloc.createGroup(groupName) : widget.bloc.updateGroupName(widget.group!, groupName);
   }
 }