🐛 fix: screenshots not uploaded in Contribute modal#3980
Conversation
…dal) Root cause: FeatureRequestModal.tsx called createRequest() without passing the screenshots field. Instead it appended a placeholder text "Screenshots: N screenshot(s) attached — paste images into this issue" to the description. The backend never received the screenshot data URIs and had nothing to upload. FeedbackModal.tsx (the simpler modal) correctly passed screenshots. FeatureRequestModal.tsx (the main Contribute modal that most users see) did not. Fix: Pass screenshot data URIs to createRequest() so the backend can upload them via GitHub Contents API and embed them in the issue body. Verified via CDP end-to-end: browser → API → GitHub Contents API → embedded  in created issue. Fixes #3637 Signed-off-by: Andrew Anderson <andy@clubanderson.com>
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
👋 Hey @clubanderson — thanks for opening this PR!
This is an automated message. |
✅ Deploy Preview for kubestellarconsole ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
Thank you for your contribution! Your PR has been merged. Check out what's new:
Stay connected: Slack #kubestellar-dev | Multi-Cluster Survey |
There was a problem hiding this comment.
Pull request overview
Fixes missing screenshot uploads for issues created via the main Contribute/FeatureRequest modal by ensuring screenshot data URIs are sent to the backend, allowing server-side upload + embedding into the created GitHub issue.
Changes:
- Stop appending a placeholder “screenshots attached” note into the issue description.
- Collect screenshot base64 data URIs from the modal state and pass them to
createRequest()via thescreenshotsfield.
| // Collect base64 data URIs for screenshots so the backend can upload | ||
| // them to GitHub and embed them directly in the created issue. | ||
| const screenshotDataURIs = screenshots.map(s => s.preview) |
There was a problem hiding this comment.
Now that screenshots are actually sent to the backend for upload/embedding, there is still UI copy later in this component telling users that screenshots will only be “noted” and they should paste them into GitHub manually. That guidance will be incorrect after this change; please update the user-facing text to reflect automatic upload/embedding (and consider whether the “copy screenshot” affordance is still needed).
| ...(screenshotDataURIs.length > 0 && { screenshots: screenshotDataURIs }), | ||
| }) | ||
| setSuccess({ issueUrl: result.github_issue_url }) | ||
| // Show thank-you for 5s (extended to give time to copy screenshots) then switch to Updates tab |
There was a problem hiding this comment.
The inline comment about keeping the success state visible longer “to give time to copy screenshots” looks stale now that screenshots are uploaded/embedded automatically. Please update the comment (and possibly the timeout rationale) so it matches the new behavior to avoid future confusion during maintenance.
| // Show thank-you for 5s (extended to give time to copy screenshots) then switch to Updates tab | |
| // Keep the success state visible for 5s so users can read the confirmation and open the issue before switching to the Updates tab |
| const result = await createRequest({ | ||
| title: extractedTitle, | ||
| description: finalDesc, | ||
| description: extractedDesc, | ||
| request_type: requestType, | ||
| target_repo: targetRepo, | ||
| ...(screenshotDataURIs.length > 0 && { screenshots: screenshotDataURIs }), | ||
| }) |
There was a problem hiding this comment.
This change fixes a regression-critical path (sending screenshot data URIs to the backend). Consider adding a unit/integration test that renders FeatureRequestModal, attaches a screenshot, submits, and asserts createRequest is called with the screenshots field populated—otherwise it’s easy for this bug to reappear unnoticed (FeatureRequestModal already has a basic vitest file in the repo).
🔄 Auto-Applying Copilot Code ReviewCopilot code review found 1 code suggestion(s) and 2 general comment(s). @copilot Please apply all of the following code review suggestions:
Also address these general comments:
Push all fixes in a single commit. Run Auto-generated by copilot-review-apply workflow. |
- Update stale UI copy: screenshots are now uploaded/embedded, not "noted" with manual paste instructions - Fix stale comment about extended timeout for copying screenshots - Fix !data truthy check in api.ts get/post/put — use === null to avoid misclassifying valid falsy JSON responses (0, false, "") Signed-off-by: Andrew Anderson <andy@clubanderson.com>
- Update stale UI copy: screenshots are now uploaded/embedded, not "noted" with manual paste instructions - Fix stale comment about extended timeout for copying screenshots - Fix !data truthy check in api.ts get/post/put — use === null to avoid misclassifying valid falsy JSON responses (0, false, "") Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Summary
Root cause of #3637:
FeatureRequestModal.tsx(the main Contribute modal) calledcreateRequest()without passing thescreenshotsfield. It appended placeholder text instead:The backend never received the data URIs and had nothing to upload. The simpler
FeedbackModal.tsxcorrectly passed screenshots — but most users submit throughFeatureRequestModal.Fix: Pass
screenshots: screenshotDataURIstocreateRequest()so the backend uploads them via GitHub Contents API and embeds them asin the issue body.Verified via CDP E2E
Test plan
npm run buildpassesdownload_urlFixes #3637