Jumoo / uSyncMigrations

Rough and ready migration code.
Mozilla Public License 2.0
47 stars 62 forks source link

feat: implemented basis for overriding aliases for doctypes #200

Closed bielu closed 1 year ago

KevinJump commented 1 year ago

Hi,

I think this one is better coming via the migration plan / context route (less breaking i think and it can be set in custom plans (they can still load from config, but it means it just follows the same patterns) .

So, I would propose: (i don't mind looking at this, but i am in the middle of another bit of code just now)

  1. There is a new Dictionary lookup in the Core Migration Context (SyncMigrationContext.cs)

something like .

    private Dictionary<string, string> _replacementAliases =
        new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);

    public void AddReplacementAlias(string original, string replacement)
        => _replacementAliases.TryAdd(original, replacement);

    public string GetReplacementAlias(string alias)
        => _replacementAliases.TryGetValue(alias, out var replacement) 
        ? replacement : alias;

Add a dictionary replacement to the MigrationOptions.cs e.g :

   public IDictionary<string, string>? ReplacementAliases { get; set; }

then in prepMigration in SyncMigrationService.cs we add the plan values to the content

        options.ReplacementAliases?
            .ForEach(kvp =>
                kvp.Value?.ForEach(value => context.AddReplacementAlias(kvp.Key, kvp.Value)));

then (last one!)

We make GetAliasAndKey also take the context (it should really, its only called from places where we have the context). and then we can do this in the same place but using the context.


but actually, as I've been thinking i have noticed a change like this has the potential to cause a couple of more issues, as the aliases of items are cross-referenced all-over Umbraco. (e.g. templates in doctypes, datatypes in doctypes, etc)

if for example you renamed a datatype in this way, we would also have to make sure when we look up the datatype as part of the ContentType migration that we do this rename.

because the context of the migration will have an entry for the datatype under the new alias (but not the old one) and we will be reading another place in the code

for example, in the SharedContentHandler.cs we look up the Datatype "Type" property which is the "Alias" of a datatype

https://github.com/Jumoo/uSyncMigrations/blob/a0d58792f9ec4b34fd0f6eb4cb634c92b43ce411/uSync.Migrations/Handlers/Shared/SharedContentTypeBaseHandler.cs#L47

And i suspect there are quite a few of these around the code - so we will need to do some cross referencing to check that.

If the datatype has been renamed, we would also need to make sure we renamed it here. uSync may well still work with the renamed aliases being wrong, but it would cause false positives and there are a few places (e.g templates) where it might cause a real issue


bielu commented 1 year ago

Hey Kevin, I just needed it for now, in our case it works perfectly fine. It is why I open it as Draft, as dont have time to pick up and look into it properly. I am gonna be off for 3 weeks from next week so might not be able to pick it up before you do :)

KevinJump commented 1 year ago

No worries, I think I have a few things to merge and tidy up this week/next so I will do this too.

If it supersedes this PR I will let you know

KevinJump commented 1 year ago

replaced by #209