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

Added support for additional labels (multi-tenancy via labels) #282

Merged
merged 11 commits into from
Aug 12, 2019
Merged

Added support for additional labels (multi-tenancy via labels) #282

merged 11 commits into from
Aug 12, 2019

Conversation

cmartin81
Copy link
Contributor

@cmartin81 cmartin81 commented Jul 24, 2019

I've implemented ability to add additional labels inside the schema for objects. The implementation is inspired by issue #237 (Auth support for multi-tenant graphs implemented via node labels)

Example of usage:
Use directive @additionalLabels which has 1 parameter 'labels' .
Labels is an array of strings. It also supports string interpolation (See example under) using lodash.template lib. The varible $cypherPrams is also accessible within the string

type Movie @additionalLabels(labels: ["u_<%= $cypherParams.userId %>", "newMovieLabel"])  {
    id: String
    name: String
    ...
    ...
    ...

When creating, updating or deleting it will add 2 new labels to the query. For this case it will be u_123 and newMovieLabel given that $cypherParams.userId = 123. This way it should be possible to add multi-tenant graph via labels.)
Labels are only applied on node (not edges)

Tests have been changed and all 110 are passing

Verified

This commit was signed with the committer’s verified signature.
cmartin81 Christian Martin

Verified

This commit was signed with the committer’s verified signature.
cmartin81 Christian Martin
Deleted pretest script since ava will precompile it out-of-the-box

Verified

This commit was signed with the committer’s verified signature.
cmartin81 Christian Martin
…ot dist)

Verified

This commit was signed with the committer’s verified signature.
cmartin81 Christian Martin
…ypherParams interpolation)

Verified

This commit was signed with the committer’s verified signature.
cmartin81 Christian Martin

Verified

This commit was signed with the committer’s verified signature.
cmartin81 Christian Martin

Verified

This commit was signed with the committer’s verified signature.
cmartin81 Christian Martin

Verified

This commit was signed with the committer’s verified signature.
cmartin81 Christian Martin
@cmartin81
Copy link
Contributor Author

cmartin81 commented Jul 25, 2019

I fixed the CI script myself since it was inside the source code :)

Verified

This commit was signed with the committer’s verified signature.
cmartin81 Christian Martin

Verified

This commit was signed with the committer’s verified signature.
cmartin81 Christian Martin
@cmartin81
Copy link
Contributor Author

I'm using this for the last week and it looks like everything works great.
Are there anything I can do to get this PR accepted?

@johnymontana
Copy link
Contributor

Hey @cmartin81 - thanks for the PR.

This is really neat when combined with cypherParams. I left just a minor request for changes in the test-all npm script, but otherwise, I think this is good to go.

We may want to rethink how this is handled when Neo4j v4.0 is released, which will include new access control features, but since this is useful for your use case I think it will be useful for others as well.

Would you mind adding another PR to the docs showing how to use this? Probably as an additional item in this file. Thanks!

@codecov-io
Copy link

Codecov Report

❗ No coverage uploaded for pull request base (master@a3376c4). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #282   +/-   ##
=========================================
  Coverage          ?   86.45%           
=========================================
  Files             ?       10           
  Lines             ?     2348           
  Branches          ?        0           
=========================================
  Hits              ?     2030           
  Misses            ?      318           
  Partials          ?        0
Impacted Files Coverage Δ
src/selections.js 87.35% <ø> (ø)
src/utils.js 93.17% <100%> (ø)
src/translate.js 98.64% <100%> (ø)

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 a3376c4...103396b. Read the comment docs.

@cmartin81
Copy link
Contributor Author

Continuous_Integration_and_Deployment

Here you can see. The failing tests have nothing to do with my work.

@johnymontana johnymontana merged commit 103396b into neo4j-graphql:master Aug 12, 2019
@johnymontana
Copy link
Contributor

Sorry about that, that was my fault :-\

I've fixed the integration tests and merged this in. Thanks for adding this feature!

@cmartin81
Copy link
Contributor Author

Great! Thanks!
Remember that I have updated the docs and made an PR.
grand-stack/grandstack.io#3

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants