Add edit functionality to KnowledgeSuggestionDetailPage

[devin-ai-integration[bot]]

trafficstars

Issue

• resolve: Add edit functionality to KnowledgeSuggestionDetailPage

Why is this change needed?

This change adds the ability for users to edit the content of knowledge suggestions. This allows users to make corrections or improvements to suggestions before approving them.

What would you like reviewers to focus on?

• The implementation of the EditableContent component
• The server action for updating the suggestion content
• The integration with the existing KnowledgeSuggestionDetailPage

Testing Verification

The changes were tested locally to ensure:

• The Edit button appears correctly in the UI
• Clicking Edit transforms the content into an editable textarea
• Changes can be saved to the database
• The UI updates to reflect the saved changes
• Cancel functionality works as expected

Additional Notes

• No edit permission restrictions were implemented as per requirements
• No edit history tracking was implemented as it would require database schema changes

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: 1d40569b548f0ff3449f146ac6d0860174383299

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[vercel[bot]]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
liam-app	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 7, 2025 7:02am
liam-erd-sample	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Apr 7, 2025 7:02am

1 Skipped Deployment

Name	Status	Preview	Comments	Updated (UTC)
liam-docs	⬜️ Ignored (Inspect)	Visit Preview		Apr 7, 2025 7:02am

[devin-ai-integration[bot]]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add "(aside)" to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• [ ] Disable automatic comment and CI monitoring

[supabase[bot]]

Updates to Preview Branch (devin/1743768295-add-diff-display) ↗︎

Deployments	Status	Updated
Database	✅	Mon, 07 Apr 2025 06:59:54 UTC
Services	✅	Mon, 07 Apr 2025 06:59:54 UTC
APIs	✅	Mon, 07 Apr 2025 06:59:54 UTC

Tasks are run on every commit but only new migration files are pushed. Close and reopen this PR if you want to apply changes from existing seed or migration files.

Tasks	Status	Updated
Configurations	✅	Mon, 07 Apr 2025 06:59:54 UTC
Migrations	✅	Mon, 07 Apr 2025 06:59:54 UTC
Seeding	✅	Mon, 07 Apr 2025 06:59:54 UTC
Edge Functions	✅	Mon, 07 Apr 2025 06:59:54 UTC

---

View logs for this Workflow Run ↗︎. Learn more about Supabase for Git ↗︎.

[qodo-merge-for-open-source[bot]]

CI Feedback 🧐

A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

Action: frontend-ci

Failed stage: Run pnpm --filter @liam-hq/db supabase:start [❌]

Failure summary: The action failed because the Supabase Docker container could not be started due to rate limiting from the Docker registry. The error message "toomanyrequests: Rate exceeded" appears multiple times in the log (lines 661, 663, 667), indicating that the GitHub Actions runner has hit a rate limit when trying to pull Docker images from the ECR AWS registry. This caused the supabase:start command to fail with exit status 1.

Relevant error logs: 1: ##[group]Operating System 2: Ubuntu ... 161: [36;1mpnpm install --frozen-lockfile --prefer-offline[0m 162: shell: /usr/bin/bash --noprofile --norc -e -o pipefail {0} 163: env: 164: PNPM_HOME: /home/runner/setup-pnpm/node_modules/.bin 165: ##[endgroup] 166: Scope: all 16 workspace projects 167: Lockfile is up to date, resolution step is skipped 168: Progress: resolved 1, reused 0, downloaded 0, added 0 169: Packages: +2103 170: ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 171: Progress: resolved 2103, reused 1087, downloaded 0, added 0 172: Progress: resolved 2103, reused 2070, downloaded 0, added 306 173: Progress: resolved 2103, reused 2070, downloaded 0, added 860 174: Progress: resolved 2103, reused 2070, downloaded 0, added 1784 175: Progress: resolved 2103, reused 2070, downloaded 0, added 2103, done 176: WARN  Failed to create bin at /home/runner/work/liam/liam/frontend/apps/erd-sample/node_modules/.bin/liam. ENOENT: no such file or directory, open '/home/runner/work/liam/liam/frontend/packages/cli/dist-cli/bin/cli.js' 177: devDependencies: 178: + @changesets/cli 2.27.10 179: + @changesets/get-github-info 0.6.0 180: + @changesets/types 6.0.0 181: + @turbo/gen 2.1.2 182: + syncpack 13.0.3 183: + turbo 2.1.2 184: + vercel 41.4.1 185: frontend/apps/docs postinstall$ fumadocs-mdx 186: frontend/apps/docs postinstall: [MDX] types generated 187: frontend/apps/docs postinstall: Done 188: frontend/apps/app postinstall$ cp ../../packages/db-structure/node_modules/@ruby/prism/src/prism.wasm prism.wasm 189: frontend/apps/app postinstall: Done 190: WARN  Failed to create bin at /home/runner/work/liam/liam/frontend/apps/erd-sample/node_modules/.bin/liam. ENOENT: no such file or directory, open '/home/runner/work/liam/liam/frontend/apps/erd-sample/node_modules/@liam-hq/cli/dist-cli/bin/cli.js' 191: Done in 8.9s ... 646: 814dd06d26c7: Waiting 647: a70653f7a2d5: Verifying Checksum 648: a70653f7a2d5: Download complete 649: 213ec9aee27d: Verifying Checksum 650: 213ec9aee27d: Download complete 651: 814dd06d26c7: Verifying Checksum 652: 814dd06d26c7: Download complete 653: 531e3bd93090: Verifying Checksum 654: 531e3bd93090: Download complete 655: 213ec9aee27d: Pull complete 656: a70653f7a2d5: Pull complete 657: 531e3bd93090: Pull complete 658: 814dd06d26c7: Pull complete 659: Digest: sha256:1b53405d8680a09d6f44494b7990bf7da2ea43f84a258c59717d4539abf09f6d 660: Status: Downloaded newer image for public.ecr.aws/supabase/kong:2.8.1 661: failed to pull docker image: Error response from daemon: toomanyrequests: Rate exceeded 662: Retrying after 4s: public.ecr.aws/supabase/inbucket:3.0.3 663: failed to pull docker image: Error response from daemon: toomanyrequests: Rate exceeded 664: Retrying after 8s: public.ecr.aws/supabase/inbucket:3.0.3 665: 3.0.3: Pulling from supabase/inbucket 666: Stopping containers... 667: failed to display json stream: toomanyrequests: Rate exceeded 668: Try rerunning the command with --debug to troubleshoot the error. 669: /home/runner/work/liam/liam/frontend/packages/db: 670: ERR_PNPM_RECURSIVE_RUN_FIRST_FAIL  @liam-hq/[email protected] supabase:start: `pnpm supabase start` 671: Exit status 1 672: ##[error]Process completed with exit code 1. 673: Post job cleanup.

[NoritakaIkeda]

I'd like to have one more person review it.

[qodo-merge-for-open-source[bot]]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Key Generation The component uses array index as part of React key with a linter ignore comment. Consider using a more reliable key generation approach for the list items to ensure proper rendering and reconciliation. {newContent.split('\n').map((line, index) => ( <div key={`added-${line}-${ // biome-ignore lint/suspicious/noArrayIndexKey: <explanation> index }`} className={styles.diffAdded} > + {line} </div> State Management The component initializes editedContent with content but doesn't update it when the content prop changes. This could lead to stale state if the parent component updates the content prop. const [isEditing, setIsEditing] = useState(false) const [editedContent, setEditedContent] = useState(content) const [isSaving, setIsSaving] = useState(false) const [savedContent, setSavedContent] = useState(content) Error Handling The error message when form data validation fails includes the entire validation issues object, which might expose internal implementation details. Consider providing a more user-friendly error message. if (!parsedData.success) { throw new Error(`Invalid form data: ${JSON.stringify(parsedData.issues)}`) }

[qodo-merge-for-open-source[bot]]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Sync state with prop changes The component initializes both editedContent and savedContent with the same content prop. If the content prop changes after initial render, these state values won't automatically update. Consider using the useEffect hook to update these states when the prop changes. frontend/apps/app/features/projects/components/EditableContent/EditableContent.tsx [24-26] const [editedContent, setEditedContent] = useState(content) const [isSaving, setIsSaving] = useState(false) const [savedContent, setSavedContent] = useState(content) +// Update state when content prop changes +React.useEffect(() => { + setEditedContent(content) + setSavedContent(content) +}, [content]) + [ ] Apply this suggestion Suggestion importance[1-10]: 8 __ Why: This is an important suggestion that addresses a real issue. Without the useEffect hook, if the content prop changes after initial render (e.g., from parent component updates), the component's state won't reflect these changes, potentially causing stale UI or data inconsistencies.	Medium
	Handle potential null values The formData.get() method can return null if the field doesn't exist, but this isn't handled before validation. This could lead to runtime errors if the form fields are missing. Consider adding null checks or providing default values. frontend/apps/app/features/projects/actions/updateKnowledgeSuggestionContent.ts [15-18] const formDataObject = { - suggestionId: formData.get('suggestionId'), - content: formData.get('content'), + suggestionId: formData.get('suggestionId') ?? '', + content: formData.get('content') ?? '', } [ ] Apply this suggestion Suggestion importance[1-10]: 7 __ Why: This suggestion addresses a potential runtime error where FormData.get() could return null if fields are missing. Using nullish coalescing operators provides proper fallbacks, making the code more robust against unexpected form submissions.	Medium
Learned best practice	Validate all input parameters before using them to prevent runtime errors and unexpected behavior The component doesn't validate the newContent parameter before using it. If newContent is undefined, null, or not a string, calling split() on it will cause a runtime error. Add validation for both input parameters to ensure they're handled safely. frontend/apps/app/features/projects/components/DiffDisplay/DiffDisplay.tsx [14-30] +// Validate both inputs if (!originalContent) { + // Ensure newContent is a string + const contentToRender = typeof newContent === 'string' ? newContent : ''; return ( <div className={styles.diffContent}> - {newContent.split('\n').map((line, index) => ( + {contentToRender.split('\n').map((line, index) => ( <div key={`added-${line}-${ // biome-ignore lint/suspicious/noArrayIndexKey: <explanation> index }`} className={styles.diffAdded} > + {line} </div> ))} </div> ) } [ ] Apply this suggestion Suggestion importance[1-10]: 6	Low
General	Improve list key uniqueness The key generation for list items should include a more unique identifier than just the line and index. Consider using a hash of the content or a more robust unique key strategy to prevent potential rendering issues. frontend/apps/app/features/projects/components/DiffDisplay/DiffDisplay.tsx [19-22] -key={`added-${line}-${ - // biome-ignore lint/suspicious/noArrayIndexKey: <explanation> - index -}`} +key={`added-${index}-${lineIndex}-${line.substring(0, 10).replace(/\s/g, '')}`} [ ] Apply this suggestion Suggestion importance[1-10]: 5 __ Why: The suggestion correctly identifies a potential React rendering issue with the current key implementation. Using a more unique key that includes both index values and a substring of the content would improve list rendering stability, though the current implementation is already using index which works for simple cases.	Low
[ ] More