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

find a string based primaryKey called id #855

Merged
merged 3 commits into from Aug 27, 2013

Conversation

4 participants
@whito
Contributor

whito commented Aug 23, 2013

I am using a String instead of an Integer for my id column ( I use a UUID ) and the find() function wasn't working. This pull request allows find to work when using a non-Integer based id instead of an Integer based id.

whito added some commits Aug 23, 2013

@durango

This comment has been minimized.

Member

durango commented Aug 23, 2013

Are you sure this isn't already fixed in #master? I thought I added a PR for this a while back...

@whito

This comment has been minimized.

Contributor

whito commented Aug 23, 2013

I came across the problem yesterday... was just the extra check for id in dao-factory:

(attributeName !== 'id')

which wouldn't allow the primaryKeys to be populated

@whito

This comment has been minimized.

Contributor

whito commented Aug 23, 2013

Strange... Travis CI fails one test... but I don't think its related to this PR... any ideas what it is?

@durango

This comment has been minimized.

Member

durango commented Aug 23, 2013

Strange... Travis CI fails one test... but I don't think its related to this PR... any ideas what it is?

Github says its fine O-o

@whito

This comment has been minimized.

Contributor

whito commented Aug 23, 2013

Ya I pushed a teeny change in the PR to start travis again and it looks good now ;)

@sdepold sdepold merged commit 82c3783 into sequelize:master Aug 27, 2013

1 check passed

default The Travis CI build passed
Details
@coveralls

This comment has been minimized.

coveralls commented Nov 13, 2014

Coverage Status

Changes Unknown when pulling 82c3783 on whito:master into * on sequelize:master*.

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