fix: ensure created comments create URLs with inspect and comment par…

…ams (#5012)

packages/sanity/src/desk/comments/src/hooks/useCommentOperations.ts

@@ -9,6 +9,8 @@ import {
   CommentPostPayload,
 } from '../types'
 import {useNotificationTarget} from './useNotificationTarget'
+import {useWorkspace} from 'sanity'
+import {useRouterState} from 'sanity/router'
 
 export interface CommentOperationsHookValue {
   operation: CommentOperations
@@ -52,18 +54,28 @@ export function useCommentOperations(
 
   const authorId = currentUser?.id
 
-  const {documentTitle, toolName, url, workspaceTitle} = useNotificationTarget({
-    documentId,
-    documentType,
-  })
+  const activeToolName = useRouterState(
+    useCallback(
+      (routerState) => (typeof routerState.tool === 'string' ? routerState.tool : undefined),
+      [],
+    ),
+  )
+  const {tools} = useWorkspace()
+  const activeTool = useMemo(
+    () => tools.find((tool) => tool.name === activeToolName),
+    [activeToolName, tools],
+  )
+  const {getNotificationValue} = useNotificationTarget({documentId, documentType})
 
   const handleCreate = useCallback(
     async (comment: CommentCreatePayload) => {
+      // The comment payload might already have an id if, for example, the comment was created
+      // but the request failed. In that case, we'll reuse the id when retrying to
+      // create the comment.
+      const commentId = comment?.id || uuid()
+
       const nextComment = {
-        // The comment payload might already have an id if, for example, the comment was created
-        // but the request failed. In that case, we'll reuse the id when retrying to
-        // create the comment.
-        _id: comment?.id || uuid(),
+        _id: commentId,
         _type: 'comment',
         authorId: authorId || '', // improve
         lastEditedAt: undefined,
@@ -76,12 +88,8 @@ export function useCommentOperations(
           payload: {
             workspace,
           },
-          notification: {
-            documentTitle,
-            url,
-            workspaceTitle,
-          },
-          tool: toolName,
+          notification: getNotificationValue({commentId}),
+          tool: activeTool?.name || '',
         },
         target: {
           path: {
@@ -114,19 +122,17 @@ export function useCommentOperations(
       }
     },
     [
+      activeTool?.name,
       authorId,
       client,
       dataset,
       documentId,
-      documentTitle,
       documentType,
+      getNotificationValue,
       onCreate,
       onCreateError,
       projectId,
-      toolName,
-      url,
       workspace,
-      workspaceTitle,
     ],
   )
 

packages/sanity/src/desk/comments/src/hooks/useNotificationTarget.ts

@@ -1,19 +1,27 @@
-import {useCallback, useMemo} from 'react'
+import {useCallback} from 'react'
 import {useMemoObservable} from 'react-rx'
 import {of} from 'rxjs'
-import {useRouterState} from 'sanity/router'
-import {useSchema, useWorkspace, useDocumentPreviewStore, getPreviewStateObservable} from 'sanity'
+import {usePaneRouter} from '../../../components'
+import {COMMENTS_INSPECTOR_NAME} from '../../../panes/document/constants'
+import {CommentContext} from '../types'
+import {getPreviewStateObservable, useDocumentPreviewStore, useSchema, useWorkspace} from 'sanity'
 
 interface NotificationTargetHookOptions {
   documentId: string
   documentType: string
 }
 
 interface NotificationTargetHookValue {
-  documentTitle: string
-  toolName: string
-  url: string
-  workspaceTitle: string
+  /**
+   * Returns an object with notification-specific values for the selected comment, such as
+   * the current workspace + document title and full URL to the comment.
+   * These values are currently used in notification emails.
+   *
+   * **Please note:** this will generate a URL for the comment based on the current _active_ pane.
+   * The current active pane may not necessarily be the right-most desk pane and in these cases,
+   * the selected comment may not be visible on initial load when visiting these URLs.
+   */
+  getNotificationValue: ({commentId}: {commentId: string}) => CommentContext['notification']
 }
 
 /** @internal */
@@ -22,18 +30,8 @@ export function useNotificationTarget(
 ): NotificationTargetHookValue {
   const {documentId, documentType} = opts || {}
   const schemaType = useSchema().get(documentType)
-  const {basePath, title: workspaceTitle, tools} = useWorkspace()
-
-  const activeToolName = useRouterState(
-    useCallback(
-      (routerState) => (typeof routerState.tool === 'string' ? routerState.tool : undefined),
-      [],
-    ),
-  )
-  const activeTool = useMemo(
-    () => tools.find((tool) => tool.name === activeToolName),
-    [activeToolName, tools],
-  )
+  const {title: workspaceTitle} = useWorkspace()
+  const {createPathWithParams, params} = usePaneRouter()
 
   const documentPreviewStore = useDocumentPreviewStore()
 
@@ -45,15 +43,25 @@ export function useNotificationTarget(
   const {published, draft} = previewState || {}
   const documentTitle = (draft?.title || published?.title || 'Sanity document') as string
 
-  const currentUrl = new URL(window.location.href)
-  const deskToolSegment = currentUrl.pathname.split('/').slice(2, 3).join('')
-  currentUrl.pathname = `${basePath}/${deskToolSegment}/__edit__${documentId},type=${documentType}`
-  const notificationUrl = currentUrl.toString()
+  const handleGetNotificationValue = useCallback(
+    ({commentId}: {commentId: string}) => {
+      // Generate a path based on the current pane params.
+      // We force a value for `inspect` to ensure that this is included in URLs when comments
+      // are created outside of the inspector context (i.e. directly on the field)
+      // @todo: consider filtering pane router params and culling all non-active RHS panes prior to generating this link
+      const path = createPathWithParams({
+        ...params,
+        comment: commentId,
+        inspect: COMMENTS_INSPECTOR_NAME,
+      })
+      const url = `${window.location.origin}${path}`
+
+      return {documentTitle, url, workspaceTitle}
+    },
+    [createPathWithParams, documentTitle, params, workspaceTitle],
+  )
 
   return {
-    documentTitle,
-    toolName: activeTool?.name || '',
-    url: notificationUrl,
-    workspaceTitle,
+    getNotificationValue: handleGetNotificationValue,
   }
 }

packages/sanity/src/desk/comments/src/types.ts

@@ -69,7 +69,11 @@ export interface CommentPath {
   field: string
 }
 
-interface CommentContext {
+/**
+ * @beta
+ * @hidden
+ */
+export interface CommentContext {
   tool: string
   payload?: Record<string, unknown>
   notification?: {

packages/sanity/src/desk/components/paneRouter/types.ts

@@ -157,7 +157,7 @@ export interface PaneRouterContextValue {
    * A function that creates a path with the given parameters without navigating to it.
    * Useful for creating links that can be e.g. copied to clipboard and shared.
    */
-  createPathWithParams: (params: Record<string, string | undefined>) => void
+  createPathWithParams: (params: Record<string, string | undefined>) => string
 
   /**
    * Proxied navigation to a given intent. Consider just exposing `router` instead?