do not add extra s after table name when generating array relationship name

[elitan]

…p name

Description

do not add extra s after table name when generating array relationship name

What component does this PR affect?

• Server
• Console
• CLI
• Docs
• Community Content
• Build System

Requires changes from other components? If yes, please mark the components:

• Server
• Console
• CLI
• Docs
• Community Content
• Build System

Related Issue

None

Solution and Design

Some (most) use plural when naming their tables. This make the extra added, hard coded, s after a table name confusing. Even if the tables are named in singular, just adding a s in the end is not the correct plural naming. Like company which in plural is companies. Just adding a s in the end would result in an incorrect naming: companys.

Type

• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Docs update
• Community content

Checklist:

• I have read the contributing guide and my code conforms to the guidelines.
• This change requires a change in the documentation.
• I have updated the documentation accordingly.
• I have added required tests.

[hasura-bot]

Beep boop! 🤖

Hey @elitan, thanks for your PR!

One of my human friends will review this PR and get back to you as soon as possible. 🕐

Stay awesome! 😎

[CLAassistant]

All committers have signed the CLA.

[elitan]

hmm, do I have to take any further action on this?

[shahidhk]

Hi @elitan, thanks for the contribution 🙏

The change you have added is for community/tools/json2graphql, which is different from the console.

@praveenweb @karthikvt26, can you point @elitan to the right place where he needs to make this change?

My suggestion is that we add a s for array realationships if the table name doesn't end with a s already (e.g. order -> ordersByUserId). If the table name already contains s, we don't add another s (e.g. orders -> ordersByUserId).

[elitan]

Ok, yes please nock me in the right direction. :)

But first maybe we should discuss if we should or shouldn't add the extra s. As I described above, English is more complex than just add a s in the end to transform any world to its plural form.

With your idea with adding a s if the word does not end with s does not work if you would name your tables in singular form and use, let's say, company.

This would result in companysBy... Not very pretty nor correct.

[shahidhk]

Makes sense @elitan. What about using something like pluralize? It should cover most of the use cases, right?

For array relationships, we take the remote table, pluralize it, and then append the ByColumnName. 🤔

[elitan]

That could work, but I do see some problems with it. As you say it would probably work "in most cases". Aslo you would have problem if you name your tables in another language than English.

I understand you want to make it as easy as possible for users when naming relations between tables. But as a developer you also want to be, or feel like you are, in full control. For me, when Hasura tries to be smart like this, and fail, it is frustrating.

And if Hasura would be somewhat successful in 'guessing' the correct name on relations I would expect Hasura to also enforce singular naming on table names for consistency. Further I would expect all query names to have the plural format as well since, if I am not misstaken, all queries return an array of objects.

Ex:

Table name: company
RelationShip: companiesBy...
Query: companies () {...

I suggest that Hasura would suggest to use plural format when naming tables, and remove any automatic/smart conversion when generating relationship names.

[shahidhk]

@elitan Sorry, lost track of this issue. @praveenweb What do you think?

[praveenweb]

@shahidhk @elitan - Related issue #1444 .

The idea is to generate singular names for object relationships and plural names for array relationships using the inflection module.

We can close this PR.

[shahidhk]

Thanks for the PR @elitan, as @praveenweb mentioned, we'll use the node module inflection to do this. Feel free to take up #1444 if you are interested 😄

[hasura-bot]

Review app https://hge-ci-pull-1072.herokuapp.com is deleted