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

Fix for related query on non-existing column #861

Merged
merged 1 commit into from
Mar 6, 2016
Merged

Fix for related query on non-existing column #861

merged 1 commit into from
Mar 6, 2016

Conversation

gfosco
Copy link
Contributor

@gfosco gfosco commented Mar 6, 2016

This will finally address #280 and #286.

@flovilmart
Copy link
Contributor

The role is expected to have a relation to users in the schema, mind to add the same test with a random object, and non existing relation key?

@drew-gross
Copy link
Contributor

I agree with @flovilmart, I don't think the test you added actually tests this change. That said, it's also a simple change so I'd say feel free to merge anyway. (test failure is just flakiness)

@gfosco
Copy link
Contributor Author

gfosco commented Mar 6, 2016

The test fails without the change, so it does work..

@codecov-io
Copy link

Current coverage is 91.87%

Merging #861 into master will increase coverage by +0.19% as of 5b7353e

@@            master    #861   diff @@
======================================
  Files           71      71       
  Stmts         4102    4454   +352
  Branches       845    1012   +167
  Methods          0       0       
======================================
+ Hit           3761    4092   +331
- Partial         10      12     +2
- Missed         331     350    +19

Review entire Coverage Diff as of 5b7353e

Powered by Codecov. Updated on successful CI builds.

@drew-gross
Copy link
Contributor

Actually that does make sense, since the relation will only be in the _SCHEMA object, the collection won't actually be created.

@flovilmart
Copy link
Contributor

oh right !

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.

5 participants