Attach fields orphaned by an earlier seed run, not just new ones - #66
Merged
Conversation
…w ones #65 added about_highlights/about_milestones but the fix only worked on a genuinely fresh database. GetOrCreateContentTypeAsync found the About content type already present (this repo's own dev Host.db has run every prior version of this seed) and returned it untouched, so both fields were created and then left attached to nothing - reported after merge, from exactly that database. The seed's own idempotency-by-name is what caused it: GetOrCreateFieldAsync found the fields already existed - created by an earlier run of the OLD code, before this content-type-append logic existed at all - so tracking "created this run" (what the first attempt at this fix did) cannot see them; they predate the run entirely. Replaced that with "attached to no content type anywhere", computed once at the start of SeedAsync over every existing content type and kept current as this run attaches more. A field with zero usages anywhere is specific enough to this bug to touch automatically for a *local dev convenience* seed - a real admin's normal workflow attaches a new field somewhere immediately, and a field still in use elsewhere is left alone either way. New usages are appended after the current highest Order rather than at their declared position, so a deliberate arrangement is not reshuffled. Verified against this repo's own dev Host.db, in the exact broken state the report described: the two fields moved from unattached to Order 2/3 on About, a second run made no further change, and a from-scratch database still gets all four fields in one pass.
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
1 participant
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.
The bug you found
#65 added
about_highlights/about_milestonesand claimed they were verified — against a throwaway database only. Against a real, already-migrated database (this repo's own devHost.db), the seed's own idempotency worked against it:GetOrCreateContentTypeAsyncfound theaboutcontent type already present and returned it untouched, so both fields got created and then sat attached to nothing.Why the straightforward "attach fields created this run" didn't work either
That was my first attempt, and it doesn't cover this exact database:
GetOrCreateFieldAsyncfound the fields already existed, created by an earlier run of the old code before this content-type-append logic existed at all. "Created this run" can't see a field that predates the run.The fix
GetOrCreateContentTypeAsyncnow computes, once at the start ofSeedAsync, every field id already attached to any content type - and appends a requested usage only when the field is attached to none. A field with zero usages anywhere is specific enough to this failure mode to touch automatically for a local dev convenience seed: a real admin's normal workflow attaches a new field to something immediately, and a field still in use elsewhere (even a different content type than this seed asks for) is left alone either way.New usages are appended after the current highest
Order, so an existing arrangement someone has deliberately ordered isn't reshuffled underneath them.Test plan
Against this repo's own dev
Host.db, in the exact broken state the report described (backed up first):about_highlights/about_milestonesexist as fields,about's content type has onlytitle/bodyOrder2/3dotnet testfor Domain (252) and EntityFrameworkCore (317)