Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 27, 2025

The Thread::mapFromDatabase method was returning attachment data as plain arrays instead of ThreadEmailAttachment objects, inconsistent with the type annotation and other code paths like ThreadDatabaseOperations.

Changes

  • Thread.php: Modified mapFromDatabase to instantiate ThreadEmailAttachment objects when loading attachments from database
  • classify-email.php: Removed redundant (object) casts (2 occurrences) that worked around the array-vs-object mismatch

Before:

$email->attachments = array_map(function($att) {
    return [
        'id' => $att['id'],
        'name' => $att['name'],
        // ...
    ];
}, $attachments);

After:

$email->attachments = array_map(function($att) {
    $attObj = new ThreadEmailAttachment();
    $attObj->id = $att['id'];
    $attObj->name = $att['name'];
    // ...
    return $attObj;
}, $attachments);

This aligns with the existing ThreadEmail type annotation @var $attachments ThreadEmailAttachment[] and eliminates type inconsistency across the codebase.

Original prompt

In Thread.php, change the return of email->attachments to be ThreadEmailAttachments.

Look for $att = (object)$att. That should find a couple of usages that can easily be fixed.


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.


Note

Aligns attachment types with declared models and simplifies consumers.

  • Thread::mapFromDatabase now instantiates ThreadEmailAttachment objects for email->attachments instead of returning plain arrays
  • Removed redundant (object) casts and adjusted attachment status handling in classify-email.php since attachments are now proper ThreadEmailAttachment instances

Written by Cursor Bugbot for commit 2e781ca. This will update automatically on new commits. Configure here.

- Updated Thread.php mapFromDatabase to create ThreadEmailAttachment objects instead of arrays
- Removed (object) casts in classify-email.php since attachments are now proper objects
- Added require for ThreadEmailAttachment.php in Thread.php

Co-authored-by: HNygard <168380+HNygard@users.noreply.github.com>
Copilot AI changed the title [WIP] Change return type of email attachments to ThreadEmailAttachments Return ThreadEmailAttachment objects from Thread::mapFromDatabase Dec 27, 2025
Copilot AI requested a review from HNygard December 27, 2025 22:38
@HNygard HNygard marked this pull request as ready for review December 30, 2025 08:21
Copilot AI review requested due to automatic review settings December 30, 2025 08:21
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a type inconsistency in Thread::mapFromDatabase by returning ThreadEmailAttachment objects instead of plain arrays, aligning with the documented type annotations and patterns used elsewhere in the codebase.

Key Changes

  • Modified Thread::mapFromDatabase to instantiate ThreadEmailAttachment objects when loading attachments from the database
  • Removed redundant (object) casts from classify-email.php that were working around the type mismatch
  • Added the required ThreadEmailAttachment.php import to Thread.php

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
organizer/src/class/Thread.php Added import for ThreadEmailAttachment class and modified mapFromDatabase to create attachment objects instead of arrays
organizer/src/classify-email.php Removed two (object)$att casts that are no longer needed since attachments are now properly returned as objects

After thoroughly reviewing the changes, I found no issues within the modified code regions. The implementation correctly:

  1. Instantiates proper objects: Each attachment is now created as a ThreadEmailAttachment instance with all 7 required properties (id, name, filename, filetype, location, status_type, status_text) properly mapped from the database result
  2. Maintains consistency: The approach matches the pattern already used in ThreadDatabaseOperations.php (lines 160-168, 292-300)
  3. Removes workarounds: The (object)$att casts in classify-email.php are correctly removed, as they were only necessary when attachments were arrays
  4. Aligns with type annotations: The change makes the code consistent with the @var $attachments ThreadEmailAttachment[] annotation in the ThreadEmail class

The changes are well-structured and achieve the stated goal of eliminating type inconsistency across the codebase.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@HNygard HNygard merged commit 7ba3498 into main Dec 30, 2025
8 checks passed
@HNygard HNygard deleted the copilot/update-email-attachments-return branch December 30, 2025 08:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants