From 37f99b9aa68cff4265d9c99a0203f80da456276f Mon Sep 17 00:00:00 2001
From: Nell Hardcastle <nell@dev-nell.com>
Date: Thu, 7 Nov 2024 12:35:58 -0800
Subject: [PATCH 1/8] chore(app): Remove unused duplicate permission mutations

---
 .../datalad/mutations/remove-permissions.jsx  | 42 ---------
 .../datalad/mutations/update-permissions.jsx  | 86 -------------------
 2 files changed, 128 deletions(-)
 delete mode 100644 packages/openneuro-app/src/scripts/datalad/mutations/remove-permissions.jsx
 delete mode 100644 packages/openneuro-app/src/scripts/datalad/mutations/update-permissions.jsx

diff --git a/packages/openneuro-app/src/scripts/datalad/mutations/remove-permissions.jsx b/packages/openneuro-app/src/scripts/datalad/mutations/remove-permissions.jsx
deleted file mode 100644
index 705eb4c19..000000000
--- a/packages/openneuro-app/src/scripts/datalad/mutations/remove-permissions.jsx
+++ /dev/null
@@ -1,42 +0,0 @@
-import React from "react"
-import PropTypes from "prop-types"
-import { gql } from "@apollo/client"
-import { Mutation } from "@apollo/client/react/components"
-import WarnButton from "../../common/forms/warn-button.jsx"
-
-const REMOVE_PERMISSION = gql`
-  mutation removePermissions($datasetId: ID!, $userId: String!) {
-    removePermissions(datasetId: $datasetId, userId: $userId)
-  }
-`
-
-/**
- * Remove any permissions matching the user
- * @param {object} userPermissions
- * @param {string} userId
- */
-export const userPermissionsFilter = (userPermissions, userId) =>
-  userPermissions.filter((permission) => permission.user.id !== userId)
-
-const RemovePermissions = ({ datasetId, userId }) => (
-  <Mutation mutation={REMOVE_PERMISSION}>
-    {(removePermissions) => (
-      <WarnButton
-        message="Remove Permission"
-        icon="fa-trash"
-        warn={true}
-        action={async (cb) => {
-          await removePermissions({ variables: { datasetId, userId } })
-          cb()
-        }}
-      />
-    )}
-  </Mutation>
-)
-
-RemovePermissions.propTypes = {
-  datasetId: PropTypes.string,
-  userId: PropTypes.string,
-}
-
-export default RemovePermissions
diff --git a/packages/openneuro-app/src/scripts/datalad/mutations/update-permissions.jsx b/packages/openneuro-app/src/scripts/datalad/mutations/update-permissions.jsx
deleted file mode 100644
index 5e660e86d..000000000
--- a/packages/openneuro-app/src/scripts/datalad/mutations/update-permissions.jsx
+++ /dev/null
@@ -1,86 +0,0 @@
-import React from "react"
-import PropTypes from "prop-types"
-import { gql } from "@apollo/client"
-import { Mutation } from "@apollo/client/react/components"
-import { toast } from "react-toastify"
-import ToastContent from "../../common/partials/toast-content.jsx"
-import { validate as isValidEmail } from "email-validator"
-
-const UPDATE_PERMISSIONS = gql`
-  mutation updatePermissions(
-    $datasetId: ID!
-    $userEmail: String!
-    $level: String!
-  ) {
-    updatePermissions(
-      datasetId: $datasetId
-      userEmail: $userEmail
-      level: $level
-    ) {
-      id
-      email
-    }
-  }
-`
-
-export const mergeNewPermission = (
-  datasetId,
-  oldPermissions,
-  userInfo,
-  metadata,
-) => {
-  return {
-    __typename: "Dataset",
-    id: datasetId,
-    permissions: {
-      ...oldPermissions,
-      userPermissions: [
-        ...oldPermissions.userPermissions,
-        {
-          __typename: "Permission",
-          user: { __typename: "User", ...userInfo },
-          level: metadata,
-        },
-      ],
-    },
-  }
-}
-
-const UpdateDatasetPermissions = ({ datasetId, userEmail, metadata, done }) => (
-  <Mutation mutation={UPDATE_PERMISSIONS}>
-    {(UpdateDatasetPermissions) => (
-      <button
-        className="btn-modal-action"
-        onClick={async () => {
-          if (isValidEmail(userEmail)) {
-            try {
-              await UpdateDatasetPermissions({
-                variables: { datasetId, userEmail, level: metadata },
-              })
-              done()
-            } catch (_err) {
-              toast.error(
-                <ToastContent body="A user with that email address does not exist" />,
-              )
-            }
-          } else {
-            toast.error(
-              <ToastContent body="Please enter a valid email address" />,
-            )
-          }
-        }}
-      >
-        Share
-      </button>
-    )}
-  </Mutation>
-)
-
-UpdateDatasetPermissions.propTypes = {
-  datasetId: PropTypes.string,
-  userEmail: PropTypes.string,
-  metadata: PropTypes.oneOf(["ro", "rw", "admin"]),
-  done: PropTypes.func,
-}
-
-export default UpdateDatasetPermissions

From 2ef50b4c666d754ff640c401fa1ddb8919d4b438 Mon Sep 17 00:00:00 2001
From: Nell Hardcastle <nell@dev-nell.com>
Date: Thu, 7 Nov 2024 12:56:16 -0800
Subject: [PATCH 2/8] feat(app): Allow sharing of permissions by ORCID

---
 .../__tests__/update-permissions.spec.jsx     | 14 +++-
 .../dataset/mutations/update-permissions.tsx  | 74 ++++++++++++++++--
 .../src/graphql/resolvers/mutation.ts         |  7 +-
 .../src/graphql/resolvers/permissions.ts      | 78 +++++++++++--------
 .../openneuro-server/src/graphql/schema.ts    |  4 +-
 5 files changed, 138 insertions(+), 39 deletions(-)

diff --git a/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/update-permissions.spec.jsx b/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/update-permissions.spec.jsx
index ae9c10e94..fdeac5ff5 100644
--- a/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/update-permissions.spec.jsx
+++ b/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/update-permissions.spec.jsx
@@ -1,7 +1,7 @@
 import React from "react"
 import { render } from "@testing-library/react"
 import { MockedProvider } from "@apollo/client/testing"
-import { UpdateDatasetPermissions } from "../update-permissions"
+import { isValidOrcid, UpdateDatasetPermissions } from "../update-permissions"
 
 describe("UpdateDatasetPermissions mutation", () => {
   it("renders with default props", () => {
@@ -25,4 +25,16 @@ describe("UpdateDatasetPermissions mutation", () => {
     )
     expect(asFragment()).toMatchSnapshot()
   })
+  describe("isValidOrcid", () => {
+    it("matches typical ORCID strings", () => {
+      expect(isValidOrcid("0000-0001-2345-678")).toBe(false)
+      expect(isValidOrcid("0000-0001-2345-678f")).toBe(false)
+      expect(isValidOrcid("19818c4d-1e60-4480-a317-6fcc1c1a88c6")).toBe(false)
+      expect(isValidOrcid("0000000123456789")).toBe(false)
+      // Check for a correct value
+      expect(isValidOrcid("0000-0001-2345-6789")).toBe(true)
+      // Test with the X checksum value
+      expect(isValidOrcid("0000-0002-1694-233X")).toBe(true)
+    })
+  })
 })
diff --git a/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx b/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx
index 3430425cd..0cd5eca19 100644
--- a/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx
+++ b/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx
@@ -1,11 +1,22 @@
 import React from "react"
 import type { FC } from "react"
 import { gql, useMutation } from "@apollo/client"
+import type { ApolloError } from "@apollo/client"
 import { toast } from "react-toastify"
 import ToastContent from "../../common/partials/toast-content"
 import { validate as isValidEmail } from "email-validator"
 import { Button } from "@openneuro/components/button"
 
+export function isValidOrcid(orcid: string) {
+  if (orcid) {
+    return /^[0-9]{4}-[0-9]{4}-[0-9]{4}-[0-9]{3}[0-9X]$/.test(orcid)
+      ? true
+      : false
+  } else {
+    return false
+  }
+}
+
 const UPDATE_PERMISSIONS = gql`
   mutation updatePermissions(
     $datasetId: ID!
@@ -18,7 +29,42 @@ const UPDATE_PERMISSIONS = gql`
       level: $level
     ) {
       id
-      email
+      userPermissions {
+        datasetId
+        userId
+        level
+        user {
+          id
+          email
+          oricd
+        }
+      }
+    }
+  }
+`
+
+const UPDATE_ORCID_PERMISSIONS = gql`
+  mutation updateOrcidPermissions(
+    $datasetId: ID!
+    $userOrcid: String!
+    $level: String!
+  ) {
+    updateOrcidPermissions(
+      datasetId: $datasetId
+      userOrcid: $userOrcid
+      level: $level
+    ) {
+      id
+      userPermissions {
+        datasetId
+        userId
+        level
+        user {
+          id
+          email
+          orcid
+        }
+      }
     }
   }
 `
@@ -53,13 +99,26 @@ interface UpdateDatasetPermissionsProps {
   done: () => void
 }
 
+function onError(err: ApolloError) {
+  toast.error(
+    <ToastContent body={err?.message} />,
+  )
+}
+
 export const UpdateDatasetPermissions: FC<UpdateDatasetPermissionsProps> = ({
   datasetId,
   userEmail,
   metadata,
   done,
 }) => {
-  const [UpdateDatasetPermissions] = useMutation(UPDATE_PERMISSIONS)
+  const [updateDatasetPermissions] = useMutation(
+    UPDATE_PERMISSIONS,
+    { onError },
+  )
+  const [updateDatasetPermissionsOrcid] = useMutation(
+    UPDATE_ORCID_PERMISSIONS,
+    { onError },
+  )
   return (
     <>
       <Button
@@ -68,9 +127,14 @@ export const UpdateDatasetPermissions: FC<UpdateDatasetPermissionsProps> = ({
         label="Share"
         size="small"
         onClick={async () => {
-          if (isValidEmail(userEmail)) {
+          if (isValidOrcid(userEmail)) {
+            await updateDatasetPermissionsOrcid({
+              variables: { datasetId, userOrcid: userEmail, level: metadata },
+            })
+            done()
+          } else if (isValidEmail(userEmail)) {
             try {
-              await UpdateDatasetPermissions({
+              await updateDatasetPermissions({
                 variables: { datasetId, userEmail, level: metadata },
               })
               done()
@@ -81,7 +145,7 @@ export const UpdateDatasetPermissions: FC<UpdateDatasetPermissionsProps> = ({
             }
           } else {
             toast.error(
-              <ToastContent body="Please enter a valid email address" />,
+              <ToastContent body="Please enter a valid email address or ORCID" />,
             )
           }
         }}
diff --git a/packages/openneuro-server/src/graphql/resolvers/mutation.ts b/packages/openneuro-server/src/graphql/resolvers/mutation.ts
index 04cfdd7a6..1cc46d5a3 100644
--- a/packages/openneuro-server/src/graphql/resolvers/mutation.ts
+++ b/packages/openneuro-server/src/graphql/resolvers/mutation.ts
@@ -19,7 +19,11 @@ import {
 import { removeUser, setAdmin, setBlocked } from "./user.js"
 import { updateSummary } from "./summary"
 import { revalidate, updateValidation } from "./validation.js"
-import { removePermissions, updatePermissions } from "./permissions"
+import {
+  removePermissions,
+  updateOrcidPermissions,
+  updatePermissions,
+} from "./permissions"
 import { followDataset } from "./follow.js"
 import { starDataset } from "./stars.js"
 import { publishDataset } from "./publish.js"
@@ -54,6 +58,7 @@ const Mutation = {
   updateValidation,
   updatePublic,
   updatePermissions,
+  updateOrcidPermissions,
   removePermissions,
   removeUser,
   setAdmin,
diff --git a/packages/openneuro-server/src/graphql/resolvers/permissions.ts b/packages/openneuro-server/src/graphql/resolvers/permissions.ts
index a957501b9..4b6912e37 100644
--- a/packages/openneuro-server/src/graphql/resolvers/permissions.ts
+++ b/packages/openneuro-server/src/graphql/resolvers/permissions.ts
@@ -29,20 +29,35 @@ const publishPermissions = async (datasetId) => {
   const ds = { id: datasetId }
   const { id, userPermissions } = await permissions(ds)
   const permissionsUpdated = {
-    id: datasetId,
-    permissions: {
-      id,
-      userPermissions: await Promise.all(
-        userPermissions.map(async (userPermission) => ({
-          ...userPermission,
-          user: await user(ds, { id: userPermission.userId }),
-        })),
-      ),
-    },
+    id,
+    userPermissions: await Promise.all(
+      userPermissions.map(async (userPermission) => ({
+        ...userPermission,
+        user: await user(ds, { id: userPermission.userId }),
+      })),
+    ),
   }
   return permissionsUpdated
 }
 
+async function updateUsers(datasetId: string, level: string, users) {
+  for (const user of users) {
+    await Permission.updateOne(
+      {
+        datasetId: datasetId,
+        userId: user.id,
+      },
+      {
+        datasetId: datasetId,
+        userId: user.id,
+        level: level,
+      },
+      { upsert: true },
+    ).exec()
+  }
+  return publishPermissions(datasetId)
+}
+
 /**
  * Update user permissions on a dataset
  */
@@ -57,28 +72,29 @@ export const updatePermissions = async (obj, args, { user, userInfo }) => {
     throw new Error("A user with that email address does not exist")
   }
 
-  const userPromises = users.map((user) => {
-    return new Promise<void>((resolve, reject) => {
-      Permission.updateOne(
-        {
-          datasetId: args.datasetId,
-          userId: user.id,
-        },
-        {
-          datasetId: args.datasetId,
-          userId: user.id,
-          level: args.level,
-        },
-        { upsert: true },
-      )
-        .exec()
-        .then(() => resolve())
-        .catch((err) => reject(err))
-    })
+  return updateUsers(args.datasetId, args.level, users)
+}
+
+/**
+ * ORCID variant of updatePermissions
+ */
+export const updateOrcidPermissions = async (obj, args, { user, userInfo }) => {
+  await checkDatasetAdmin(args.datasetId, user, userInfo)
+  // Get all users associated with the ORCID provided
+  const users = await User.find({
+    "$or": [{ "orcid": args.userOrcid }, {
+      "providerId": args.userOrcid,
+      "provider": "orcid",
+    }],
   })
-  return Promise.all(userPromises).then(() =>
-    publishPermissions(args.datasetId)
-  )
+    .collation({ locale: "en", strength: 2 })
+    .exec()
+
+  if (!users.length) {
+    throw new Error("A user with that ORCID does not exist")
+  }
+
+  return updateUsers(args.datasetId, args.level, users)
 }
 
 /**
diff --git a/packages/openneuro-server/src/graphql/schema.ts b/packages/openneuro-server/src/graphql/schema.ts
index 787f13241..ee5078edc 100644
--- a/packages/openneuro-server/src/graphql/schema.ts
+++ b/packages/openneuro-server/src/graphql/schema.ts
@@ -121,7 +121,9 @@ export const typeDefs = `
     # Update a draft with validation results
     updateValidation(validation: ValidationInput!): Boolean
     # Update a users's permissions on a dataset
