Skip to content

Add a meta option to allow field names renaming #178

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

Closed

Conversation

Nabellaleen
Copy link
Collaborator

Example is given in the README.md and in tests

It allows to implement :

class User(SQLAlchemyObjectType):
    class Meta:
        model = UserModel
        aliased_fields = {'name': 'first_name'}

To be able to have a field named first_name in your graphql schema instead of being stucked with UserModel field name name

@coveralls
Copy link

coveralls commented Nov 26, 2018

Coverage Status

Coverage decreased (-0.4%) to 91.46% when pulling a70f254 on Nabellaleen:feature-field-name-alias into 6a96d37 on graphql-python:master.

@Nabellaleen
Copy link
Collaborator Author

Existing tests were not covering lines I've edited and I admit I don't know how to test its (I don't know what are composite fields and hybrid fields)

If someone want to help me on that, it's with pleasure ;)

@Cito
Copy link
Member

Cito commented Mar 20, 2019

Nice. This might be useful when you can't change the SQLAlchemy model or are using reflection.

But maybe the name "aliased_fields" is a bit misleading, because an alias is usually an alternative, additional name that exists parallel to the original name, but what you're doing is renaming the fields. Also, the other attributes use the imperative form "skip_", "use_", "exclude_", instead of "skipped_", "used_", "exclude_". So to be more clear and consistent I suggest the name "rename_fields" instead.

Btw, to answer your question above, composite fields and hybrid fields are rarely used concepts of SQLAlchemy. Composite fields allow you to pack multiple columns into one field, and hybrid attributes can act on both the instance and class level.

Copy link
Member

@Cito Cito left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the inline comments and the comment above regarding the name. Otherwise looks fine to me.

@Nabellaleen Nabellaleen force-pushed the feature-field-name-alias branch from a70f254 to ec8afb5 Compare March 28, 2019 19:32
@Nabellaleen Nabellaleen self-assigned this Mar 28, 2019
@Nabellaleen Nabellaleen requested a review from Cito March 28, 2019 19:36
@Nabellaleen Nabellaleen added this to the 2.1.1 milestone Mar 28, 2019
@jnak
Copy link
Collaborator

jnak commented Jan 24, 2020

Closing this since it is now possible to rename fields via ORMField.model_attr. This will be more thoroughly documented soon. For now, you can also refer to the some of the tests.

@jnak jnak closed this Jan 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants