Skip to content

Commit

Permalink
fix: ensure created comments create URLs with inspect and comment par…
Browse files Browse the repository at this point in the history
…ams (#5012)
  • Loading branch information
robinpyon authored and hermanwikner committed Oct 30, 2023
1 parent 8ece770 commit 2bb2440
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 47 deletions.
42 changes: 24 additions & 18 deletions packages/sanity/src/desk/comments/src/hooks/useCommentOperations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -76,12 +88,8 @@ export function useCommentOperations(
payload: {
workspace,
},
notification: {
documentTitle,
url,
workspaceTitle,
},
tool: toolName,
notification: getNotificationValue({commentId}),
tool: activeTool?.name || '',
},
target: {
path: {
Expand Down Expand Up @@ -114,19 +122,17 @@ export function useCommentOperations(
}
},
[
activeTool?.name,
authorId,
client,
dataset,
documentId,
documentTitle,
documentType,
getNotificationValue,
onCreate,
onCreateError,
projectId,
toolName,
url,
workspace,
workspaceTitle,
],
)

Expand Down
Original file line number Diff line number Diff line change
@@ -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 */
Expand All @@ -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()

Expand All @@ -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,
}
}
6 changes: 5 additions & 1 deletion packages/sanity/src/desk/comments/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,11 @@ export interface CommentPath {
field: string
}

interface CommentContext {
/**
* @beta
* @hidden
*/
export interface CommentContext {
tool: string
payload?: Record<string, unknown>
notification?: {
Expand Down
2 changes: 1 addition & 1 deletion packages/sanity/src/desk/components/paneRouter/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down

0 comments on commit 2bb2440

Please sign in to comment.