-    updatePermissions(datasetId: ID!, userEmail: String!, level: String): User
+    updatePermissions(datasetId: ID!, userEmail: String!, level: String!): DatasetPermissions
+    # Update a users's permissions for a given ORCID
+    updateOrcidPermissions(datasetId: ID!, userOrcid: String!, level: String!): DatasetPermissions
     # Remove a users's permissions on a dataset
     removePermissions(datasetId: ID!, userId: String!): Boolean
     # Remove a user

From 790f55adbfdfb6d8667ff3e4f8cd318963a9f65f Mon Sep 17 00:00:00 2001
From: Nell Hardcastle <nell@dev-nell.com>
Date: Thu, 7 Nov 2024 13:01:05 -0800
Subject: [PATCH 3/8] fix(app): Show ORCID for user permissions with email
 fallback

---
 .../dataset/dataset-query-fragments.js        |  1 +
 .../dataset/routes/manage-permissions.jsx     | 24 +++++++++++++++----
 2 files changed, 20 insertions(+), 5 deletions(-)

diff --git a/packages/openneuro-app/src/scripts/datalad/dataset/dataset-query-fragments.js b/packages/openneuro-app/src/scripts/datalad/dataset/dataset-query-fragments.js
index 7c64060fe..b96152028 100644
--- a/packages/openneuro-app/src/scripts/datalad/dataset/dataset-query-fragments.js
+++ b/packages/openneuro-app/src/scripts/datalad/dataset/dataset-query-fragments.js
@@ -74,6 +74,7 @@ export const PERMISSION_FRAGMENT = gql`
         user {
           id
           email
+          orcid
         }
         level
       }
diff --git a/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx b/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
index 2d7122a82..3d9d99f80 100644
--- a/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
+++ b/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
@@ -14,12 +14,24 @@ const description = {
   ro: "View dataset",
 }
 
-export const PermissionRow = ({ datasetId, userId, userEmail, access }) => (
+export const PermissionRow = (
+  { datasetId, userId, userEmail, userOrcid, access },
+) => (
   <div className="data-table-content">
-    <span>
-      <label>Email:</label>
-      <a href={`mailto:${userEmail}`}>{userEmail}</a>
-    </span>
+    {userOrcid
+      ? (
+        <span>
+          <label>ORCID:</label>
+          <a href={`https://orcid.org/${userOrcid}`}>{userOrcid}</a>
+        </span>
+      )
+      : (
+        <span>
+          <label>Email:</label>
+          <a href={`mailto:${userEmail}`}>{userEmail}</a>
+        </span>
+      )}
+
     <span>
       <label>Access:</label>
       {description[access]}
@@ -35,6 +47,7 @@ PermissionRow.propTypes = {
   datasetId: PropTypes.string,
   userId: PropTypes.string,
   userEmail: PropTypes.string,
+  userOrcid: PropTypes.string,
   access: PropTypes.oneOf(["ro", "rw", "admin"]),
 }
 
@@ -51,6 +64,7 @@ export const ShareTable = ({ datasetId, permissions }) => (
         datasetId={datasetId}
         userId={perm.user.id}
         userEmail={perm.user.email}
+        userOrcid={perm.user.orcid}
         access={perm.level}
         key={index}
       />

From 599e2295a5b53ceac9e0479978bd4fba3c356fa3 Mon Sep 17 00:00:00 2001
From: Nell Hardcastle <nell@dev-nell.com>
Date: Thu, 7 Nov 2024 13:10:58 -0800
Subject: [PATCH 4/8] fix(app): Add description for new ORCID permission
 feature

---
 .../src/scripts/dataset/routes/manage-permissions.jsx          | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx b/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
index 3d9d99f80..a623191b2 100644
--- a/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
+++ b/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
@@ -95,7 +95,8 @@ const Share = ({ datasetId, permissions, reviewers, hasSnapshot }) => {
           <div className="dataset-form-body">
             <ShareTable datasetId={datasetId} permissions={permissions} />
             <p>
-              Enter a user&#39;s email address and select access level to share
+              Enter a user&#39;s ORCID or email address and select access level
+              to share
             </p>
             <div className="share-input-group">
               <input

From a49622ace0ca46923e1361828c4fea4d031c9050 Mon Sep 17 00:00:00 2001
From: Nell Hardcastle <nell@dev-nell.com>
Date: Thu, 7 Nov 2024 13:18:12 -0800
Subject: [PATCH 5/8] fix(app): Show user name with ORCID/email on permissions
 view

---
 .../datalad/dataset/dataset-query-fragments.js        |  1 +
 .../src/scripts/dataset/routes/manage-permissions.jsx | 11 +++++++++--
 .../src/scripts/scss/dataset/dataset-tool.scss        |  2 +-
 3 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/packages/openneuro-app/src/scripts/datalad/dataset/dataset-query-fragments.js b/packages/openneuro-app/src/scripts/datalad/dataset/dataset-query-fragments.js
index b96152028..7d249c3f3 100644
--- a/packages/openneuro-app/src/scripts/datalad/dataset/dataset-query-fragments.js
+++ b/packages/openneuro-app/src/scripts/datalad/dataset/dataset-query-fragments.js
@@ -75,6 +75,7 @@ export const PERMISSION_FRAGMENT = gql`
           id
           email
           orcid
+          name
         }
         level
       }
diff --git a/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx b/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
index a623191b2..ae2e5219c 100644
--- a/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
+++ b/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
@@ -15,9 +15,13 @@ const description = {
 }
 
 export const PermissionRow = (
-  { datasetId, userId, userEmail, userOrcid, access },
+  { datasetId, userId, userName, userEmail, userOrcid, access },
 ) => (
   <div className="data-table-content">
+    <span>
+      <label>User:</label>
+      {userName}
+    </span>
     {userOrcid
       ? (
         <span>
@@ -46,6 +50,7 @@ export const PermissionRow = (
 PermissionRow.propTypes = {
   datasetId: PropTypes.string,
   userId: PropTypes.string,
+  userName: PropTypes.string,
   userEmail: PropTypes.string,
   userOrcid: PropTypes.string,
   access: PropTypes.oneOf(["ro", "rw", "admin"]),
@@ -54,7 +59,8 @@ PermissionRow.propTypes = {
 export const ShareTable = ({ datasetId, permissions }) => (
   <>
     <div className="data-table-header">
-      <span>Email</span>
+      <span>Name</span>
+      <span>Reference</span>
       <span>Access</span>
       <span>Edit</span>
     </div>
@@ -63,6 +69,7 @@ export const ShareTable = ({ datasetId, permissions }) => (
       <PermissionRow
         datasetId={datasetId}
         userId={perm.user.id}
+        userName={perm.user.name}
         userEmail={perm.user.email}
         userOrcid={perm.user.orcid}
         access={perm.level}
diff --git a/packages/openneuro-app/src/scripts/scss/dataset/dataset-tool.scss b/packages/openneuro-app/src/scripts/scss/dataset/dataset-tool.scss
index 9f222c3c7..6438da4de 100644
--- a/packages/openneuro-app/src/scripts/scss/dataset/dataset-tool.scss
+++ b/packages/openneuro-app/src/scripts/scss/dataset/dataset-tool.scss
@@ -54,7 +54,7 @@
     }
     > span {
       display: block;
-      min-width: 33.3333%;
+      min-width: 25%;
       margin-bottom: 11px;
       label {
         font-weight: bold;

From 5b9de4a398d538d65bb6de4d4d17f3c366363fdd Mon Sep 17 00:00:00 2001
From: Nell Hardcastle <nell@dev-nell.com>
Date: Thu, 7 Nov 2024 13:21:36 -0800
Subject: [PATCH 6/8] fix(app): Load name/orcid after setting new email
 permissions

---
 .../src/scripts/dataset/mutations/update-permissions.tsx      | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx b/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx
index 0cd5eca19..197a366ec 100644
--- a/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx
+++ b/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx
@@ -36,7 +36,8 @@ const UPDATE_PERMISSIONS = gql`
         user {
           id
           email
-          oricd
+          orcid
+          name
         }
       }
     }
