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: honor fields in the filter #59

Merged
merged 2 commits into from Apr 12, 2019

Conversation

Projects
None yet
4 participants
@jannyHou
Copy link
Contributor

commented Apr 5, 2019

Description

  • Fixes the failure for missing FK in the fields

    • The fields filter was ignored and that's why the test above fails. This PR adds it to the query.
    • fromDB method will remove the id field is original filter doesn't have it in fields
  • Fixes catching the inclusion error

    • _findRecursive invokes the include handler function without catching the error. That's why the test above returns null instead of throwing the expected error.
    • The fix exposed two new failures. They should have failed, while passed because the error was not propagated. The failure reason is the test case specifies order in filter but doesn't define index for the sortable field(which is required for couchdb2/cloudant connector).
    • The corresponding fix for ^ will be in juggler, we either make property title indexable for Post model, or skip that test for couchdb/cloudant connector. I will submit another PR.

Related issues

  • connect to <link_to_referenced_issue>

Checklist

  • New tests added or existing tests modified to cover all changes The test cases are in juggler.
  • Code conforms with the style
    guide

@jannyHou jannyHou requested review from b-admike, dhmlau and virkt25 as code owners Apr 5, 2019

@dhmlau

This comment has been minimized.

Copy link
Contributor

commented Apr 5, 2019

commit message to be .. honor fields in the filter instead of ` hornor fields in the filter?

@dhmlau dhmlau referenced this pull request Apr 6, 2019

Closed

Fix CI #60

@jannyHou jannyHou force-pushed the fix/support-fields-in-filter branch from ab8fe4d to e242663 Apr 8, 2019

@jannyHou jannyHou changed the title fix: hornor fields in the filter fix: honor fields in the filter Apr 8, 2019

@jannyHou jannyHou referenced this pull request Apr 8, 2019

Merged

Do not apply default values on data from database [3.x] #1705

2 of 2 tasks complete

@jannyHou jannyHou force-pushed the fix/support-fields-in-filter branch from e242663 to 44188ff Apr 9, 2019

@bajtos

bajtos approved these changes Apr 12, 2019

Copy link
Member

left a comment

I am not familiar with this code base, but don't see any obvious problems.

FWIW, LGTM.

@b-admike
Copy link
Member

left a comment

Not too familiar as well, but the explanation and code changes LGTM 👍

@dhmlau

dhmlau approved these changes Apr 12, 2019

Copy link
Contributor

left a comment

The changes look reasonable to me.
There are another set of failures in CI. Is it what you mentioned that after the existing issues are fixed, some other issues are exposed?

@jannyHou

This comment has been minimized.

Copy link
Contributor Author

commented Apr 12, 2019

@dhmlau

There are another set of failures in CI. Is it what you mentioned that after the existing issues are fixed, some other issues are exposed?

Exactly.

@jannyHou jannyHou merged commit 36c978e into master Apr 12, 2019

4 of 8 checks passed

[cis-jenkins] PR Builder Build finished.
Details
[cis-jenkins] x64 && linux && nvm && docker-in-docker,10 Failed! (44188ff)
Details
[cis-jenkins] x64 && linux && nvm && docker-in-docker,6 Failed! (44188ff)
Details
[cis-jenkins] x64 && linux && nvm && docker-in-docker,8 Failed! (44188ff)
Details
Commit Linter commits are all properly formatted
Details
PR Linter PR is up to date
Details
clahub All contributors have signed the Contributor License Agreement.
Details
security/snyk - package.json (StrongLoop) No manifest changes detected

@jannyHou jannyHou deleted the fix/support-fields-in-filter branch Apr 12, 2019

@jannyHou jannyHou referenced this pull request Apr 16, 2019

Merged

Fix: add index for couchdb #1720

2 of 2 tasks complete

jannyHou added a commit that referenced this pull request May 7, 2019

1.4.0
 * chore: update strong-globalize version (Diana Lau)
 * chore: update copyrights years (Diana Lau)
 * fix: update lodash (#58) (Janny)
 * fix: honor fields in the filter (#59) (Janny)
 * fix: eslint (#56) (Janny)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.