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

Filtering behaviour isn't comprehensively documented #479

Closed
pcraig3 opened this issue Apr 12, 2018 · 2 comments
Closed

Filtering behaviour isn't comprehensively documented #479

pcraig3 opened this issue Apr 12, 2018 · 2 comments

Comments

@pcraig3
Copy link
Contributor

pcraig3 commented Apr 12, 2018

feedback on the docs

The first thing to say on this, before anything else, is that your docs are very well done, Eva. Both in tone and in content, they are fantastic. My comments below are mostly nitpicks on top of what currently exists as excellent documentation.

👏 👏 👏 👏 👏 👏 👏 👏 👏 👏 👏

So let's get into it. I've tried to put them in rough order of (my idea of) importance.

filtering on non-dwelling fields returns more than you would expect

Filtering works by looking for at least one matching value and then returning a matching dwelling with all of its data.

For example, searching for a specific evaluation file ID will return the dwelling containing that evaluation, along with all other evaluations belonging to that dwelling.

As an API consumer, once you have the returned dwelling, you’ll know it contains an evaluation with the file ID you wanted, but you still have to loop through them to find the one that you want.

I don't know if there's a more concise way to explain this behaviour, but we should mention it because I think it's surprising that it works this way.

filters are always AND, never OR

Our filters are all AND filters, which means that returned results must match all conditions to be returned.

This means that these queries are possible:

  • return all dwellings built in 1970
  • return all dwellings built in 1970 in Oakville

But these queries are not possible

  • return all dwellings built in 1970 or 1975
  • return all dwellings built in 1970 in either Ottawa or Toronto

what are the comparators

Fairly easy, this one. We've have picked a subset of all of the comparisons mongodb lets you do, that can be done through our API. Specifically, we let you use:

  • greater than
  • equal to
  • less than

The list of defined comparators is just written into the schema, which is pretty cool.

Other operations mongodb allows (not equal to (!==), less than or equal to (<=), etc.) are not supported. It would not be so hard to add more comparators, but for now this is what's on offer.

Quick aside

Also, might be worth quickly mentioning that all comparison values must be string values. Even if you are comparing against yearBuilt, for example (yearBuilt is returned as an integer), you still have to write your filter like filters: [{field: dwellingYearBuilt comparator: eq value: "1970"}])

example queries missing subfields won't work in pure graphql

The example queries which return subfields won't work in pure GraphQL. For example, this query

query {
  dwellings 
}

Results in this error message if you just sent it straight to GraphQL.

{
  "errors":[{
    "locations":[{"column":2,"line":1}],
    "message":"Field \"dwellings\" of type \"PaginatedResultSet\" must have a selection of subfields. Did you mean \"dwellings { ... }\"?"
   }]
}

GraphiQL takes care of entering the subfields for us, but someone else using a different tool might run into problems.

(This is pretty minor, I think.)

@pcraig3 pcraig3 changed the title Filtering behaviour isn't comprehensively documented [WIP] Filtering behaviour isn't comprehensively documented Apr 12, 2018
@evadb
Copy link
Contributor

evadb commented Apr 12, 2018

Hey! I took a stab at documenting filtering here: https://github.com/cds-snc/nrcan_api/blob/master/docs/using_the_api.md

Maybe something can be added to make it more clear?

@pcraig3 pcraig3 changed the title [WIP] Filtering behaviour isn't comprehensively documented Filtering behaviour isn't comprehensively documented Apr 13, 2018
@evadb
Copy link
Contributor

evadb commented Apr 16, 2018

This has been addressed

@evadb evadb closed this as completed Apr 16, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants