-
Notifications
You must be signed in to change notification settings - Fork 29
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
[wvzcpHQH] Add SSFR check for apoc.spatial procedures #305
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These tests do not seem to be passing in the CI or on my machine either. Would it be possible to fix them before resubmitting this for review?
9097787
to
7e162e7
Compare
@AzuObs ok, now it seems to work in the CI as well |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you very much for fixing the tests. It's very pleasing to see that these tests clearly demonstrate that these procedures were already protected 🎉
I left a couple of nitpicks regarding the time these tests take to run, and also regarding whether these tests could be more readable.
76875cb
to
0202999
Compare
0202999
to
9728eae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for actioning the comments. Nice work 🙌
* [wvzcpHQH] Add SSFR check for apoc.spatial procedures * [wvzcpHQH] Fix test in the CI * [wvzcpHQH] added descriptive method and speed up tests
#3451) * [wvzcpHQH] Add SSFR check for apoc.spatial procedures (neo4j/apoc#305) * [wvzcpHQH] Add SSFR check for apoc.spatial procedures * [wvzcpHQH] Fix test in the CI * [wvzcpHQH] added descriptive method and speed up tests * [wvzcpHQH] changed err. message in 4.4
It's not necessary to change the implementation,
as both
apoc.spatial.geocode
andapoc.spatial.reverseGeocode
callJsonUtil.loadJson(...)
which ends up on the work done in this pr.