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

console: show only compatible postgres functions in computed fields section (close #5155) #5978

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
Closed
wants to merge 7 commits into from
Closed

console: show only compatible postgres functions in computed fields section (close #5155) #5978

wants to merge 7 commits into from

Conversation

@GrizliK1988
Copy link
Contributor

@GrizliK1988 GrizliK1988 commented Oct 10, 2020

Show only compatible postgres functions in computed fields section

Description

When computed field is being created, user will see only functions that have table row as one of arguments

Changelog

  • CHANGELOG.md is updated with user-facing content relevant to this PR. If no changelog is required, then add the no-changelog-required label.

Affected components

  • Console

Related Issues

#5155

Solution and Design

At the beginning I planned to adjust SQL query to fetch only needed functions like

SELECT * FROM hdb_catalog.hdb_function_agg WHERE "input_arg_types"::jsonb @> '[{"schema":"table_schema","name":"table_name"}]'::jsonb;

Unfortunately backend doesn't accept json fields in where statements and I adjusted filtering on js side, which shouldn't be a problem, until user will have thousands of functions.

Steps to test and verify

There is a new test case in Cypress to verify functionality.

@GrizliK1988 GrizliK1988 requested review from a team as code owners October 10, 2020 13:59
@hasura-bot
Copy link
Contributor

hasura-bot commented Oct 10, 2020

Beep boop! 🤖

Hey @GrizliK1988, thanks for your PR!

One of my human friends will review this PR and get back to you as soon as possible.

Stay awesome! 😎

@beerose beerose self-assigned this Oct 10, 2020
@beerose beerose added the hacktoberfest-accepted Valid hacktoberfest PR label Oct 10, 2020
beerose
beerose suggested changes Oct 16, 2020
View reviewed changes
Copy link
Contributor

@beerose beerose left a comment

Choose a reason for hiding this comment

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

Thanks for your PR!

Filtering out functions that don't accept table row argument works great!

However, we could go one step forward and handle a few more requirements mentioned here: https://hasura.io/docs/1.0/graphql/core/schema/computed-fields.html#supported-sql-functions.

Specifically, we could:

  • Check for the function_type and accept only STABLE or IMMUTABLE
  • Check for the input arg types (we need 'b' — base or 'c' — composite)

Screenshot 2020-10-16 at 19 16 48

console/cypress/integration/data/modify/test.ts Outdated
it('Moving to the table', passMTMoveToTable);
it('Creating functions', passMTCreateFunctions);
it('Modify table button opens the correct route', passMTCheckRoute);
it('See only functions for current table', passMTFunctionList);
Copy link
Contributor

@beerose beerose Oct 16, 2020

Choose a reason for hiding this comment

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

Suggested change
it('See only functions for current table', passMTFunctionList);
it('Can create comupted field with compatibile functions', passMTFunctionList);

Copy link
Contributor Author

@GrizliK1988 GrizliK1988 Oct 16, 2020

Choose a reason for hiding this comment

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

Hi @beerose,

Thank you for your feedback! Described changes look good to me and I'll apply them in upcoming days.

Copy link
Contributor Author

@GrizliK1988 GrizliK1988 Oct 17, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Hi!

Code is updated. VOLATILE functions and functions with unsupported arguments are now filtered out.

Thanks for your suggestions once again!

@netlify
Copy link

netlify bot commented Oct 17, 2020 •
edited
Loading

Deploy preview for hasura-docs ready!

Built with commit 0fd2050

https://deploy-preview-5978--hasura-docs.netlify.app

@GrizliK1988 GrizliK1988 force-pushed the issue_5155 branch 2 times, most recently from 6e49f76 to 8a19794 Compare October 17, 2020 15:00
@GrizliK1988 GrizliK1988 force-pushed the issue_5155 branch 2 times, most recently from 60ebefb to c177e1d Compare October 17, 2020 16:07
beerose
beerose approved these changes Nov 2, 2020
View reviewed changes
Copy link
Contributor

@beerose beerose left a comment

Choose a reason for hiding this comment

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

Thank you again for your PR and for taking time to add tests! 🎉

Approving console changes

beerose
beerose approved these changes Jan 15, 2021
View reviewed changes
Copy link
Contributor

@beerose beerose left a comment

Choose a reason for hiding this comment

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

Console

@hasura-bot hasura-bot closed this Jan 26, 2021
hasura-bot added a commit that referenced this pull request Jan 26, 2021
@hasura-bot @GrizliK1988 @beerose
console: show only compatible postgres functions in computed fields s…
3cac1c3 
…ection (close #5155)

GITHUB_PR_NUMBER: 5978
GITHUB_PR_URL: #5978

Co-authored-by: Dmitry Grachikov <696824+GrizliK1988@users.noreply.github.com>
Co-authored-by: Aleksandra Sikora <9019397+beerose@users.noreply.github.com>
GitOrigin-RevId: 9399fae
@hasura-bot
Copy link
Contributor

hasura-bot commented Jan 26, 2021

Beep boop! 🤖

GIF

Awesome work @GrizliK1988!

Your changes were merged successfully. All of us at Hasura ❤️ what you did.

Thanks again 🤗

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

2 more reviewers

@beerose beerose beerose approved these changes

@shaonkabir8 shaonkabir8 shaonkabir8 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@beerose beerose

Labels

c/console Related to console community-contrib 🙏🏽 hacktoberfest-accepted Valid hacktoberfest PR merged

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@GrizliK1988 @hasura-bot @beerose @shaonkabir8