这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@karthikvt26
Copy link
Contributor

Passes x-hasura-* headers as part of the user object in the body and removes it from reqHeader as analyze is admin only endpoint

#1457

Description

Previously all the analyze requests were sent with user: { 'x-hasura-role': 'admin'}. This allows passing arbitrary x-hasura-* as part of the user object

@0x777 It removes the x-hasura-* values from the request headers and puts it inside the user object inside body.

What component does this PR affect?

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Requires changes from other components? If yes, please mark the components:

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System

Related Issue

Solution and Design

Type

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Docs update
  • Community content

Checklist:

  • I have read the contributing guide and my code conforms to the guidelines.
  • This change requires a change in the documentation.
  • I have updated the documentation accordingly.
  • I have added required tests.

…d removes it from reqHeader as analyze is admin only endpoint
@hasura-bot
Copy link
Contributor

Review app for commit 1b13ff7 deployed to Heroku: https://hge-ci-pull-1459.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1459-1b13ff7


// Check if x-hasura-role is available in some form in the headers
const totalHeaders = Object.keys(reqHeaders);
for (let i = 0; i < totalHeaders.length; i += 1) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we use totalHeaders.map instead of for syntax?

for (let i = 0; i < totalHeaders.length; i += 1) {
// If header has x-hasura-*
const lHead = totalHeaders[i].toLowerCase();
if (lHead.indexOf('x-hasura-') !== -1) {
Copy link
Member

Choose a reason for hiding this comment

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

use slice?

@0x777
Copy link
Member

0x777 commented Jan 24, 2019

The functionality looks good to me. We'll need to improve the explain UI though, showing the role for which it was analyzed for. Maybe we can open another issue.

0x777
0x777 previously approved these changes Jan 24, 2019
Copy link
Member

@0x777 0x777 left a comment

Choose a reason for hiding this comment

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

LGTM.

@hasura-bot
Copy link
Contributor

Review app for commit 86b907c deployed to Heroku: https://hge-ci-pull-1459.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1459-86b907c

@hasura-bot
Copy link
Contributor

Review app for commit 8950545 deployed to Heroku: https://hge-ci-pull-1459.herokuapp.com
Docker image for server: hasura/graphql-engine:pull1459-8950545

@0x777 0x777 merged commit 8342a96 into hasura:master Jan 24, 2019
@hasura-bot
Copy link
Contributor

Review app https://hge-ci-pull-1459.herokuapp.com is deleted

hasura-bot pushed a commit that referenced this pull request Dec 19, 2024
### What
Consider the following query:

```graphql
query {
institution_in_a_country_with_a_city_named_stockholm: InstitutionMany(
    where: { location: { country: { cities: { name: { _eq: "Stockholm" } } } } }
  ) {
    id
    location {
      country {
        cities {
          name
        }
      }
    }
  }
}
```

We are filtering by the object field `name` inside the nested array
`cities` field, that itself is nested inside `location.countries`. This
previously did not work because the nested field pathing sent to the
connector was incorrect.

### How

The code did not correctly set the `column` vs the `field_path` on
`Expression::LocalNestedArray`. This has been fixed.

The test postgres DB schema has been updated to add the `cities` array
in order to facilitate testing this.

The justfile script that is used to update the postgres schema in test
DataConnectorLinks got modifies as the schema started to exceed the size
limit that can be sent via command line args. It now writes the schema
and capabilities to temporary files and feeds them into `jq`, instead of
using command line args to pass in schema and capabilities.

V3_GIT_ORIGIN_REV_ID: aca7a0b8deac91355a21be6bdab4edcf617ff2e1
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.

4 participants