Skip to content

fix(identity): use correct sender identity for non-draft messages edited as new messages - #11432

Open
danielcoderman wants to merge 1 commit into
thunderbird:mainfrom
danielcoderman:fix-issue-11348
Open

fix(identity): use correct sender identity for non-draft messages edited as new messages#11432
danielcoderman wants to merge 1 commit into
thunderbird:mainfrom
danielcoderman:fix-issue-11348

Conversation

@danielcoderman

Copy link
Copy Markdown

Contribution Summary

Linked Issue/Ticket: #11348
RFC / Technical Design (if applicable):

Description

This PR fixes issue #11348 by getting the correct sender identity for non-draft messages edited as new messages. It:

  • Adds an else block in the processDraftMessage method of MessageCompose which assigns the correct sender identity
  • Creates a new getSenderIdentityFromMessage function in IdentityHelper
  • Modifies the existing findIdentity method in LegacyAccountDto to first match an identity by email and name before matching by only email
  • Adds unit tests for the new IdentityHelper function

Problem:
Messages edited as new messages go through the same code path as draft messages opened for editing, starting from the call to MessageActions.actionEditDraft they both make (see openMessage and onEditAsNewMessage in MessageHomeActivity). At some point after this call they both go through the processDraftMessage method in MessageCompose. Here is where the problem is. In processDraftMessage the identity metadata is extracted from the K9 identity header if it's attached to the message being processed. This K9 identity header is only attached to messages saved as drafts which means that since this was the only way of getting the identity metadata, in processDraftMessage, non-draft messages ended up using the default identity set in the onCreate method of MessageCompose (line 497).

Approach:
I added an else block to the if statement in processDraftMessage (line 1561) that basically checks if there's an available k9 identity header. In this else block a call to a new function I wrote, IdentityHelper.getSenderIdentityFromMessage, is made. This function takes in the account the message belongs to and the message itself as arguments. It then gets the Address object corresponding to the sender of the message and passes it to the updated findIdentity method which returns the matching identity. The identity field in MessageCompose is set to this returned matching identity in the else block and from there the rest of the code takes care of updating the screen so that the right identity metadata is displayed.

I noticed the findIdentity method didn't take into account the personal field of the passed in Address object (corresponds to the name field of an Identity object), so I updated it so that it first tries to match an identity to both email and name, and if that doesn't work then it finds an identity that just matches the email. At first I removed matching by email only but that failed existing tests, and I later found that names for identities aren't required, and that identity names can be edited. That's why I decided to leave the email only matching because it's necessary. For the case where identity names can be edited, the default identity is still returned if the edited identity shares the same email as the default identity. I don't know if that matters, but the only way to get around that with my current approach would be if the Address object corresponding to the sender of the message contained some sort of ID that doesn't change. Of course the Identity objects would also need this same kind of ID in order for matching to work.

Tests:
I created 3 new tests and a private helper function, in the existing IdentityHelperTest file, for the new getSenderIdentityFromMessage function which also happen to indirectly test the updated findIdentity method. These tests follow the same style of tests in the existing test file, but I can update them to follow the AAA pattern mentioned in the test guide.

I couldn't find an existing test file for LegacyAccountDto to directly test findIdentity after updating it. I can also add tests for it, but since I couldn't find them I first want to make sure I'm not doing anything wrong. Similarly, I couldn't find existing tests for the processDraftMessage method of MessageCompose, but I'm not exactly sure how the testing would work there.

Notes:

  • The update to findIdentity also fixed a bug where clicking the message header of an email you sent with a second identity (sharing the same email as the default identity) showed a bottom sheet displaying the default identity
  • The new code in MessageCompose solves this issue "Edit as new mail" after send fail always sets the identity to default identity? #11348 which happens to occur for any message edited as a new message, not just for failed messages in the Outbox which are edited as new messages.

AI Disclosure

Select one of the following (mandatory)

  • This contribution does not include any changes created or assisted by AI.
  • This contribution includes changes assisted by AI.
  • This contribution includes changes created by AI.

Contribution Checklist

  • I have read and affirm that my contribution adheres to Mozilla’s Community Participation Guidelines
  • This contribution is in Kotlin where possible
  • This contribution does not use merge commits
  • This contribution adheres to the existing codestyle (run gradlew spotlessCheck to check and gradlew spotlessApply to format your source code; will be checked by CI).
  • This contribution does not break existing unit tests (run gradlew testDebugUnitTest; will be checked by CI).
  • This contribution includes tests for any new functionality, and maintains tests for any updated functionality.
  • This contribution adheres to our Engineering process (RFC/Technical Design/ADR)
  • This PR has a descriptive title and body that accurately outlines all changes made, and contains a reference to any issues that it fixes (e.g. Closes #XXX or Fixes #XXX).

The sender identity is retrieved from non-draft messages being
edited as new messages with the help of an IdentityHelper
function because messages that aren't saved as drafts
(sent/received) don't have k9identity metadata attached.

Fixes thunderbird#11348
@github-actions

Copy link
Copy Markdown
Contributor

Missing report label. Set exactly one of: report: include, report: exclude OR report: highlight.

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