טוסט ביטול פעולות בכמות - #2886
Conversation
- Complete guide for implementing Toast with Undo button - Includes frontend JavaScript (UndoToastManager class) - Includes CSS styles with theme support and RTL - Includes backend Python API endpoints - Covers Bulk Delete, Tag, and Move operations - Testing examples and implementation checklist Co-authored-by: amirbiron <amirbiron@gmail.com>
|
Cursor Agent can help with this pull request. Just |
🧯 Dangerous deletes guard reportPolicy: see .cursorrules — dangerous deletions are blocked unless wrapped safely. Summary:
Flagged findings (file:line:snippet): Excluded matches (by path pattern) |
⏱️ Performance report(No performance test durations collected. Mark tests with |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Security fixes: - Add XSS protection with escapeHtml() and sanitizeIconName() - Sanitize user input before innerHTML injection Code quality fixes: - Add parameter validation in show() method - Add null safety with optional chaining in addTags() - Replace deprecated substr() with slice() - Remove unused variables (originalMessage, fileNames) - Replace bare except clauses with specific exceptions - Add type validation for file_ids (must be list) - Add InvalidId import note for Python code Co-authored-by: amirbiron <amirbiron@gmail.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Add updateMessage() helper with null safety for DOM elements - Fix get-tags API to return proper error status (500) instead of 200 - Add success field to get-tags response for explicit error checking - Update getFilesTags() to throw on API failure - Handle getFilesTags() failure gracefully - continue without undo - Show regular notification instead of undo toast when no previous state Co-authored-by: amirbiron <amirbiron@gmail.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Add pendingDelete flag set immediately (before animation timeout) - Store timeout IDs in Map to allow cancellation - Cancel pending hide timeout when restore is called quickly - Check both hiddenByDelete and pendingDelete in restore - Clear both flags on restore to handle all states Co-authored-by: amirbiron <amirbiron@gmail.com>
- Check response.ok before calling response.json() - Wrap JSON parsing in try/catch for non-JSON responses - Provide clear error messages for each failure type Co-authored-by: amirbiron <amirbiron@gmail.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
- Add isUndoing flag to toast data - Set flag when handleUndo starts processing - Check flag in dismiss() to skip onExpire when undoing - Prevents incorrect state updates when undo completes Co-authored-by: amirbiron <amirbiron@gmail.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
1. Extract generateOperationId() as shared helper (DRY) 2. Clear existing timeout before creating new in hideFilesFromUI 3. Handle partial restore - only restore files that succeeded 4. Prevent double-click on undo with isUndoing check at entry 5. Track cleanup timeouts in handleUndo 6. Track and clear all timeouts in remove() 7. Add response.ok checks before JSON parsing in all fetch calls 8. Add RTL support for mobile close button position 9. Wrap onExpire callback in try/catch in dismiss() Co-authored-by: amirbiron <amirbiron@gmail.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| // בטל כל ה-timeouts הפעילים | ||
| clearTimeout(timeout); | ||
| if (cleanupTimeout) clearTimeout(cleanupTimeout); | ||
| if (fallbackTimeout) clearTimeout(fallbackTimeout); |
There was a problem hiding this comment.
Dead code attempts to clear nonexistent timeout
Low Severity
The remove method destructures fallbackTimeout from toastData and attempts to clear it, but fallbackTimeout is never stored in the toastData object. It's only created after the map entry is deleted. The clearTimeout(fallbackTimeout) on line 326 never executes because fallbackTimeout is always undefined from the destructuring.
| const { | ||
| operationId, | ||
| message, | ||
| duration = 5000, |
There was a problem hiding this comment.
Duration parameter lacks validation for invalid values
Medium Severity
The duration parameter in the show method accepts any value without validation. Non-numeric values (e.g., "hello") are coerced by setTimeout to NaN, which is treated as 0, causing the toast to dismiss immediately. Negative values produce invalid CSS transitions and near-instant dismissal. Users would be unable to see the message or click the undo button, defeating the entire purpose of the toast.
Additional Locations (1)
| // סגור אחרי שנייה - שמור reference ל-timeout | ||
| const cleanupTimeout = setTimeout(() => { | ||
| this.remove(operationId); | ||
| }, 1500); |
There was a problem hiding this comment.
Race condition in handleUndo removes wrong toast
Medium Severity
After handleUndo awaits the async onUndo() callback, it calls this.remove(operationId) to remove the toast. During the await, if show() is called with the same operationId, the original toast is dismissed and a new one is created with that ID. When the original handleUndo resumes, it removes the new toast instead of the old one, causing the wrong toast to disappear unexpectedly.
Additional Locations (1)
2 participants
✨ תיאור קצר
📦 שינויים עיקריים
פירוט נקודות:
GUIDES/UNDO_TOAST_IMPLEMENTATION_GUIDE.md.UndoToastManagerב-JavaScript ו-CSS), ומימוש Backend (API לשחזור קבצים, שמירת/הגדרת תגיות).🧪 בדיקות
🧪 בדיקות נדרשות ב‑PR
📝 סוג שינוי
✅ צ'קליסט
services/register_jobs.py(כולל Callback/Trigger להפעלה ידנית — למשלcallback_name/trigger_funcלפי המבנה) כדי שיופיעו בדשבורדdocs/environment-variables.rstוגםservices/config_inspector_service.pydocs/webapp/theming_and_css.rst+FEATURE_SUGGESTIONS/theme_matrix.md🧩 השפעות/סיכונים
🔗 קישורים
🧯 סיכון / החזרה לאחור (Rollback)
Note
Introduces a comprehensive guide for implementing an Undo-enabled toast after bulk operations.
GUIDES/UNDO_TOAST_IMPLEMENTATION_GUIDE.mdwith architecture (soft delete/deferred action), UX/security considerations, and a rollout checklistUndoToastManager(JS) with safe rendering, timers, and optimistic UI flows fordeleteSelectedandaddTags; accompanying CSS for themes, RTL, and mobilePOST /api/files/bulk-restore,POST /api/files/get-tags,POST /api/files/bulk-set-tagswith Mongo updates and validationUNDO_TOAST_DURATION,SOFT_DELETE_TTL) and example unit/integration testsWritten by Cursor Bugbot for commit f4c2e93. This will update automatically on new commits. Configure here.