At first look, this is a clever attempt to determine if this entity will run into an insert or update. But unfortunately it contains a flaw 🙈 .

Yes on the first migration run this will tell you if the product "was previously attempted to migrate" and thus give the the wanted result.

But if the migration for this entity somehow fails:

• either further down in this converter
• in the validation layer that comes after - which we currently implement in our feature branch
• or later while writing the entity to the corresponding SW6 table

it would mean the next migration run (retry) would still consider this entity as an "update", because the mapping already exists, which produces the wrong outcome in this case.

We have to think further about this, to not introduce yet another N+1 Query performance issue 😬

Sorry for not raising this concern earlier:

Do you really think it's a good idea to have such a ImmutableFlag ? (Was introduced in this PR )

Because I already see the extra work it creates for us here, every entity that uses that flag needs an extra special treatment in the migration and if someone adds that flag to other fields in the future (or introduces such fields), we will run into issues again.

It basically makes "upserts" (insert / update) really difficult to do for such entities, because you want to create them if they don't exists yet, or update them otherwise, so you need the field to be defined but also not defined at the same time... 🤯

I also can't imagine that it's nice to work with such an API, e.g. imagine:

• You read the data of an existing entity over our admin API
• You modify some value
• You want to update that entity with the same payload

I think this was possible in the past (correct me if I'm wrong), but now you need to remove certain fields from your payload to send the update request, which is really annoying.

Of course in our administration codebase this is a non-issue, because there we use some extra logic / magic to build a changeset diff and only send that back to the server, but do you really expect every other API client to do the same or treat it differently like we do here in the migration? 😬

I would suggest either:

1. Allowing updates in the DAL when the value of the Immutable field stays the same (is unchanged), as it should be in these cases like here in the "upsert" case for migration. Otherwise it would be a valid error if the migration tries to change such a value
2. Or "silently" ignore that field in the DAL update case, without throwing an error. Maybe it should still create a log entry though 🤷‍♂️

I'm open for other suggestions as well, but I think we should have a better answer for the "upsert" case than just deal with it by throwing extra code at it. The Migration-Assistant makes heave use of "upsert" and basically writes all it's data that way:

cc: @patzick @keulinho what's your opinion on this?

That do makes sense, I was also thinking about the update should allow the payload with immutable key if the value doesn't change, but at the query time, we haven't aware of the current value in the database, fetching it before wouldn't be ideal, I will have a look further at this problem and your suggestion and provide an improvement for the flag.

For an external API consumer, I think ideally they don't want to send the whole entities; a subset of what changes is more practical and lightweight. But there might be cases where they want to resend the entire payload and catch the immutable error, as in your mentioned scenario.

Another simple option is to bypass the immutable check for system context scope; it will work for examples like MIG, but not if they update the entities via the admin api.

Allowing updates in the DAL when the value of the Immutable field stays the same (is unchanged), as it should be in these cases like here in the "upsert" case for migration. Otherwise it would be a valid error if the migration tries to change such a value

This should be possible, I added a draft PR here: shopware/shopware#14422