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

refactor(IsNew): check string type PrimaryKey first #538 #542

Merged
merged 3 commits into from Aug 2, 2019

Conversation

@ArgoZhang
Copy link
Contributor

commented May 31, 2019

#Issue
link #538

@pleb

This comment has been minimized.

Copy link
Member

commented Jun 11, 2019

Hey @ArgoZhang why was this causing an issue? I don't see how this change affects anything.

@ArgoZhang

This comment has been minimized.

Copy link
Contributor Author

commented Jun 11, 2019

Hey @ArgoZhang why was this causing an issue? I don't see how this change affects anything.

[TableName("Groups")]
public class Group
{
    public string Id { get; set; }
    public string GroupName { get; set; }

    public void Save(Group poco)
    {
        var db = DbManager.Create();
        db.Save(new Group() { Id = "", GroupName = "UnitTest" });
    }
}

because Id value is String.Empty not null, so IsNew method work incorrectly.
the IsNew method will always run false when PrimaryKey is string type and value is "". so run into Update method.
i think that this is a bug. and you?
by the way it works when value is NULL

@pleb

This comment has been minimized.

Copy link
Member

commented Aug 1, 2019

@ArgoZhang could you please fix the line endings?

@ArgoZhang ArgoZhang force-pushed the ArgoZhang:dev-StringPrimary branch from fd5ebdd to 40fcc65 Aug 2, 2019

@ArgoZhang ArgoZhang force-pushed the ArgoZhang:dev-StringPrimary branch from 40fcc65 to 90d6928 Aug 2, 2019

ArgoZhang added some commits Aug 2, 2019

@ArgoZhang ArgoZhang force-pushed the ArgoZhang:dev-StringPrimary branch from 62f0ad0 to 8ca4095 Aug 2, 2019

@ArgoZhang

This comment has been minimized.

Copy link
Contributor Author

commented Aug 2, 2019

@pleb done! resolved the conflicts.
thank you for pick up this PR.
😃

@pleb pleb merged commit 401c49e into CollaboratingPlatypus:development Aug 2, 2019

1 check failed

continuous-integration/appveyor/pr AppVeyor build failed
Details

@ArgoZhang ArgoZhang deleted the ArgoZhang:dev-StringPrimary branch Aug 2, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.