Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make SetIfDifferent no longer check if a key is mutable. #41

Merged
merged 1 commit into from
Nov 18, 2015

Conversation

richardjrossiii
Copy link
Contributor

This would cause issues when updating an installation. By forcing removal of a key using an operation, we no longer check for children who override IsMutableForKey.

Fixes #40, and probably several others.

cc @grantland @hallucinogen

SetIfDifferent("appVersion", ParseClient.PlatformHooks.AppBuildVersion);
SetIfDifferent("appIdentifier", ParseClient.PlatformHooks.AppIdentifier);
SetIfDifferent("appName", ParseClient.PlatformHooks.AppName);
ForceSetIfDifferent("deviceType", ParseClient.PlatformHooks.DeviceType);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SetIfDifferent will not check for mutability. https://github.com/ParsePlatform/Parse-SDK-dotNET/blob/master/Parse/ParseObject.cs#L1238

I'm not sure this is the right fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It does check for mutability when it calls Remove, though.I guess we can change each of these SetIfDifferentCalls and bound them by a null check on the value, although I'm not certain that makes a ton of sense (we could end up with really stale values).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the correct solution for this would be to refactor SetIfDifferent so that it also forces Remove, and not make another ForceSetIfDifferent. SetIfDifferent by definition should force update.

This would cause issues when updating an installation. By forcing removal of a key using an operation, we no longer check for children who override `IsMutableForKey`.

Fixes #40, and probably several others.
@richardjrossiii richardjrossiii changed the title Made installation fields properly update during save. Make SetIfDifferent no longer check if a key is mutable. Nov 18, 2015
@richardjrossiii
Copy link
Contributor Author

PR updated.

@hallucinogen
Copy link
Contributor

Perfecto

@hallucinogen hallucinogen added this to the 1.6.2 milestone Nov 18, 2015
richardjrossiii added a commit that referenced this pull request Nov 18, 2015
Make `SetIfDifferent` no longer check if a key is mutable.
@richardjrossiii richardjrossiii merged commit 6351b4d into master Nov 18, 2015
@richardjrossiii richardjrossiii deleted the richardross.installation.fix branch November 18, 2015 01:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants