-
Notifications
You must be signed in to change notification settings - Fork 2.8k
reuse buttons across console for uniformity #1400
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
Conversation
…e-code-reusability
|
Review app for commit 9d9bef7 deployed to Heroku: https://hge-ci-pull-1400.herokuapp.com |
|
Review app for commit fcbfaf8 deployed to Heroku: https://hge-ci-pull-1400.herokuapp.com |
|
Review app for commit 866c9c9 deployed to Heroku: https://hge-ci-pull-1400.herokuapp.com |
|
Review app for commit 8cd52e9 deployed to Heroku: https://hge-ci-pull-1400.herokuapp.com |
praveenweb
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Review app https://hge-ci-pull-1400.herokuapp.com is deleted |
… by expressions (#1400) <!-- The PR description should answer 2 important questions: --> ### What This PR corrects an issue where model permissions are not applied to models used via relationships in order by expressions. ### How The permissions are now correctly read out of the GraphQL schema annotations and applied it to the predicate inside the relationship path in order by targets in `crates/graphql/ir/src/order_by.rs`. Because there are now predicates inside order by expressions, the execution pipeline had to be updated to handle evaluating remote predicates inside order by expressions too. The tests in `crates/engine/tests/execute/models/select_many/order_by/relationships/` have been updated to test applied model permissions affecting ordering. V3_GIT_ORIGIN_REV_ID: 783c204c44099e5319580e51e7549527d7a56245
…n the connector does not support it (#1443) ### What This PR blocks the usage of nested fields and nested relationships (where a relationship starts from a nested object) in order_by clauses when the connector does not support it. NDC v0.1.x has a capability for ordering by nested fields, and does not support ordering by nested relationships at all. NDC 0.2.0 adds support (and a capability) for ordering by nested relationships. It also blocks being able to configure non-scalar fields as orderable in Model v1. These new errors are only raised if the new OpenDD flags are set. The compatibility date to enable these flags is currently unset, pending a production release date. Actual support for nested relationships in ordering was "accidentally" added in #1400, when relationship paths in order by targets were properly modelled and implemented. ### How Two new flags were added to OpenDD to cover the two new breaking changes: `require_nested_support_for_order_by_expressions` and `disallow_model_v1_ordering_non_scalar_fields`. Logic in `crates/metadata-resolve/src/stages/models/order_by.rs` was refactored and enhanced to check compatibility of order by expressions used in a model against the model's source data connector. Metadata resolve tests have been updated to test nested relationships in ordering, and new tests added to test the new errors. Some existing tests needed fixing to comply with the new logic without raising warnings. A new `execute` test has been added to cover ordering by nested relationships in `crates/engine/tests/execute/models/select_many/order_by/nested_relationships/`. V3_GIT_ORIGIN_REV_ID: 3f4e01cddbbd80c4f190260df56dbdb13d0a1253
Description
What component does this PR affect?
Requires changes from other components? If yes, please mark the components:
Related Issue
Solution and Design
Type
Checklist: