Graphql sorting #572
Merged
Graphql sorting #572
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
msprunck
force-pushed
the
graphql-sorting
branch
from
d94fb7f
to
094265d
Compare
msprunck
force-pushed
the
graphql-sorting
branch
2 times, most recently
from
65eb458
to
ac28ae7
Compare
- Renamed source and target Relationship fields to source_entity and target_entity. source was in conflict with an existing entity field. - Removed the specification sort field for Indicators. It's an union of several schemas (ThreatBrainSpecification, SnortSpecification, ...), there is no common field to sort on it.
yogsototh
approved these changes
Jun 16, 2017
| @@ -17,7 +18,7 @@ | |||
| :startCursor {:type Scalars/GraphQLString} | |||
| :endCursor {:type Scalars/GraphQLString}})) | |||
|
|
|||
| (def connection-arguments | |||
| (def connection-arguments ; | |||
There was a problem hiding this comment.
I think leaving the ; here is just a typo
test/ctia/test_helpers/graphql.clj
Outdated
| (str "Nodes are correctly sorted by " sort-field)) | ||
| (is (not-empty (:edges connection-data))) | ||
| (is (= edges-ref edges) | ||
| (str "Edges are correctly sorted by " sort-field)))))) |
There was a problem hiding this comment.
perhaps replace the "are" by "should be" in the test messages.
test/ctia/test_helpers/graphql.clj
Outdated
| operation-name) | ||
| connection-data (get-in data connection-path) | ||
| nodes (->> (:nodes connection-data) | ||
| (map sort-field-fn) |
There was a problem hiding this comment.
as sort-field-fn is used by the the graphql query. I think it would be nice to have one manual test which compare datas directly to check that sort is doing something.
craigbro
approved these changes
Jun 16, 2017
There was a problem hiding this comment.
I did not see any glaring issues after a quick review.
I do think we will need a special sort field/keyword for the "UI ordering" of judgements. Please coordinate with @polygloton on that since he is working on implementing an ES sort fragement that handles the sorting.
msprunck
force-pushed
the
graphql-sorting
branch
from
ac28ae7
to
5639934
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Connected to #571