[28.1] Multiple runs of email retrieval on the same emails don't persist again - #11617
Joshua (mynjj) wants to merge 2 commits into
Conversation
Good Sense Reviewer - Round 1Recommendation: AcceptWhat this PR doesThis change adds a central lookup for emails that were already retrieved by account and external message id, then lets a connector return that existing inbox entry instead of creating another one. It also adds a specialized attachment path for messages that were retrieved earlier without attachments. The lookup is scoped by account and external id, handles temporary result records, and marks the existing row so the existing retrieval flow can return it. The attachment path skips only the email-message Problem-solution fitFit: Strong The reported problem is that retrieving the same provider message again can persist duplicate inbox entries. The new lookup, key, attachment overload, and tests cover the existing-entry, wrong-account/wrong-message, temporary-result, and missing-attachment cases, so the code matches the scenario. SuggestionsNone. Risk assessment and necessityRisk: The change touches public email APIs and the Email Inbox lookup path, so the main regression surface is connectors that retrieve into persistent records and extensions that call the new attachment overload. Existing Necessity: The change is needed because filtering alone cannot stop a connector from storing the same provider message again. Centralizing duplicate detection in the email module keeps the rule consistent for connectors and allows the missing-attachment case without reopening general edits to retrieved messages.
|
77ffc55
Good Sense Reviewer - Round 2Recommendation: AcceptWhat this PR doesThe PR adds a central lookup for already retrieved emails and a safe attachment path for a retrieved message that was first stored without attachments. Since round 1, the only author change renames local test variables from Status of previous suggestionsNone - round 1 had no suggestions. New observations (commits since round 1)None - the round-2 change only updates local test variable names. Risk assessment and necessityRisk: The regression surface is the public email API, the Email Inbox lookup by account and external message id, and attachment loading for retrieved messages. The added APIs are additive, the lookup is scoped, and the attachment insert still keeps the existing sent and queued checks. Necessity: The change is needed to avoid storing duplicate inbox entries when the same provider message is retrieved again. The scope is right for this branch: it adds the shared email-module support and leaves the branch-specific connector consumption to the repository where that connector lives.
|
Backport of #11612.
Fix at the email module layer of the bug causing additional entries to be persisted when retrieving the same email.
If the email had already been retrieved and stored, then such email is returned without further persisting, unless the previous email was stored without attachments, in which case only the attachment is added to such email message.
Tested the behavior against my email inbox locally.
Fix overview
FindRetrievedEmailon the email module lets connectors check whether an email had been retrieved before, with the idea of centralizing this potentially tricky logic and benefitting other connectors.AddAttachmentwas needed, since theModifyon the email message that was run after, triggers a subscriber that errors when modifying a received email (rationale of the error is that the system shouldn't modify emails after they have been received, but in this scenario we are not modifying the EmailMessage, but rather adding an attachment and leaving the message as it was, so we don't want this validation to run)Note
In this version the
Email - Outlook REST APIapp does not live in BCApps, so this PR carries the System Application part only. The consuming change in the Outlook connector (Email - Outlook API Helper) has to be done in the NAV repository for this branch.Fixes AB#650461