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

better handle list types #347

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

matthew-emw
Copy link

where our GraphQL schema has a list field as follows:

accounts(
    # ...
    """sort the rows by one or more columns"""
    order_by: [accounts_order_by!]
    # ...
  )

we were observing generated code with non-list arguments like:

def accounts(
        cls,
        *,
        # ...
        order_by: Optional[accounts_order_by] = None,
        # ...
    )

this is pehaps related to issue #321 and PR #327.

my PR reworks this to result in:

def accounts(
        cls,
        *,
        # ...
        order_by: Optional[List[accounts_order_by]] = None,
        # ...
    )

i'd like to extend my PR with tests, but:

  • i see ArgumentGenerator is currently not covered by any tests
  • in custom_operations.py i see both an ArgumentsGenerator being injected and an ArgumentGenerator (no ess) being instantiated
  • does this imply a direction of travel and that ArgumentsGenerator should be used going forward?

i'd be grateful for any guidance. thanks!

Copy link

@normanre normanre left a comment

Choose a reason for hiding this comment

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

Hi,

i have the same issue and have fixed it myself.

I think your solution wont work only because of #321.
When you add the solution from #327 your solution would be perfect.

Furthermore, the solution in #327 does have an syntax error. I commented a solution in #327.

used_custom_scalar: Optional[str],
) -> None:
"""Accumulates return arguments."""
constant_value = f"{final_type.name}!" if is_required else final_type.name
constant_value = f"[{final_type.name}]" if is_list else final_type.name
constant_value = f"{constant_value}!" if is_required else constant_value

Choose a reason for hiding this comment

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

this will not work for this type of fields [String!]! (see #321)

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

Successfully merging this pull request may close these issues.

3 participants