Skip to content
This repository was archived by the owner on Sep 3, 2021. It is now read-only.

Filter out labels with underscores. #301

Closed
wants to merge 1 commit into from

Conversation

kraney
Copy link

@kraney kraney commented Aug 30, 2019

This is perhaps more of a workaround than a real fix

Provided interface types are named with a leading underscore, this will
allow nodes with multiple labels (the concrete type with no underscore,
plus some number of interfaces that all have an underscore prefix) to
work with fields referencing interfaces.

Provided interface types are named with a leading underscore, this will
allow nodes with multiple labels (the concrete type with no underscore,
plus some number of interfaces that all have an underscore prefix.) to
work with fields referencing interfaces.
@codecov-io
Copy link

Codecov Report

Merging #301 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #301   +/-   ##
=======================================
  Coverage   95.47%   95.47%           
=======================================
  Files          10       10           
  Lines        2345     2345           
=======================================
  Hits         2239     2239           
  Misses        106      106
Impacted Files Coverage Δ
src/translate.js 98.64% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90bd912...7348337. Read the comment docs.

1 similar comment
@codecov-io
Copy link

codecov-io commented Aug 30, 2019

Codecov Report

Merging #301 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #301   +/-   ##
=======================================
  Coverage   95.47%   95.47%           
=======================================
  Files          10       10           
  Lines        2345     2345           
=======================================
  Hits         2239     2239           
  Misses        106      106
Impacted Files Coverage Δ
src/translate.js 98.64% <ø> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90bd912...7348337. Read the comment docs.

@johnymontana
Copy link
Contributor

Thanks for the PR @kraney! And apologies for taking so long to respond.

Instead of choosing the correct interface label based on naming convention what we'd like to do is identify all the implementing types of an interface and pass those implementing types as an array of valid node labels as a Cypher parameter. Then the Cypher query can check to see which of those labels the node has and the Cypher query can return the correct implementing type.

For example, this is how we did it in neo4j-graphql-java: https://github.com/neo4j-graphql/neo4j-graphql-java/pull/51/files#diff-84c0d5102c74b3eb155b1749146d578aR167-R171

and you can see how the generated Cypher queries work with this "valid types" Cypher parameter:

https://github.com/neo4j-graphql/neo4j-graphql-java/blob/master/src/test/resources/translator-tests1.adoc#inline-fragments-on-interfaces

Would you be interested in working on that logic for this PR instead of using the underscore based convention?

@kraney
Copy link
Author

kraney commented Oct 22, 2019

OK, makes sense. If I can find time, I can give a try at that approach.

@johnymontana
Copy link
Contributor

Great. If not we'll get to it as part of fixing up interface handling (as well as ensuring the generated mutations handle the multiple label approach for interface types)

@kraney
Copy link
Author

kraney commented Nov 6, 2019

I believe this change has been obsoleted by my colleague over at #336 and can be closed.

@kraney kraney closed this Nov 6, 2019
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants