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

🐛 Fixed known filter limitations #10159

Merged
merged 83 commits into from
Dec 11, 2018
Merged

Conversation

kirrg001
Copy link
Contributor

@kirrg001 kirrg001 commented Nov 13, 2018

refs #10105, closes #10108

Tasks

  • NQL must forward relation config
  • use NQL tarball for now
  • remove preProcessFilters in filter plugin
  • remove preProcessFilters in filter plugin
  • check order = (SELECT count(*) FROM posts_tags WHERE post_id = posts.id) DESC, ${order}; is tested well
  • change NQL API
  • tag_spec count test
  • double protect extraFilters fn in Ghost (check public context!) -> not needed, see commit
  • double check if we have .skip tests because of older GQL bugs
  • test members cases
  • QA

@kirrg001 kirrg001 force-pushed the replace-gql branch 2 times, most recently from 8106f1b to 43f833d Compare November 13, 2018 17:22
@kirrg001 kirrg001 changed the title [WIP] Replaced GQL with NQL [HOLD/WIP] Replaced GQL with NQL Nov 13, 2018
kirrg001 added a commit that referenced this pull request Nov 15, 2018
refs #10105

- `options.where` is an older deprecated logic
- before the filter language was invented, Ghost generates statements for knex
- if we want to replace GQL with NQL, we can't generate these statements
- they are not understood from NQL, because NQL uses mongo JSON
- go through usages and rewrite the statements
- invent `extraFilters` for now
- we need to keep the support for `status` or `staticPages` for now (API requirement)
- IMO both shortcuts in the extra filters should be removed in the future

This commit is required for #10159!
@kirrg001 kirrg001 force-pushed the replace-gql branch 2 times, most recently from 7db2913 to ee5ac5a Compare November 15, 2018 14:55
@kirrg001
Copy link
Contributor Author

kirrg001 commented Nov 15, 2018

@gargol Could you pls work on getting rid of the functions preProcessFilters and postProcessFilters.

The goal should be to enable existing test cases which used these two filters in the past. If you can't find a test case, add one for now as functional test.

The test cases will be restricted by the functionality of mongo-knex..

You might need to adapt the filter plugin or the model layer to make them work.

I will work on finishing up TryGhost/mongo-knex#16.

@naz
Copy link
Contributor

naz commented Nov 15, 2018

Sure, starting on preProcessFilters and postProcessFilters now.

@kirrg001 kirrg001 changed the title [HOLD/WIP] Replaced GQL with NQL [WIP] Replaced GQL with NQL Dec 6, 2018
@kirrg001 kirrg001 changed the title 🐛 Fixed known GQL bugs 🐛 Fixed known GQL/filter limitations Dec 11, 2018
@kirrg001 kirrg001 changed the title 🐛 Fixed known GQL/filter limitations 🐛 Fixed known filter limitations Dec 11, 2018
@kirrg001
Copy link
Contributor Author

Travis is failing because of a random failure, tracked in #7470 again 🤨
Will merge anyway!

@kirrg001 kirrg001 merged commit 9d7c3bd into TryGhost:master Dec 11, 2018
daniellockyer pushed a commit to TryGhost/framework that referenced this pull request Jun 15, 2021
refs #10105

- `options.where` is an older deprecated logic
- before the filter language was invented, Ghost generates statements for knex
- if we want to replace GQL with NQL, we can't generate these statements
- they are not understood from NQL, because NQL uses mongo JSON
- go through usages and rewrite the statements
- invent `extraFilters` for now
- we need to keep the support for `status` or `staticPages` for now (API requirement)
- IMO both shortcuts in the extra filters should be removed in the future

This commit is required for TryGhost/Ghost#10159!
daniellockyer pushed a commit to TryGhost/framework that referenced this pull request Jun 15, 2021
refs #10105

- `options.where` is an older deprecated logic
- before the filter language was invented, Ghost generates statements for knex
- if we want to replace GQL with NQL, we can't generate these statements
- they are not understood from NQL, because NQL uses mongo JSON
- go through usages and rewrite the statements
- invent `extraFilters` for now
- we need to keep the support for `status` or `staticPages` for now (API requirement)
- IMO both shortcuts in the extra filters should be removed in the future

This commit is required for TryGhost/Ghost#10159!
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.

Better error handling for incorrect sql
3 participants