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

Conversation

@ajeetdsouza
Copy link
Contributor

@ajeetdsouza ajeetdsouza commented Jun 19, 2019

Description

The console currently allows a user to add a description to a table via Data > Schema > schema_name > table_name > Modify, but it does not show this description anywhere else. This PR adds descriptions to the GraphQL schema, viewable via the Documentation Explorer:
image

Affected components

  • Server

Related Issues

#446

Solution and Design

Descriptions are stored via Postgres comments, viewable in the pg_descriptions table. A descriptions field has been added to the hdb_catalog.hdb_table_info_agg view, and this description is concatenated with the default description when being served to the frontend.

Steps to test and verify

Add a description to any of the tables as described above. It should now be visible in the Documentation Explorer.

Limitations, known bugs & workarounds

None

To the Reviewer (server)

  • No new modules have been added.
  • The catalog version has been upgraded to 18. A migration for this has been added (from17To18), which drops the hdb_catalog.hdb_table_info_agg view and recreates it with an added description field.
  • A newtype PGDescription has been added.
  • TableInfo now has a new field for descriptions: tiDescription :: Maybe PGDescription.
  • The following functions now take an extra parameter for table descriptions: mkTableObj, mkGCtxRole, mkGCtxRole'.

Critical files:

  • src-exec/Migrate.hs
  • src-lib/Hasura/SQL/Types.hs
  • src-lib/Hasura/GraphQL/Schema.hs
  • src-lib/Hasura/RQL/Types/SchemaCache.hs
  • src-rsr/initialise.sql
  • src-rsr/migrate_from_17_to_18.sql

@ajeetdsouza ajeetdsouza added the c/server Related to server label Jun 19, 2019
@ajeetdsouza ajeetdsouza requested a review from rakeshkky June 19, 2019 09:59
@ajeetdsouza ajeetdsouza requested a review from 0x777 as a code owner June 19, 2019 09:59
@ajeetdsouza ajeetdsouza self-assigned this Jun 19, 2019
@hasura-bot
Copy link
Contributor

Beep boop! 🤖

Hey @ajeetdsouza, thanks for your PR!

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

Stay awesome! 😎

@netlify
Copy link

netlify bot commented Jun 19, 2019

Deploy preview for hasura-docs ready!

Built with commit 458506c

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

@hasura-bot
Copy link
Contributor

Review app for commit db9f137 deployed to Heroku: https://hge-ci-pull-2397.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2397-db9f137c

@rakeshkky rakeshkky changed the title Add Postgres comments to table descriptions (close #446) propagate Postgres table comments to GraphQL schema descriptions (close #446) Jun 20, 2019
@rakeshkky
Copy link
Member

@ajeetdsouza
Can a table have multiple descriptions? If yes, how are you handling them?

@ajeetdsouza
Copy link
Contributor Author

@rakeshkky
Postgres allows only one comment per object, so this won't be a problem: https://www.postgresql.org/docs/current/sql-comment.html

rakeshkky
rakeshkky previously approved these changes Jul 2, 2019
@hasura-bot
Copy link
Contributor

Review app for commit 10635b3 deployed to Heroku: https://hge-ci-pull-2397.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2397-10635b3f

@coco98
Copy link
Contributor

coco98 commented Jul 9, 2019

@ajeetdsouza Why aren't we propagating other comments? Let's just propagate all of them?
I tried column comments, but it doesn't work.

In order of priority, I would say:

  1. Tables
  2. Views
  3. Columns
  4. Functions

@hasura-bot
Copy link
Contributor

Review app for commit ac8f9b1 deployed to Heroku: https://hge-ci-pull-2397.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2397-ac8f9b12

@hasura-bot
Copy link
Contributor

Review app for commit c26324b deployed to Heroku: https://hge-ci-pull-2397.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2397-c26324b7

@coco98
Copy link
Contributor

coco98 commented Jul 16, 2019

I don't think this is correct.
I added comments to table/column.
https://hge-ci-pull-2397.herokuapp.com/console/data/schema/public/tables/test/modify

They show up in the wrong place on the graphql docs explorer

@dsandip dsandip assigned rakeshkky and unassigned ajeetdsouza Jul 23, 2019
@rakeshkky rakeshkky requested a review from lexi-lambda as a code owner July 23, 2019 11:51
@hasura-bot
Copy link
Contributor

Review app for commit 06cfe5c deployed to Heroku: https://hge-ci-pull-2397.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2397-06cfe5c1

lexi-lambda
lexi-lambda previously approved these changes Aug 16, 2019
Copy link
Contributor

@lexi-lambda lexi-lambda left a comment

Choose a reason for hiding this comment

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

@rakeshkky I think this LGTM, but could I ask you to rebase to resolve the merge conflicts?

@dsandip dsandip added this to the beta.5 milestone Aug 19, 2019
@hasura-bot
Copy link
Contributor

Review app for commit e22bebc deployed to Heroku: https://hge-ci-pull-2397.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2397-e22bebc2

@dsandip dsandip removed this from the beta.5 milestone Sep 4, 2019
Resolve Conflicts:
	server/src-exec/Migrate.hs
	server/src-lib/Hasura/GraphQL/Schema.hs
	server/src-lib/Hasura/GraphQL/Schema/BoolExp.hs
	server/src-lib/Hasura/GraphQL/Schema/Mutation/Common.hs
	server/src-lib/Hasura/GraphQL/Schema/Select.hs
	server/src-lib/Hasura/RQL/DDL/Schema/Diff.hs
	server/src-lib/Hasura/RQL/DDL/Schema/Function.hs
	server/src-lib/Hasura/RQL/DDL/Schema/Table.hs
	server/src-lib/Hasura/RQL/DML/Internal.hs
	server/src-lib/Hasura/RQL/DML/Returning.hs
	server/src-lib/Hasura/RQL/GBoolExp.hs
	server/src-lib/Hasura/RQL/Types/Common.hs
	server/src-lib/Hasura/RQL/Types/SchemaCache.hs
	server/src-rsr/catalog_metadata.sql
	server/src-rsr/initialise.sql
	server/src-rsr/migrate_from_19_to_20.sql
	server/src-rsr/table_meta.sql
@ajeetdsouza ajeetdsouza requested a review from rikinsk as a code owner September 4, 2019 09:28
@hasura-bot
Copy link
Contributor

Review app for commit 8254304 deployed to Heroku: https://hge-ci-pull-2397.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2397-8254304c

@mastoj
Copy link

mastoj commented Sep 4, 2019

I still don't believe this to be fixed. I went ahead and created a table and a column on the heroku instance. I can't see the comment I added for the column when I browse through docs.

@rakeshkky
Copy link
Member

I still don't believe this to be fixed. I went ahead and created a table and a column on the heroku instance. I can't see the comment I added for the column when I browse through docs.

@mastoj Thank you reporting this. This has been fixed in the next commit.

@hasura-bot
Copy link
Contributor

Review app for commit 10f2ec0 deployed to Heroku: https://hge-ci-pull-2397.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2397-10f2ec06

@mastoj
Copy link

mastoj commented Sep 4, 2019

@rakeshkky nice. Verified that it works.

@hasura-bot
Copy link
Contributor

Review app for commit 4c5b326 deployed to Heroku: https://hge-ci-pull-2397.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2397-4c5b3266

@rakeshkky
Copy link
Member

rakeshkky commented Sep 13, 2019

@lexi-lambda Ready to be merged. But please have a final glance. :)

@hasura-bot
Copy link
Contributor

Review app for commit 4111019 deployed to Heroku: https://hge-ci-pull-2397.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2397-41110199

@hasura-bot
Copy link
Contributor

Review app for commit 458506c deployed to Heroku: https://hge-ci-pull-2397.herokuapp.com
Docker image for server: hasura/graphql-engine:pull2397-458506c6

@lexi-lambda lexi-lambda merged commit 99174cc into hasura:master Sep 17, 2019
@hasura-bot
Copy link
Contributor

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

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

Labels

c/server Related to server

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants