Returning single record only (postgres) about objection-graphql HOT 14 CLOSED

This same thing happened to me, @paulbjensen, for me it was a query like:
{
snapshots {name,id}
}
And I would only get one record at a time.

What fixed it for me was doing instead:

{
snapshotss {name, id}
}

It looks strange, but adding the "s" cleared things up right away. Categories also ends in "s", maybe try "categoriess" instead.

from objection-graphql.

@PatrickBeamECR thanks for the tip. I will give it a try in a bit, and try and dig into what is happening there.

from objection-graphql.

So work project ate my homework, but I've checked this now, and can confirm that putting an extra s on the end returns the records, so it looks like a bug with pluralization.

Thank you so much for finding this, I would never have discovered it and just assumed that I was doing something wrong. I will dig into the code a bit more into the source code and see whats going in.

from objection-graphql.

Of course! I found it completely by accident trying to figure the same thing out--held down the 's' a bit too long and ran it. Best of luck to you.

from objection-graphql.

Here's the bit of code that appends the s on the end:

https://github.com/Vincit/objection-graphql/blob/master/lib/SchemaBuilder.js#L119

Going to keep digging around to understand the context around singleFieldName and listFieldName, as I wasn't aware of this distinction.

from objection-graphql.

I'd like to propose this change.

Given a model with a modelClass value of Category, the singleFieldName value should be 'category', and the listFieldName value should be 'categories'.

I will fork the repo and make the change there, and then submit a PR if it's working fine and there's no objection to the idea (I love a good pun).

from objection-graphql.

Actually, I explored the code a bit more and realised that passing some additional options to the model builder worked fine:

const graphQlSchema = graphQlBuilder()
	.model(Category, { fieldName: 'category', listFieldName: 'categories' })
	.build();

One issue that remains though is that although the single field result is one item, the database query is still making a request for all of the records in the database table. Ideally we'd want to pass a LIMIT 1 to the database query if returning a single record to the result

from objection-graphql.

I scanned the code a bit more and realised that 'limit' and 'offset' options were used for the 'range' query argument's function, but had no separate implementations, so I've added them in this PR:

#45

There is one lingering question that remains. When requesting a single object for a type, would it make sense for limit: 1 to be applied to this, as it would stop us making a request that fetches all of the records in the database table and returns the first, and actually tells the DB to return 1 record.

from objection-graphql.

Hi @paulbjensen , thanks for the PR. I agree, that the default pluralization rules here are primitive and adding 's' to the end of the word doesn't make sense for many cases. I'll think how to make it work for more cases out of the box and maybe utilize some external libraries that solve this problem like pluralize. Regarding the #45, I see that many changes are just formatting. Did you use npm run lint:js before commit?

from objection-graphql.

Hi @nasushkov, I didn't. Apologies, my text editor had Prettier enabled, but it didn't pick up rules on formatting tabs/spaces in the .eslintrc file.

I will make those tweaks to the PR to remove the formatting changes, and potentially make a new PR with just the required changes.

As for the way to apply singular/plural naming of the modelClass in the library, it was possible to do it with tweaking the combination of the _.camelCase and utils.typeNameForModel to achieve the desired singular/plural combinations. I will find that code in a bit.

from objection-graphql.

I'm going to close the PR for #45 and make a fresh PR that doesn't contain extra formatting.

from objection-graphql.

Submitted #46

from objection-graphql.

Hi @paulbjensen, sorry for a late response. Could you also add tests to the PR #46?

from objection-graphql.

@nasushkov yes, that's done now.

from objection-graphql.