Skip to content
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

console: mark inconsistent schemas in the UI (close #5093) #5181

Merged
merged 44 commits into from
Aug 24, 2020
Merged

console: mark inconsistent schemas in the UI (close #5093) #5181

merged 44 commits into from
Aug 24, 2020

Conversation

kolharsam
Copy link
Contributor

@kolharsam kolharsam commented Jun 23, 2020 •
edited

closes #5093

Description

Inconsistent Remote Schemas are not rendered on the Remote Schema page on updating metadata. This PR fixes that and allows users to modify the inconsistent schema from the Remote Schema page

Changelog (required)

  • 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

@kolharsam kolharsam added s/do-not-merge Do not merge this pull request to master c/console Related to console no-changelog-required labels Jun 23, 2020
@kolharsam kolharsam self-assigned this Jun 23, 2020
@hasura-bot
Copy link
Contributor

Review app for commit c9deb67 deployed to Heroku: https://hge-ci-pull-5181.herokuapp.com
Docker image for server: hasura/graphql-engine:pull5181-c9deb670

@hasura-bot
Copy link
Contributor

Review app for commit ffc6958 deployed to Heroku: https://hge-ci-pull-5181.herokuapp.com
Docker image for server: hasura/graphql-engine:pull5181-ffc69588

@kolharsam kolharsam marked this pull request as ready for review June 24, 2020 15:08
@kolharsam kolharsam requested review from a team as code owners June 24, 2020 15:08
kolharsam
kolharsam commented Jun 24, 2020
View reviewed changes
Copy link
Contributor Author

@kolharsam kolharsam left a comment •
edited

Choose a reason for hiding this comment

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

@beerose right now, this PR accomplishes all that was mentioned on #5093, to render a different icon along with a tooltip if the remote schema is inconsistent. But I noticed that there are the following problems that we probably must address:
- Reload button which is present on the View page of the Remote Schema page does not work as expected - goes to a 404
- Reload Metadata button also fails due to some redux error that happens on the Remote Schema page.

These issues were handled.

console/src/components/Services/RemoteSchema/Actions.js Outdated Show resolved Hide resolved
console/src/components/Services/Settings/MetadataOptions/ReloadRemoteSchema.js Outdated Show resolved Hide resolved
@hasura-bot
Copy link
Contributor

Review app for commit 16100f7 deployed to Heroku: https://hge-ci-pull-5181.herokuapp.com
Docker image for server: hasura/graphql-engine:pull5181-16100f70

@hasura-bot
Copy link
Contributor

Review app for commit dddcc5d deployed to Heroku: https://hge-ci-pull-5181.herokuapp.com
Docker image for server: hasura/graphql-engine:pull5181-dddcc5d4

@kolharsam kolharsam removed the s/do-not-merge Do not merge this pull request to master label Jun 25, 2020
@hasura-bot
Copy link
Contributor

Review app for commit acaf3c0 deployed to Heroku: https://hge-ci-pull-5181.herokuapp.com
Docker image for server: hasura/graphql-engine:pull5181-acaf3c04

@hasura-bot
Copy link
Contributor

Review app for commit 1ef1bd8 deployed to Heroku: https://hge-ci-pull-5181.herokuapp.com
Docker image for server: hasura/graphql-engine:pull5181-1ef1bd8f

@beerose
Copy link
Contributor

beerose commented Jul 10, 2020

Please apply the same padding as in Data section. Also, I guess something went wrong with the tooltip.
Screenshot 2020-07-10 at 15 43 59

@beerose beerose removed their assignment Jul 10, 2020
@hasura-bot
Copy link
Contributor

Review app for commit 5d31c80 deployed to Heroku: https://hge-ci-pull-5181.herokuapp.com
Docker image for server: hasura/graphql-engine:pull5181-5d31c80c

kolharsam
kolharsam commented Jul 10, 2020
View reviewed changes
console/src/components/Services/RemoteSchema/Edit/Edit.js Outdated
Comment on lines 244 to 247
<small>
Note: Please make sure to rectify the errors present in the schema
before trying to modify it's details.
</small>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@beerose added this note to sort of mitigate the problem that arises when we try to modify inconsistent schemas. I'll remove this if it is not required.

Copy link
Contributor

@beerose beerose Jul 12, 2020 •
edited

Choose a reason for hiding this comment

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

I think it should only be present if the remote schema is inconsistent. It may be confusing IMO. However, if getting this information on the modify page is non-trivial then we can skip it.

Copy link
Member

Choose a reason for hiding this comment

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

Updated this to be display msg on details page and disabling modify/delete btns

kolharsam
kolharsam commented Jul 10, 2020
View reviewed changes
Copy link
Contributor Author

@kolharsam kolharsam left a comment •
edited

Choose a reason for hiding this comment

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

Self-review note(s)

@hasura-bot
Copy link
Contributor

Review app for commit 9a52ad8 deployed to Heroku: https://hge-ci-pull-5181.herokuapp.com
Docker image for server: hasura/graphql-engine:pull5181-9a52ad8d

@kolharsam kolharsam requested a review from beerose July 10, 2020 18:14
beerose
beerose previously approved these changes Jul 12, 2020
View reviewed changes
@beerose beerose self-requested a review July 12, 2020 16:47
@beerose beerose dismissed their stale review July 12, 2020 16:48

One issue needs to be resolved.

@beerose beerose assigned rikinsk and unassigned kolharsam Jul 13, 2020
rikinsk
rikinsk approved these changes Aug 24, 2020
View reviewed changes
Copy link
Member

@rikinsk rikinsk left a comment

Choose a reason for hiding this comment

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

UX approved

@rikinsk rikinsk assigned beerose and unassigned rikinsk Aug 24, 2020
@hasura-bot
Copy link
Contributor

Review app for commit 0550bd2 deployed to Heroku: https://hge-ci-pull-5181.herokuapp.com
Docker image for server: hasura/graphql-engine:pull5181-0550bd2f

@hasura-bot
Copy link
Contributor

Review app for commit 1bd0da9 deployed to Heroku: https://hge-ci-pull-5181.herokuapp.com
Docker image for server: hasura/graphql-engine:pull5181-1bd0da9d

@beerose beerose changed the title console: mark inconsistent schemas console: mark inconsistent schemas in the UI (close #5093) Aug 24, 2020
@beerose beerose merged commit ad3cfb4 into hasura:master Aug 24, 2020
@hasura-bot
Copy link
Contributor

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

stevefan1999-personal pushed a commit to stevefan1999-personal/graphql-engine that referenced this pull request Sep 12, 2020
@stevefan1999-personal
console: mark inconsistent schemas in the UI (close hasura#5093) (has…
2fad4ac 
…ura#5181)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rikinsk rikinsk rikinsk approved these changes

@beerose beerose beerose approved these changes

Assignees

@beerose beerose

Labels
c/console Related to console
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

console: mark inconsistent remote schemas in the UI
4 participants
@kolharsam @hasura-bot @beerose @rikinsk