docs(feedback): fix 8 bugs/gaps identified in systematic review
- Fix inconsistent screenshotUrl fields (removed, SAS generated on-demand) - Fix blob path pattern to match feedbackScreenshots container - Clarify flow: direct upload to final container (no temp/move) - Add rate limiting specs to endpoint table - Clarify access control: users submit but cannot view (security) - Remove sas.ts from appendix (not created) - Align size limits to 5MB consistently - Add missing screenshotContentType and screenshotSizeBytes
This commit is contained in:
parent
0996534fb4
commit
fdaffdb13c
@ -49,9 +49,9 @@ User Flow:
|
|||||||
1. User taps "Report Issue" in app
|
1. User taps "Report Issue" in app
|
||||||
2. Client captures screenshot (optional annotation)
|
2. Client captures screenshot (optional annotation)
|
||||||
3. Client requests SAS URL: POST /api/feedback/sas
|
3. Client requests SAS URL: POST /api/feedback/sas
|
||||||
4. Client uploads image directly to Azure Blob
|
4. Client uploads image directly to Azure Blob (to feedbackScreenshots container)
|
||||||
5. Client submits feedback: POST /api/feedback (with screenshotBlobPath)
|
5. Client submits feedback: POST /api/feedback (with screenshotBlobPath, contentType, sizeBytes)
|
||||||
6. Admin views feedback in dashboard with image preview
|
6. Admin views feedback in dashboard with image preview (fresh SAS URL generated on-demand)
|
||||||
```
|
```
|
||||||
|
|
||||||
---
|
---
|
||||||
@ -65,10 +65,10 @@ User Flow:
|
|||||||
interface FeedbackDoc {
|
interface FeedbackDoc {
|
||||||
// ... existing fields ...
|
// ... existing fields ...
|
||||||
|
|
||||||
// Screenshot attachment (single)
|
// Screenshot attachment (single) — SAS URLs generated on-demand, not stored
|
||||||
screenshotBlobPath?: string; // "feedback/{productId}/{feedbackId}/{screenshotId}.png"
|
screenshotBlobPath?: string; // "feedbackScreenshots/{productId}/{feedbackId}/{screenshotId}.png"
|
||||||
screenshotUrl?: string; // Time-limited SAS URL for viewing
|
screenshotContentType?: 'image/png' | 'image/jpeg' | 'image/webp';
|
||||||
screenshotUrlExpiresAt?: string; // When SAS URL expires
|
screenshotSizeBytes?: number; // Actual uploaded size
|
||||||
|
|
||||||
// Device context for debugging
|
// Device context for debugging
|
||||||
deviceContext?: {
|
deviceContext?: {
|
||||||
@ -82,6 +82,8 @@ interface FeedbackDoc {
|
|||||||
|
|
||||||
// Add to CreateFeedbackSchema
|
// Add to CreateFeedbackSchema
|
||||||
screenshotBlobPath: z.string().optional(),
|
screenshotBlobPath: z.string().optional(),
|
||||||
|
screenshotContentType: z.enum(['image/png', 'image/jpeg', 'image/webp']).optional(),
|
||||||
|
screenshotSizeBytes: z.number().int().min(1).max(5 * 1024 * 1024).optional(), // 5MB limit
|
||||||
deviceContext: z.object({
|
deviceContext: z.object({
|
||||||
osVersion: z.string(),
|
osVersion: z.string(),
|
||||||
appVersion: z.string(),
|
appVersion: z.string(),
|
||||||
@ -142,12 +144,12 @@ DELETE /api/feedback/:id/attachments/:attId // Remove attachment (admin)
|
|||||||
|
|
||||||
### New Endpoints
|
### New Endpoints
|
||||||
|
|
||||||
| Method | Endpoint | Auth | Description |
|
| Method | Endpoint | Auth | Rate Limit | Description |
|
||||||
|--------|----------|------|-------------|
|
|--------|----------|------|------------|-------------|
|
||||||
| `POST` | `/api/feedback/sas` | User | Get SAS URL for screenshot upload |
|
| `POST` | `/api/feedback/sas` | User | 5 per 10 min | Get SAS URL for screenshot upload |
|
||||||
| `POST` | `/api/feedback` | User | Submit feedback (with optional screenshotBlobPath) |
|
| `POST` | `/api/feedback` | User | 10 per hour | Submit feedback (with optional screenshotBlobPath) |
|
||||||
| `GET` | `/api/feedback/:id/screenshot` | Admin | Get fresh SAS URL for viewing screenshot |
|
| `GET` | `/api/feedback/:id/screenshot` | Admin | 30 per hour | Get fresh SAS URL for viewing screenshot |
|
||||||
| `DELETE` | `/api/feedback/:id/screenshot` | Admin | Delete screenshot (GDPR/privacy) |
|
| `DELETE` | `/api/feedback/:id/screenshot` | Admin | 10 per hour | Delete screenshot (GDPR/privacy) |
|
||||||
|
|
||||||
### SAS Generation Endpoint
|
### SAS Generation Endpoint
|
||||||
|
|
||||||
@ -161,7 +163,7 @@ DELETE /api/feedback/:id/attachments/:attId // Remove attachment (admin)
|
|||||||
|
|
||||||
// Response (201)
|
// Response (201)
|
||||||
{
|
{
|
||||||
"blobPath": "feedback/lysnrai/feedback_abc123/screenshot_xyz.png",
|
"blobPath": "feedbackScreenshots/lysnrai/fb_abc123/screenshot_xyz.png",
|
||||||
"uploadUrl": "https://bytelyst.blob.core.windows.net/...?sv=...",
|
"uploadUrl": "https://bytelyst.blob.core.windows.net/...?sv=...",
|
||||||
"expiresIn": 300, // 5 minutes
|
"expiresIn": 300, // 5 minutes
|
||||||
"maxSizeBytes": 5242880 // 5MB limit
|
"maxSizeBytes": 5242880 // 5MB limit
|
||||||
@ -178,7 +180,7 @@ DELETE /api/feedback/:id/attachments/:attId // Remove attachment (admin)
|
|||||||
"title": "App crashes when tapping record",
|
"title": "App crashes when tapping record",
|
||||||
"body": "Steps: 1. Open app 2. Tap red button 3. Crash",
|
"body": "Steps: 1. Open app 2. Tap red button 3. Crash",
|
||||||
"screen": "RecordingScreen",
|
"screen": "RecordingScreen",
|
||||||
"screenshotBlobPath": "feedback/lysnrai/feedback_abc123/screenshot_xyz.png",
|
"screenshotBlobPath": "feedbackScreenshots/lysnrai/fb_abc123/screenshot_xyz.png",
|
||||||
"deviceContext": {
|
"deviceContext": {
|
||||||
"osVersion": "iOS 17.4",
|
"osVersion": "iOS 17.4",
|
||||||
"appVersion": "2.3.1",
|
"appVersion": "2.3.1",
|
||||||
@ -276,9 +278,9 @@ DELETE /api/feedback/:id/attachments/:attId // Remove attachment (admin)
|
|||||||
- [ ] User-initiated deletion: "Delete my feedback and screenshot"
|
- [ ] User-initiated deletion: "Delete my feedback and screenshot"
|
||||||
|
|
||||||
### Access Control
|
### Access Control
|
||||||
- [ ] Users can only view their own screenshots
|
- [ ] Users submit screenshots but cannot directly view/download them (security)
|
||||||
- [ ] Admins can view all screenshots for their product
|
- [ ] Admins can view all screenshots for their product via fresh SAS URLs
|
||||||
- [ ] Fresh SAS URLs generated per view (time-limited, 15 minutes)
|
- [ ] SAS URLs are time-limited (15 minutes) and generated on-demand per view
|
||||||
|
|
||||||
---
|
---
|
||||||
|
|
||||||
@ -308,7 +310,6 @@ DELETE /api/feedback/:id/attachments/:attId // Remove attachment (admin)
|
|||||||
### New Files
|
### New Files
|
||||||
```
|
```
|
||||||
services/platform-service/src/modules/feedback/
|
services/platform-service/src/modules/feedback/
|
||||||
├── sas.ts # SAS generation helpers
|
|
||||||
└── attachment.types.ts # (if Option B)
|
└── attachment.types.ts # (if Option B)
|
||||||
```
|
```
|
||||||
|
|
||||||
@ -329,3 +330,16 @@ services/platform-service/src/server.ts
|
|||||||
---
|
---
|
||||||
|
|
||||||
**Last Updated:** 2026-03-02
|
**Last Updated:** 2026-03-02
|
||||||
|
|
||||||
|
## Bugs Fixed During Review (2026-03-02)
|
||||||
|
|
||||||
|
| # | Bug | Fix |
|
||||||
|
|---|-----|-----|
|
||||||
|
| 1 | Inconsistent fields: `screenshotUrl`/`screenshotUrlExpiresAt` not used | Removed - SAS URLs generated on-demand |
|
||||||
|
| 2 | Blob path pattern mismatch | Updated to `feedbackScreenshots/{productId}/{feedbackId}/{screenshotId}.png` |
|
||||||
|
| 3 | Flow showed "temp path then move" but implementation uploads directly | Clarified flow to show direct upload to final container |
|
||||||
|
| 4 | Missing rate limiting specs | Added rate limit column to endpoint table |
|
||||||
|
| 5 | Privacy claimed "users can view own screenshots" but only admin endpoint exists | Clarified users submit but cannot view (security) |
|
||||||
|
| 6 | Appendix listed `sas.ts` file that wasn't created | Removed - SAS logic is in routes.ts |
|
||||||
|
| 7 | Inconsistent size limits: 5MB vs 10MB | Aligned to 5MB throughout |
|
||||||
|
| 8 | Missing `screenshotContentType` and `screenshotSizeBytes` | Added to data model example |
|
||||||
|
|||||||
@ -52,9 +52,6 @@ import { generateSasUrl } from '../../lib/blob.js';
|
|||||||
import * as auditRepo from '../audit/repository.js';
|
import * as auditRepo from '../audit/repository.js';
|
||||||
import type { AuditDoc } from '../audit/types.js';
|
import type { AuditDoc } from '../audit/types.js';
|
||||||
|
|
||||||
// TODO-1: Event bus integration - emit events for session lifecycle
|
|
||||||
// Import event bus: import { bus } from '../../lib/event-bus.js';
|
|
||||||
|
|
||||||
// Re-export shared helpers from types
|
// Re-export shared helpers from types
|
||||||
export { generateId, buildPk } from './types.js';
|
export { generateId, buildPk } from './types.js';
|
||||||
|
|
||||||
@ -533,10 +530,6 @@ export async function diagnosticsRoutes(app: FastifyInstance) {
|
|||||||
throw new BadRequestError(`Session is not active (status: ${session.status})`);
|
throw new BadRequestError(`Session is not active (status: ${session.status})`);
|
||||||
}
|
}
|
||||||
|
|
||||||
// TODO-8: Blob SAS token generation
|
|
||||||
// Need to integrate with existing blob module to generate SAS URL
|
|
||||||
// For now, return placeholder
|
|
||||||
|
|
||||||
const screenshotId = generateId('scr');
|
const screenshotId = generateId('scr');
|
||||||
const blobPath = `screenshots/${productId}/${id}/${screenshotId}.png`;
|
const blobPath = `screenshots/${productId}/${id}/${screenshotId}.png`;
|
||||||
const containerName = 'diagnostics-screenshots';
|
const containerName = 'diagnostics-screenshots';
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user