@@ -63,6 +64,7 @@ const UPDATE_ORCID_PERMISSIONS = gql`
           id
           email
           orcid
+          name
         }
       }
     }

From 397753ee98d198aa042bc9c30e53225f103fcd06 Mon Sep 17 00:00:00 2001
From: Nell Hardcastle <nell@dev-nell.com>
Date: Fri, 8 Nov 2024 13:46:18 -0800
Subject: [PATCH 7/8] fix(app): Rename userEmail to userIdentifier since it can
 be an ORCID or email

---
 .../dataset/mutations/update-permissions.tsx  | 51 ++++++++-----------
 .../dataset/routes/manage-permissions.jsx     | 10 ++--
 2 files changed, 25 insertions(+), 36 deletions(-)

diff --git a/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx b/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx
index 197a366ec..6279dfdfa 100644
--- a/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx
+++ b/packages/openneuro-app/src/scripts/dataset/mutations/update-permissions.tsx
@@ -17,7 +17,7 @@ export function isValidOrcid(orcid: string) {
   }
 }
 
-const UPDATE_PERMISSIONS = gql`
+export const UPDATE_PERMISSIONS = gql`
   mutation updatePermissions(
     $datasetId: ID!
     $userEmail: String!
@@ -44,7 +44,7 @@ const UPDATE_PERMISSIONS = gql`
   }
 `
 
-const UPDATE_ORCID_PERMISSIONS = gql`
+export const UPDATE_ORCID_PERMISSIONS = gql`
   mutation updateOrcidPermissions(
     $datasetId: ID!
     $userOrcid: String!
@@ -71,32 +71,9 @@ const UPDATE_ORCID_PERMISSIONS = gql`
   }
 `
 
-export const mergeNewPermission = (
-  datasetId,
-  oldPermissions,
-  userInfo,
-  metadata,
-) => {
-  return {
-    __typename: "Dataset",
-    id: datasetId,
-    permissions: {
-      ...oldPermissions,
-      userPermissions: [
-        ...oldPermissions.userPermissions,
-        {
-          __typename: "Permission",
-          user: { __typename: "User", ...userInfo },
-          level: metadata,
-        },
-      ],
-    },
-  }
-}
-
 interface UpdateDatasetPermissionsProps {
   datasetId: string
-  userEmail: string
+  userIdentifier: string
   metadata: string
   done: () => void
 }
@@ -107,9 +84,13 @@ function onError(err: ApolloError) {
   )
 }
 
+/**
+ * Add permissions to a dataset based on a value provided
+ * userIdentifier is either an email or ORCID
+ */
 export const UpdateDatasetPermissions: FC<UpdateDatasetPermissionsProps> = ({
   datasetId,
-  userEmail,
+  userIdentifier,
   metadata,
   done,
 }) => {
@@ -129,15 +110,23 @@ export const UpdateDatasetPermissions: FC<UpdateDatasetPermissionsProps> = ({
         label="Share"
         size="small"
         onClick={async () => {
-          if (isValidOrcid(userEmail)) {
+          if (isValidOrcid(userIdentifier)) {
             await updateDatasetPermissionsOrcid({
-              variables: { datasetId, userOrcid: userEmail, level: metadata },
+              variables: {
+                datasetId,
+                userOrcid: userIdentifier,
+                level: metadata,
+              },
             })
             done()
-          } else if (isValidEmail(userEmail)) {
+          } else if (isValidEmail(userIdentifier)) {
             try {
               await updateDatasetPermissions({
-                variables: { datasetId, userEmail, level: metadata },
+                variables: {
+                  datasetId,
+                  userEmail: userIdentifier,
+                  level: metadata,
+                },
               })
               done()
             } catch (_err) {
diff --git a/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx b/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
index ae2e5219c..def5996c9 100644
--- a/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
+++ b/packages/openneuro-app/src/scripts/dataset/routes/manage-permissions.jsx
@@ -85,7 +85,7 @@ ShareTable.propTypes = {
 }
 
 const Share = ({ datasetId, permissions, reviewers, hasSnapshot }) => {
-  const [userEmail, setUserEmail] = useState("")
+  const [userIdentifier, setUserIdentifier] = useState("")
   const [access, setAccess] = useState("ro")
 
   const readActive = access === "ro" && "active"
@@ -109,8 +109,8 @@ const Share = ({ datasetId, permissions, reviewers, hasSnapshot }) => {
               <input
                 className="form-control"
                 type="email"
-                value={userEmail}
-                onChange={(e) => setUserEmail(e.target.value)}
+                value={userIdentifier}
+                onChange={(e) => setUserIdentifier(e.target.value)}
               />
               <div className="input-group-btn">
                 <Button
@@ -140,9 +140,9 @@ const Share = ({ datasetId, permissions, reviewers, hasSnapshot }) => {
           <div className="share-form-controls">
             <UpdateDatasetPermissions
               datasetId={datasetId}
-              userEmail={userEmail}
+              userIdentifier={userIdentifier}
               metadata={access}
-              done={() => setUserEmail("")}
+              done={() => setUserIdentifier("")}
             />
           </div>
         </div>

From 85ddbfc6506bba3851885f18a186abd691653f36 Mon Sep 17 00:00:00 2001
From: Nell Hardcastle <nell@dev-nell.com>
Date: Fri, 8 Nov 2024 14:39:29 -0800
Subject: [PATCH 8/8] tests(app): Integration test UpdatePermissions component
 based on input type

---
 .../update-permissions.spec.jsx.snap          |  17 ++-
 .../__tests__/update-permissions.spec.jsx     | 100 ++++++++++++++++--
 2 files changed, 107 insertions(+), 10 deletions(-)

diff --git a/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/__snapshots__/update-permissions.spec.jsx.snap b/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/__snapshots__/update-permissions.spec.jsx.snap
index a7e905e71..5c4f721d1 100644
--- a/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/__snapshots__/update-permissions.spec.jsx.snap
+++ b/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/__snapshots__/update-permissions.spec.jsx.snap
@@ -1,6 +1,19 @@
 // Vitest Snapshot v1, https://vitest.dev/guide/snapshot.html
 
-exports[`UpdateDatasetPermissions mutation > renders with default props 1`] = `
+exports[`UpdateDatasetPermissions mutation > calls UPDATE_ORCID_PERMISSIONS when clicked with an ORCID 1`] = `
+<DocumentFragment>
+  <button
+    aria-label="Share"
+    class="on-button on-button--small on-button--primary  btn-modal-action"
+    role="button"
+    type="button"
+  >
+    Share
+  </button>
+</DocumentFragment>
+`;
+
+exports[`UpdateDatasetPermissions mutation > calls UPDATE_PERMISSIONS when clicked with an email address 1`] = `
 <DocumentFragment>
   <button
     aria-label="Share"
@@ -13,7 +26,7 @@ exports[`UpdateDatasetPermissions mutation > renders with default props 1`] = `
 </DocumentFragment>
 `;
 
-exports[`UpdateDatasetPermissions mutation > renders with typical props 1`] = `
+exports[`UpdateDatasetPermissions mutation > renders with default props 1`] = `
 <DocumentFragment>
   <button
     aria-label="Share"
diff --git a/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/update-permissions.spec.jsx b/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/update-permissions.spec.jsx
index fdeac5ff5..b5d87c1e2 100644
--- a/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/update-permissions.spec.jsx
+++ b/packages/openneuro-app/src/scripts/dataset/mutations/__tests__/update-permissions.spec.jsx
@@ -1,7 +1,44 @@
 import React from "react"
-import { render } from "@testing-library/react"
+import { fireEvent, render, screen, waitFor } from "@testing-library/react"
 import { MockedProvider } from "@apollo/client/testing"
-import { isValidOrcid, UpdateDatasetPermissions } from "../update-permissions"
+import {
+  isValidOrcid,
+  UPDATE_ORCID_PERMISSIONS,
+  UPDATE_PERMISSIONS,
+  UpdateDatasetPermissions,
+} from "../update-permissions"
+
+function permissionMocksFactory(
+  updatePermissionsCalled,
+  updateOrcidPermissionsCalled,
+) {
+  return [
+    {
+      request: {
+        query: UPDATE_PERMISSIONS,
+
+        variables: {
+          datasetId: "ds000005",
+          userEmail: "test@example.com",
+          level: "ro",
+        },
+      },
+      newData: updatePermissionsCalled,
+    },
+    {
+      request: {
+        query: UPDATE_ORCID_PERMISSIONS,
+
+        variables: {
+          datasetId: "ds000005",
+          userOrcid: "0000-0002-1694-233X",
+          level: "ro",
+        },
+      },
+      newData: updateOrcidPermissionsCalled,
+    },
+  ]
+}
 
 describe("UpdateDatasetPermissions mutation", () => {
   it("renders with default props", () => {
@@ -12,18 +49,65 @@ describe("UpdateDatasetPermissions mutation", () => {
     )
     expect(asFragment()).toMatchSnapshot()
   })
-  it("renders with typical props", () => {
+  it("calls UPDATE_PERMISSIONS when clicked with an email address", async () => {
+    const updatePermissionsCalled = vi.fn()
+    const updateOrcidPermissionsCalled = vi.fn()
+    const mocks = permissionMocksFactory(
+      updatePermissionsCalled,
+      updateOrcidPermissionsCalled,
+    )
+    const done = vi.fn()
     const { asFragment } = render(
-      <MockedProvider>
+      <MockedProvider mocks={mocks} addTypename={false}>
         <UpdateDatasetPermissions
           datasetId="ds000005"
-          userEmail="test@example.com"
-          access="ro"
-          done={vi.fn()}
+          userIdentifier="test@example.com"
+          metadata="ro"
+          done={done}
         />
       </MockedProvider>,
     )
-    expect(asFragment()).toMatchSnapshot()
+    const fragment = asFragment()
+    expect(fragment).toMatchSnapshot()
+    // Try clicking the button and make sure the right mutation runs
+    const button = screen.getByRole("button")
+    await fireEvent.click(button)
+    // Make sure it ran at all
+    await waitFor(() => expect(done).toHaveBeenCalled())
+    // Verify the expected query ran
+    await waitFor(() => expect(updatePermissionsCalled).toHaveBeenCalled())
+    await waitFor(() =>
+      expect(updateOrcidPermissionsCalled).not.toHaveBeenCalled()
+    )
+  })
+  it("calls UPDATE_ORCID_PERMISSIONS when clicked with an ORCID", async () => {
+    const updatePermissionsCalled = vi.fn()
+    const updateOrcidPermissionsCalled = vi.fn()
+    const mocks = permissionMocksFactory(
+      updatePermissionsCalled,
+      updateOrcidPermissionsCalled,
+    )
+    const done = vi.fn()
+    const { asFragment } = render(
+      <MockedProvider mocks={mocks} addTypename={false}>
+        <UpdateDatasetPermissions
+          datasetId="ds000005"
+          userIdentifier="0000-0002-1694-233X"
+          metadata="ro"
+          done={done}
+        />
+      </MockedProvider>,
+    )
+    const fragment = asFragment()
+    expect(fragment).toMatchSnapshot()
+    // Try clicking the button and make sure the right mutation runs
+    const button = screen.getByRole("button")
+    await fireEvent.click(button)
+    // Make sure it ran at all
+    await waitFor(() => expect(done).toHaveBeenCalled())
+    // Verify the expected query ran
+    await waitFor(() => expect(updatePermissionsCalled).not.toHaveBeenCalled())
+    await waitFor(() => expect(updateOrcidPermissionsCalled).toHaveBeenCalled())
   })
   describe("isValidOrcid", () => {
     it("matches typical ORCID strings", () => {