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

Update adapter-knex.js #4719

Closed
wants to merge 2 commits into from
Closed

Update adapter-knex.js #4719

wants to merge 2 commits into from

Conversation

josemf
Copy link
Contributor

@josemf josemf commented Jan 26, 2021

Fixes an issue where List that have a relationship field as its first field will have this field used for orderBy clause in query. This will fail the query for those relationships where fields don't exist in the List.

Issue: #3164

Fixes an issue where List that have a relationship field as its first field will have this field used for orderBy clause in query. This will fail the query for those relationships where fields don't exist in the List.

Issue: #3164
@changeset-bot
Copy link

changeset-bot bot commented Jan 26, 2021

⚠️ No Changeset found

Latest commit: 397fb63

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

@timleslie timleslie left a comment

Choose a reason for hiding this comment

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

Hi @josemf , thanks for tracking down this bug. I think you're changes will help solve the issue.

In the future we will need to address the issue of sorting by relationship fields which do not have a realKey, but that can be solved once this fix has gone through.

To get through CI could I ask you to run yarn format so that yarn lint passes, and also to add a changeset to capture the changes for the changelog (yarn changeset). Thanks!

@josemf
Copy link
Contributor Author

josemf commented Feb 8, 2021

@timleslie thank you for following up

I followed your instructions on a new PR for this issue.
#4791

You can close this one,

@timleslie timleslie closed this Feb 8, 2021
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.

2 participants