fix(sharing): correct the item sharing logic to reflect what the api …

…actually allows

Correct groupMembership enum to include the actual non-member value of 'none'
Update sharing so
group members can share items they don't own to groups they belong to
Update sharing so that only
owners can share/unshare items from shared edit groups

AFFECTS PACKAGES:
@esri/arcgis-rest-portal
@esri/arcgis-rest-types

packages/arcgis-rest-portal/src/sharing/group-sharing.ts

@@ -72,7 +72,8 @@ function changeGroupSharing(
   requestOptions: IGroupSharingUnsharingOptions
 ): Promise<ISharingResponse> {
   const username = requestOptions.authentication.username;
-  const owner = requestOptions.owner || username;
+  const itemOwner = requestOptions.owner || username;
+  const isSharedEditingGroup = requestOptions.confirmItemControl || false;
 
   return isOrgAdmin(requestOptions).then(admin => {
     const resultProp =
@@ -95,57 +96,50 @@ function changeGroupSharing(
         // next check to ensure the user is a member of the group
         return getUserMembership(requestOptions)
           .then(membership => {
-            if (membership === "nonmember") {
+            // if user is not a member of the group and not an admin
+            if (membership === "none" && !admin) {
               // abort and reject promise
               throw Error(
-                `This item can not be ${
-                  requestOptions.action
-                }d by ${username} as they are not a member of the specified group ${
-                  requestOptions.groupId
-                }.`
+                `This item can not be ${requestOptions.action}d by ${username} as they are not a member of the specified group ${requestOptions.groupId}.`
               );
             } else {
-              // if owner (and member of group) share using the owner url
-              if (owner === username) {
-                return `${getPortalUrl(
+              // they are some level of member or org-admin
+              // but only item owners can share/unshare items w/ shared editing groups
+              if (isSharedEditingGroup && itemOwner !== username) {
+                throw Error(
+                  `This item can not be ${requestOptions.action}d to shared editing group ${requestOptions.groupId} by ${username} as they not the item owner.`
+                );
+              }
+              // at this point, the user should be able to share the item to the group
+              // only question is what url to use
+
+              // default to the non-owner url...
+              let url = `${getPortalUrl(requestOptions)}/content/items/${
+                requestOptions.id
+              }/${requestOptions.action}`;
+
+              // but if they are the owner, we use a different path...
+              if (itemOwner === username) {
+                url = `${getPortalUrl(
                   requestOptions
-                )}/content/users/${owner}/items/${requestOptions.id}/${
+                )}/content/users/${itemOwner}/items/${requestOptions.id}/${
                   requestOptions.action
                 }`;
-              } else {
-                // if org admin, group admin/owner, use the bare item url
-                if (membership === "admin" || membership === "owner" || admin) {
-                  return `${getPortalUrl(requestOptions)}/content/items/${
-                    requestOptions.id
-                  }/${requestOptions.action}`;
-                } else {
-                  // otherwise abort
-                  throw Error(
-                    `This item can not be ${
-                      requestOptions.action
-                    }d by ${username} as they are neither the owner, a groupAdmin of ${
-                      requestOptions.groupId
-                    }, nor an org_admin.`
-                  );
-                }
               }
+
+              // now its finally time to do the sharing
+              requestOptions.params = {
+                groups: requestOptions.groupId,
+                confirmItemControl: requestOptions.confirmItemControl
+              };
+
+              return request(url, requestOptions);
             }
           })
-          .then(url => {
-            // now its finally time to do the sharing
-            requestOptions.params = {
-              groups: requestOptions.groupId,
-              confirmItemControl: requestOptions.confirmItemControl
-            };
-            // dont mixin to ensure that old query parameters from the search request arent included
-            return request(url, requestOptions);
-          })
           .then(sharingResponse => {
             if (sharingResponse[resultProp].length) {
               throw Error(
-                `Item ${requestOptions.id} could not be ${
-                  requestOptions.action
-                }d to group ${requestOptions.groupId}.`
+                `Item ${requestOptions.id} could not be ${requestOptions.action}d to group ${requestOptions.groupId}.`
               );
             } else {
               // all is well

packages/arcgis-rest-portal/src/sharing/helpers.ts

@@ -71,7 +71,7 @@ export function getUserMembership(
       return group.userMembership.memberType;
     })
     .catch(() => {
-      return "nonmember" as GroupMembership;
+      return "none" as GroupMembership;
     });
 }
 

packages/arcgis-rest-portal/test/mocks/items/item.ts

@@ -113,7 +113,7 @@ export const ItemGroupResponse: IGetItemGroupsResponse = {
       access: "public",
       userMembership: {
         username: "jsmith",
-        memberType: "nonmember"
+        memberType: "none"
       },
       isFav: false,
       autoJoin: false,

packages/arcgis-rest-portal/test/mocks/users/user.ts

@@ -97,7 +97,7 @@ export const GroupNonMemberUserResponse: IUser = {
       access: "org",
       userMembership: {
         username: "jsmith",
-        memberType: "member",
+        memberType: "none",
         applications: 0
       },
       protected: false,

packages/arcgis-rest-portal/test/sharing/group-sharing.test.ts

@@ -94,7 +94,7 @@ describe("shareItemWithGroup() ::", () => {
     fetchMock.post("https://myorg.maps.arcgis.com/sharing/rest/generateToken", {
       token: "fake-token",
       expires: TOMORROW.getTime(),
-      username: " jsmith"
+      username: "jsmith"
     });
 
     // make sure session doesnt cache metadata
@@ -210,7 +210,6 @@ describe("shareItemWithGroup() ::", () => {
       "https://myorg.maps.arcgis.com/sharing/rest/content/items/n3v/share",
       SharingResponse
     );
-
     shareItemWithGroup({
       authentication: MOCK_USER_SESSION,
       id: "n3v",
@@ -238,7 +237,7 @@ describe("shareItemWithGroup() ::", () => {
       });
   });
 
-  it("should share an item with a group by org administrator and pass through confirmItemControl", done => {
+  it("should fail share an item with a group by org administrator and pass through confirmItemControl", done => {
     fetchMock.once(
       "https://myorg.maps.arcgis.com/sharing/rest/community/users/jsmith?f=json&token=fake-token",
       OrgAdminUserResponse
@@ -255,38 +254,21 @@ describe("shareItemWithGroup() ::", () => {
       GroupOwnerResponse
     );
 
-    fetchMock.once(
-      "https://myorg.maps.arcgis.com/sharing/rest/content/items/n3v/share",
-      SharingResponse
-    );
-
     shareItemWithGroup({
       authentication: MOCK_USER_SESSION,
       id: "n3v",
       groupId: "t6b",
       owner: "casey",
       confirmItemControl: true
-    })
-      .then(response => {
-        expect(fetchMock.done()).toBeTruthy(
-          "All fetchMocks should have been called"
-        );
-        const [url, options]: [string, RequestInit] = fetchMock.lastCall(
-          "https://myorg.maps.arcgis.com/sharing/rest/content/items/n3v/share"
-        );
-        expect(url).toBe(
-          "https://myorg.maps.arcgis.com/sharing/rest/content/items/n3v/share"
-        );
-        expect(options.method).toBe("POST");
-        expect(response).toEqual(SharingResponse);
-        expect(options.body).toContain("f=json");
-        expect(options.body).toContain("groups=t6b");
-        expect(options.body).toContain("confirmItemControl=true");
-        done();
-      })
-      .catch(e => {
-        fail(e);
-      });
+    }).catch(e => {
+      expect(fetchMock.done()).toBeTruthy(
+        "All fetchMocks should have been called"
+      );
+      expect(e.message).toBe(
+        "This item can not be shared to shared editing group t6b by jsmith as they not the item owner."
+      );
+      done();
+    });
   });
 
   it("should share an item with a group by group owner/admin", done => {
@@ -370,7 +352,7 @@ describe("shareItemWithGroup() ::", () => {
       });
   });
 
-  it("should throw if the person trying to share doesnt own the item, is not an admin or member of said group", done => {
+  it("should allow group owner/admin/member to share item they do not own", done => {
     fetchMock.once(
       "https://myorg.maps.arcgis.com/sharing/rest/community/users/jsmith?f=json&token=fake-token",
       GroupMemberUserResponse
@@ -387,17 +369,67 @@ describe("shareItemWithGroup() ::", () => {
       GroupMemberResponse
     );
 
+    fetchMock.once(
+      "https://myorg.maps.arcgis.com/sharing/rest/content/items/n3v/share",
+      SharingResponse
+    );
+
     shareItemWithGroup({
       authentication: MOCK_USER_SESSION,
       id: "n3v",
       groupId: "t6b",
       owner: "casey"
+    })
+      .then(response => {
+        expect(fetchMock.done()).toBeTruthy(
+          "All fetchMocks should have been called"
+        );
+        const [url, options]: [string, RequestInit] = fetchMock.lastCall(
+          "https://myorg.maps.arcgis.com/sharing/rest/content/items/n3v/share"
+        );
+        expect(url).toBe(
+          "https://myorg.maps.arcgis.com/sharing/rest/content/items/n3v/share"
+        );
+        expect(options.method).toBe("POST");
+        expect(response).toEqual(SharingResponse);
+        expect(options.body).toContain("f=json");
+        expect(options.body).toContain("groups=t6b");
+        done();
+      })
+      .catch(e => {
+        fail(e);
+      });
+  });
+
+  it("should throw is non-owner tries to share to shared editing group", done => {
+    fetchMock.once(
+      "https://myorg.maps.arcgis.com/sharing/rest/community/users/jsmith?f=json&token=fake-token",
+      GroupMemberUserResponse
+    );
+
+    fetchMock.once(
+      "https://myorg.maps.arcgis.com/sharing/rest/search",
+      SearchResponse
+    );
+
+    // called when we determine if the user is a member of the group
+    fetchMock.get(
+      "https://myorg.maps.arcgis.com/sharing/rest/community/groups/t6b?f=json&token=fake-token",
+      GroupMemberResponse
+    );
+
+    shareItemWithGroup({
+      authentication: MOCK_USER_SESSION,
+      id: "n3v",
+      groupId: "t6b",
+      owner: "casey",
+      confirmItemControl: true
     }).catch(e => {
       expect(fetchMock.done()).toBeTruthy(
         "All fetchMocks should have been called"
       );
       expect(e.message).toContain(
-        "This item can not be shared by jsmith as they are neither the owner, a groupAdmin of t6b, nor an org_admin."
+        "This item can not be shared to shared editing group t6b by jsmith as they not the item owner."
       );
       done();
     });

packages/arcgis-rest-portal/test/sharing/helpers.test.ts

@@ -24,7 +24,7 @@ describe("sharing helpers ::", () => {
       })
         .then(result => {
           expect(fetchMock.done()).toBeTruthy();
-          expect(result).toBe("nonmember", "should return nonmember");
+          expect(result).toBe("none", "should return none");
           done();
         })
         .catch(e => {

packages/arcgis-rest-types/src/group.ts

@@ -8,7 +8,7 @@
  * import { GroupMembership } from "@esri/arcgis-rest-portal";
  * ```
  */
-export type GroupMembership = "owner" | "admin" | "member" | "nonmember";
+export type GroupMembership = "owner" | "admin" | "member" | "none";
 
 /**
  * A [Group](https://developers.arcgis.com/rest/users-groups-and-items/common-parameters.htm) that has not been created yet.