Fixes #9144 - External File Attachment - Migrating files fails#9295
Open
dorianwoolger wants to merge 2 commits into
Open
Fixes #9144 - External File Attachment - Migrating files fails#9295dorianwoolger wants to merge 2 commits into
dorianwoolger wants to merge 2 commits into
Conversation
Author
|
@microsoft-github-policy-service agree |
|
I like the now used approach but wouldn't it be an alternative solution to simple clear the media Id on the the document attachment an then later use the platform capability to delete orphaned media ? What is the advantage of deleting it right away? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Fixes a bug in the "DA External Storage Sync" migration (report 8752) where the shared Tenant Media record behind multiple Document Attachment rows could be deleted after the first attachment was migrated, causing subsequent attachments referencing the same media to fail.
DeleteFromInternalStorage now takes a pre-built Guid → reference count map and only physically deletes a Tenant Media record once its count reaches zero, i.e. once every Document Attachment referencing it has been migrated. The map is built with a single pass over Document Attachment (BuildMediaReferenceCounts) before each batch runs, rather than re-querying per record — Media fields can't be filtered or passed as parameters directly, so this avoids an O(n²) scan while staying correct across partial runs and Maximum Records to Process limits.
Linked work
Fixes #9144
How I validated this
What I tested and the outcome (required — be specific: scenarios, commands, screenshots for UI changes)
I started with a fresh Cronus company. With External file storage disabled.
I attached some documents to an un-posted invoice and posted it. Verified that the new attachments on the posted invoice and the posted receipt were sharing the same Tenant Media Record.
I then enabled External File storage and ran the Sync with Destination to external storage and action of Move.
Let the report run.
The report ran without an failed files.
Checked that all attachments migrated correctly and that nothing was left un migrated.
Existing test updated to include the new MediaReferenceCounts Dictionary.
Risk & compatibility
This change only affects the migration of document attachments from internal to external storage.