deleteRecord does not set hasDirtyAttributes to true

[jayseo5953]

Reproduction

Please provide one of the following:

• Similar issue discussed in discord channel: https://discord.com/channels/480462759797063690/486549196837486592/1052220887069368422

Description

• Have a persisted model (order)
• Delete record using order.deleteRecord()
• Check order.isDeleted is true
• But order.hasDirtyAttributes is false
• Also order.rollbackAttributes() revert isDeleted to false

Versions

• "ember-cli": "4.4.1",
• "ember-source": "~4.4.0"
• "ember-data": "~4.4.0"

[runspired]

This was fixed in later versions. Broad strokes hasDirtyAttributes responding true for isDeleted is a mistake we don't want to carry forward to the replacement for @ember-data/model but it is one we intend to support until then.

If @fivetanley or @richgt wants to get this change into 4.4 then it shouldn't be hard to backport.

[jayseo5953]

@runspired sorry do you mean hasDirtyAttributes being set to true by deleteRecord is a mistake? the document says otherwise tho https://api.emberjs.com/ember-data/4.4/classes/Model/properties/isDeleted?anchor=isDeleted

[runspired]

@jayseo5953 yes, it was a mistake that we did that is what I mean. hasDirtyAttributes should mean only "do I have dirty attributes" not "do I have dirty attributes or am I deleted".

Newer cache APIs allow us to differentiate these things and also provide access to whether relationships are dirty. We should likely have a single isDirty check that is a macro across cache.isDeleted / cache.isNew / cache.hasChangedAttrs / cache.hasChangedRelationships

[jayseo5953]

@runspired right ok thanks for confirmation. Maybe the doc needs editing then?

[runspired]

@jayseo5953 no the docs are right and its a bug that in 4.4 it doesn't work that way. There's two different points being made here:

1. that there is a bug in 4.4 that is fixed in later releases that maybe we should backport to 4.4
2. that the bug is due to us supporting an undesirable behavior which we want to change in the future

I only mention (2) because for folks converting from @ember-data/model to @warp-drive/schema-record the 1:1 migration path will be to check both [STATE].isDeleted and [STATE].hasChangedAttrs, and to a certain degree its probably better to bake the assumption that you need to check both into your code in advance.

[jayseo5953]

@runspired would you be able to point me to the closest version that has the fix? if it was fixed in minor versions

[runspired]

@jayseo5953 according to git blame the fix was part of this PR https://github.com/emberjs/data/pull/8849 (I recalled it being older than this but alas). That PR is currently part of 5.3, but we intend to port its work back to the 4.12 LTS

[jayseo5953]

@runspired thank you!