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

console: permission builder fixes #3823

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Feb 4, 2020
Merged

console: permission builder fixes #3823

merged 8 commits into from
Feb 4, 2020

Conversation

@rikinsk
Copy link
Member

@rikinsk rikinsk commented Feb 3, 2020 •
edited
Loading

Description

Affected components

  • Server
  • Console
  • CLI
  • Docs
  • Community Content
  • Build System
  • Tests
  • Other (list it)

Related Issues

Solution and Design

Steps to test and verify

Limitations, known bugs & workarounds

@rikinsk rikinsk added c/console Related to console s/wip Status: This issue is a work in progress labels Feb 3, 2020
@netlify
Copy link

netlify bot commented Feb 3, 2020 •
edited
Loading

Deploy preview for hasura-docs ready!

Built with commit be1cbfa

https://deploy-preview-3823--hasura-docs.netlify.com

@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 3, 2020

Review app for commit 60809ec deployed to Heroku: https://hge-ci-pull-3823.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3823-60809ec3

@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 4, 2020

Review app for commit 2b4cf6c deployed to Heroku: https://hge-ci-pull-3823.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3823-2b4cf6c4

@rikinsk rikinsk marked this pull request as ready for review February 4, 2020 09:30
@rikinsk rikinsk removed the s/wip Status: This issue is a work in progress label Feb 4, 2020
@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 4, 2020

Review app for commit 9aa3745 deployed to Heroku: https://hge-ci-pull-3823.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3823-9aa37457

@rikinsk rikinsk requested a review from beerose February 4, 2020 10:13
beerose
beerose reviewed Feb 4, 2020
View reviewed changes
console/src/components/Services/Data/TablePermissions/Permissions.js Outdated
dispatch(fetchFunctionInit());
}

componentWillReceiveProps(nextProps) {
Copy link
Contributor

@beerose beerose Feb 4, 2020

Choose a reason for hiding this comment

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

componentWillReceiveProps will be deprecated and in React 16.3 there was getDerivedStateFromProps introduced to replace it. So can we use this instead?

static getDerivedStateFromProps(nextProps, prevState) {
    const currPermissionsState = prevState.permissionsState;
    const nextPermissionsState = nextProps.permissionsState;

    if (
      currPermissionsState.role !== nextPermissionsState.role ||
      currPermissionsState.query !== nextPermissionsState.query
    ) {
      return { filterString: '' };
    }
    return null;
}

Copy link
Member Author

@rikinsk rikinsk Feb 4, 2020

Choose a reason for hiding this comment

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

I didnt go down that route as I would then have to store the previousProps in state which I wanted to avoid. Is there any other way I am not aware of?

In any case, we should be converting all these to hooks anyway when we refactor so it might not be worth adding a lot of code to achieve this

Copy link
Member Author

@rikinsk rikinsk Feb 4, 2020

Choose a reason for hiding this comment

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

Figured out an easy workaround

@rikinsk rikinsk requested a review from beerose February 4, 2020 11:43
@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 4, 2020

Review app for commit be1cbfa deployed to Heroku: https://hge-ci-pull-3823.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3823-be1cbfac

@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 4, 2020

Review app for commit 8fc27c3 deployed to Heroku: https://hge-ci-pull-3823.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3823-8fc27c31

@rikinsk rikinsk merged commit 916ccc2 into hasura:master Feb 4, 2020
@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 4, 2020

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

@rikinsk rikinsk deleted the console-permissions-fixes branch February 4, 2020 19:03
@rikinsk rikinsk mentioned this pull request Feb 4, 2020
Closed
@hasura-bot
Copy link
Contributor

hasura-bot commented Feb 4, 2020

Review app for commit fca8247 deployed to Heroku: https://hge-ci-pull-3823.herokuapp.com
Docker image for server: hasura/graphql-engine:pull3823-fca82476

polRk pushed a commit to polRk/graphql-engine that referenced this pull request Feb 12, 2020
@rikinsk @polRk
console: permission builder fixes (hasura#3823)
8a46dc2 
* allow manually editing permissions (hasura#1734)
* separate operators for json and jsonb column types
* allow null as valid JSON type
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@beerose beerose beerose approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

c/console Related to console

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@rikinsk @hasura-bot @beerose