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

bug(cli): cli does not respect custom relation getter name #4209

Closed
derdeka opened this issue Nov 26, 2019 · 8 comments · Fixed by #4373
Closed

bug(cli): cli does not respect custom relation getter name #4209

derdeka opened this issue Nov 26, 2019 · 8 comments · Fixed by #4373

Comments

@derdeka
Copy link
Contributor

derdeka commented Nov 26, 2019

Steps to reproduce

Generating hasMany relation between News and NewsFile object. Naming it files instead of the default newsFiles:

image

@loopback/[email protected]

Current Behavior

The custom name files is used in the model, but not in the repository:

image

image

Expected Behavior

Expecting consistent naming across model and repository.

@derdeka derdeka added the bug label Nov 26, 2019
@achrinza
Copy link
Member

Correct me if I'm wrong but I believe this is expected behaviour. The CLI prompts for setting the property name (not the relation name), which in this case works fine.

@derdeka
Copy link
Contributor Author

derdeka commented Nov 26, 2019

@achrinza if this is the expected behavior, maybe there should be another prompt for customizing the naming in the repository?

@achrinza
Copy link
Member

achrinza commented Nov 26, 2019 via email

@derdeka
Copy link
Contributor Author

derdeka commented Nov 26, 2019

Just curious, what’s the use case for custom relation names?

I have a model inheritance like NewsFile extends BaseFile and ContractFile extends BaseFile to have shared properties and some additional custom properties per subclass - each subclass is storing in it's own relational database table. On the api it's just named files.

@achrinza
Copy link
Member

achrinza commented Nov 26, 2019

@derdeka Thanks for providing extra info. I think this feature is a possibility (albeit the decision is up to the core maintainers), though there are limitations on using class inheritance that might be important to take note of.

@achrinza
Copy link
Member

achrinza commented Nov 26, 2019

I've dug into the code and docs and found some inconsistencies:

  1. --relationName option is used for setting the property (see here)
  2. Internally, the function responsible referred to as a promptRelationName (see here)
  3. Internally, the constant storing the prompt message is called PROMPT_MESSAGE_PROPERTY_NAME (see here)
  4. Internally, the default FK property generator is called _getDefaultRelationName (see here)
  5. The end result is the FK property name being affected and not the relations name.

There seems to be ambiguity on what this prompt is supposed to do.

It doesn't help that the hasMany and hasOne docs also mention that their respective property relation decorators support the name field name which is defined as the name of the relation (which is different from what the CLI references).

@achrinza
Copy link
Member

@dhmlau Maybe the core maintainers might want to look into this; if this is expected behaviour

@bajtos
Copy link
Member

bajtos commented Nov 29, 2019

Expecting consistent naming across model and repository.

+1

I think this is a bug, an oversight on the side of the person implementing lb4 relation command.

@derdeka @achrinza would you like to contribute the fix yourself?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants