Skip to content

feat(google-docs): Selec entries to create [INTEG-3894]#10966

Merged
Joaquin (joaquincasal) merged 5 commits into
masterfrom
integ-3894
May 7, 2026
Merged

feat(google-docs): Selec entries to create [INTEG-3894]#10966
Joaquin (joaquincasal) merged 5 commits into
masterfrom
integ-3894

Conversation

@joaquincasal
Copy link
Copy Markdown
Collaborator

@joaquincasal Joaquin (joaquincasal) commented May 7, 2026

Summary

Adds per-entry selection to the Google Docs import review flow, so users can choose which previewed entries are created.

Changes

  • Added entry checkboxes, selected by default.
  • Updated CTA to Create selected entries.
  • Sends only selected entries in the resumed entryBlockGraph.
  • Prunes references to unselected entries.

Testing

  • Added focused frontend and backend tests.
Screen.Recording.2026-05-07.at.15.59.29.mov

# Conflicts:
#	apps/google-docs/src/locations/Page/components/overview/OverviewSection.tsx
#	apps/google-docs/src/locations/Page/components/review/ReviewPage.tsx
#	apps/google-docs/src/services/agents-api.ts
@joaquincasal Joaquin (joaquincasal) requested review from a team as code owners May 7, 2026 14:25
Copy link
Copy Markdown
Collaborator

@JuliRossi JuliRossi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Left a few comments

Comment on lines +59 to +66
<Checkbox
aria-label={`Create entry ${row.contentTypeName || 'Untitled'}${
row.entryTitle ? ` (${truncateLabel(row.entryTitle, 150)})` : ''
}`}
isChecked={isEntrySelectedForCreation}
isDisabled={areEntrySelectionsDisabled}
onChange={(event) => onToggleEntrySelection(row.id, event.target.checked)}
/>
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤔 There seem to be an issue with the checkbox (saw this on the video). Instead of showing the hand pointer (default from forma) is showing the arrow pointer. Perhaps it has to do with the missing props in Card component that were setting up the cursor

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's weird, but that only happens when I record the video, when I use the app without recording, it shows the hand pointer. I cannot prove it as taking a screenshot also removes the cursor, but I checked and it works as expected


const { resumeWorkflow } = useWorkflowAgent({ sdk, documentId: '', oauthToken: '' });

const handleToggleEntrySelection = useCallback((entryKey: string, isSelected: boolean) => {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⛏️ Does this need to be a callback?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm probably not, I'll remove it

Copy link
Copy Markdown
Contributor

@mgoudy91 Mitch Goudy (mgoudy91) left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few issues worth addressing before merge — two correctness/behavioral risks and one quick perf fix, plus one UX suggestion.

  • Issue 1: getEntrySelectionKey index fallback can silently map keys to wrong entries if entries are reordered, and risks collision with numeric tempId values.
  • Issue 2: filterEntryBlockGraphBySelection doesn't prune the referenceGraph — stale edges to deselected entries may cause backend errors depending on whether the server rebuilds it.
  • Issue 3: handleToggleEntrySelection is missing useCallback, causing unnecessary re-renders of the full entry tree on every selection change.
  • Issue 5 (suggestion): The removed UX hint about clicking entry rows to preview content left users with no affordance for that interaction.

@joaquincasal
Copy link
Copy Markdown
Collaborator Author

A few issues worth addressing before merge — two correctness/behavioral risks and one quick perf fix, plus one UX suggestion.

* **Issue 1**: `getEntrySelectionKey` index fallback can silently map keys to wrong entries if entries are reordered, and risks collision with numeric `tempId` values.

* **Issue 2**: `filterEntryBlockGraphBySelection` doesn't prune the `referenceGraph` — stale edges to deselected entries may cause backend errors depending on whether the server rebuilds it.

* **Issue 3**: `handleToggleEntrySelection` is missing `useCallback`, causing unnecessary re-renders of the full entry tree on every selection change.

* **Issue 5** (suggestion): The removed UX hint about clicking entry rows to preview content left users with no affordance for that interaction.

Issue 1: I addressed the key collision risk by namespacing selection keys. Regarding reordering, I don’t think entries can be reordered in the current UI, so it shouldn’t be a problem unless I’m missing something.

Issue 2: The existing referenceGraph is not sent after filtering. We only send the selected-only entryBlockGraph to the backend, and the backend recreates referenceGraph, so I don’t think we’re at risk of errors caused by stale edges. Let me know if you think otherwise.

Issue 3: I think adding useCallback wouldn’t save us from re-renders, as selection changes still create a new selectedEntryKeys set and that prop flows through the entry tree. The child rows are also not memoized, so stabilizing only the handler identity doesn’t change much.

Issue 5: That’s how it is in the designs, and I checked with Erin.

Copy link
Copy Markdown
Contributor

@mgoudy91 Mitch Goudy (mgoudy91) left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for thinking through that, lgtm

@joaquincasal Joaquin (joaquincasal) merged commit 459d8f2 into master May 7, 2026
14 checks passed
@joaquincasal Joaquin (joaquincasal) deleted the integ-3894 branch May 7, 2026 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants