Fix SC.Record.normalize: accept record attributes based on classes extending SC.RecordAttribute #1127

Closed
wants to merge 2 commits into
from

Conversation

Projects
None yet
2 participants
Contributor

mirion commented Nov 22, 2013

When using custom types of RecordAttributes the normalize method was ignoring them (using SC.instanceOf rather than SC.kindOf ) so the transform using fromType was not executed, therefore the data into the hash was invalid (not the expected type).

Also remove a developer warning produced by the primary key being at the beginning stored into the hash and eventually removed if the record is new. Initially, in the first commit, I preserved the rowKey into the hash as it was set at the beginning of normalize, but arrived at the conclusion that storing it into the record hash is really bad practice, therefore I removed it completely.

mirion added some commits Nov 19, 2013

@mirion mirion Fix SC.Record.normalize: accept record attributes based on classes ex…
…tending SC.RecordAttribute, remove a developer warning produced by the primary key not being marked as preserved (into keysTokeep)
b3b7b9d
@mirion mirion Remove the primaryKey from the record hash during the normalize proce…
…dure. As general note, it is bad practice to store the id into the hash
4869ae0
Owner

dcporter commented Feb 12, 2014

I like the kindOf fix, but I'm not sure about actively removing the primary key from the hash. I agree that it's not great to have it in there, but removing it is awfully opinionated behavior.

dcporter closed this in 07abb34 Feb 14, 2014

Owner

dcporter commented Feb 14, 2014

You're right that there's all kinds of inconsistent behavior in normalize regarding primaryKey. It looked at a glance like it would add primaryKey in, but then remove it later if it's not defined as an attribute. I've taken the hint and added it to the keysToKeep list to prevent it from being removed, but not actively adding it. We remain for now studiously agnostic on the subject. =)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment