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.

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

Use GraphQL version of Relate-API when available (in Neo4j Desktop) #979

Merged
merged 6 commits into from
Jan 13, 2020

Conversation

oskarhane
Copy link
Member

@oskarhane oskarhane commented Oct 24, 2019

Replacement of the reverted #969

Neo4j Browser should behave just as before, but using the GraphQL version rather than the injected version of the relate-api.
We're using the Apollo lib as our client.

The GraphQL queries are loaded on build so we don't need to process AST's on the client side, see https://www.apollographql.com/docs/react/recipes/webpack/.

  • No changes when operating standalone
  • Automatically connect to running graph in Neo4j Desktop
  • Automatically switch to connect to the active graph when user starts a different one in Neo4j Desktop and has Neo4j Browser open
  • Adapt to color theme preferences set in macOS (not working yet, might not be implemented in Neo4j Desktop, need to check) (not implemented on desktop side)
  • Update e2e tests (removed them)*

Note: We're still using one function from the injected API and that's neo4jDesktopApi.getKerberosTicket since it's not ported to the GraphQL API yet.

  • The relate-api will provide a graphql server that can be used for E2E test for graph apps. E2E tests for the desktop env will be added again when that is available.

Should not be merged until Neo4j Desktop implements the relate-api correctly.

@oskarhane oskarhane requested a review from linuslundahl January 7, 2020 14:16
@oskarhane oskarhane requested review from huboneo and removed request for linuslundahl January 10, 2020 08:32
@oskarhane
Copy link
Member Author

You reviewed the original PR (linked in description) for this @huboneo, could I ask you to have a look at this new one as well?

Copy link
Contributor

@huboneo huboneo left a comment

Choose a reason for hiding this comment

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

code LGTM. Havent tested

src/browser/components/relate-api/relate-api.hooks.jsx Outdated Show resolved Hide resolved
src/browser/components/relate-api/relate-api.jsx Outdated Show resolved Hide resolved
Use relate-api to connect to neo4j in desktop env

Refactor and isolate the relate-api code a bit

Fix test failures from switching to GraphQL relate-api

Fix rebase conflicts

Undo dev changes

Remove unrelevant e2e tests

Remove scheme from http endpoint
@oskarhane oskarhane merged commit 7346c36 into neo4j:master Jan 13, 2020
@oskarhane oskarhane deleted the relate-api-take-2 branch January 13, 2020 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants