Skip to content

Conversation

@Ahmed-Naguib93
Copy link
Contributor

refs: CLX-3721
builds: Student
affects: Student
release note: none

What is new?

  1. Removed unnecessary code, including properties and functions.
  2. Removed SwiftUI and HorizonUI imports from ViewModels.
  3. Fixed pull-to-refresh to call all APIs.
  4. Enhanced API calls when loading the view.
  5. Fixed issues with uploading and downloading files.
  6. Called the notifications API to show announcements and set indicators for read and unread. This was added because announcements were previously hidden from notifications.
  7. Temporarily removed the GetAllAnnouncementsRequest call, as the implementation for conversation is not yet set.
Before After Comment
https://github.com/user-attachments/assets/0dcddf58-e0c1-4bc1-a740-122c12de3935 https://github.com/user-attachments/assets/b4c1de3e-9b7f-47b6-a1c5-f2194ebc3066 Selecting multiple recipients isn't working.
Before After Comment
https://github.com/user-attachments/assets/f7814cd5-5b32-4d70-b1a8-5b646d7ff28c https://github.com/user-attachments/assets/576a241f-ecfb-47c9-9d2b-e9f6ec43b012 When selecting different files, uploading files with the same name still shows the loader until you take any action.
Before After Comment
https://github.com/user-attachments/assets/a1544665-2072-4f22-8c19-83f54dba38b1 https://github.com/user-attachments/assets/955b0ee3-5370-4f63-8f5a-b46ab4d35119 When you exceed your quota, an error message is shown.
Before After Comment
https://github.com/user-attachments/assets/d8e79b49-0a32-4949-aea4-f9907066ecf7 https://github.com/user-attachments/assets/60f8c250-a4e2-4fe4-a5a6-0d84d49d4882 Enhanced launch inbox view
Before After Comment
https://github.com/user-attachments/assets/caf8abed-0e7e-4853-82cd-46d33d817b0c https://github.com/user-attachments/assets/f6264171-c60e-4d76-8632-8c4d41031b35 Added an indicator for unread announcements and applied it to the unread filter as well.

refs: CLX-3721
builds: Student
affects: Student
release note: none

test plan: none
@Ahmed-Naguib93 Ahmed-Naguib93 self-assigned this Jan 4, 2026
@Ahmed-Naguib93 Ahmed-Naguib93 added the career Canvas Career Experience label Jan 4, 2026
@claude
Copy link

claude bot commented Jan 4, 2026

Claude Code Review

Updated: 2026-01-04

Approved - No critical issues found.

Summary:

  • Refactored Inbox feature with proper separation of concerns (announcements vs messages)
  • Added UI helper modifiers (loader overlay, keyboard dismiss, rounded corners)
  • Improved attachment handling with upload state management
  • Enhanced notification integration with unread indicators
  • Properly refactored HMessageDetailsView into separate components

Key observations:

  • Old view models properly replaced with new architecture
  • Subscription management correctly implemented
  • API integration logic properly refactored to use Combine publishers
  • All deleted files have appropriate replacements
  • No breaking changes detected

✅ Approved

@inst-danger
Copy link
Contributor

inst-danger commented Jan 4, 2026

Warnings
⚠️ This pull request will not generate a release note.
⚠️ One or more files are below the minimum test coverage 50%

Affected Apps: Student

Builds: Student

Coverage New % Master % Delta
Canvas iOS 91.12% 81.06% 10.05%
Horizon/Horizon/Sources/Common/View/CourseDetailsAccessibility.swift 0% -- --

Generated by 🚫 dangerJS against be5e3d4

@inst-danger
Copy link
Contributor

inst-danger commented Jan 4, 2026

Builds

Commit: fix: filter (be5e3d4)
Build Number: 1080
Built At: Jan 05 13:58 CET (01/05 05:58 AM MST)

Student

@claude
Copy link

claude bot commented Jan 5, 2026

Claude Code Review

Updated: 2026-01-05

Critical Issues Found:

  1. Debug print statement in RecipientSelectionViewModel.swift:75 - print(value) should be removed
  2. Logic issue in HInboxViewModel.swift:183 - announcement filter may exclude announcements prematurely
  3. isSending state not reset on sendMessage failure in HMessageDetailsViewModel.swift:136
  4. Conversation object may be nil when accessed in HMessageDetailsViewModel.swift:82-85

🚫 Issues found

@claude
Copy link

claude bot commented Jan 5, 2026

Claude Code Review

Updated: 2026-01-05

Critical Issues Found

  1. Missing self prefix in HMessageDetailsViewModel - Code filters recipients with .filter { $0 != userID } but userID is not in scope - should be self.userID. Will crash on reply message send.

  2. Broken announcement route fallback - Changed from /announcements/:announcementID to /announcements requiring announcement in userInfo. Guard returns nil on missing data, causing silent navigation failure with no error handling.

  3. Incomplete AttachmentFileModel refactor - Deleted AttachmentItemViewModel with download logic but AttachmentFileModel lacks performAction() implementation. UI expects download functionality that no longer exists.

Positive Changes

  • Added didUploadFiles subject for proper file upload state tracking
  • Improved pull-to-refresh to call all APIs
  • Fixed keyboard dismissal handling

🔴 Issues found

@claude
Copy link

claude bot commented Jan 5, 2026

Claude Code Review

Updated: 2026-01-05

✅ Approved

No critical issues found. Large-scale inbox refactoring with proper subscription management, weak self captures, and sound filtering logic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

career Canvas Career Experience

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants