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

GraphQL: Cannot query field "alignments" on type "Monster" #358

Closed
khaosophy opened this issue Jun 17, 2023 · 14 comments
Closed

GraphQL: Cannot query field "alignments" on type "Monster" #358

khaosophy opened this issue Jun 17, 2023 · 14 comments

Comments

@khaosophy
Copy link

I was looking for the monster alignment data, and found it in the REST API, but I'm not seeing it in the GraphQL (I tried alignments and alignment).

I can try to dig into this myself this week, but I wanted to post the issue first, in case I'm missing something obvious.

@fergcb
Copy link
Member

fergcb commented Jun 17, 2023

This definitely looks like a legitimate flaw in the GraphQL schema. A PR would be most welcome!

@ecshreve
Copy link
Contributor

ahh nice catch, poked around a bit and i think i have a handle on what's going on. this stems from Monster field definitions all being slightly different from the comparable Race and Class fields

for example, we have a defintion for armor class

type ArmorClass {
  base: Int!
  dex_bonus: Boolean!
  max_bonus: Int
}

and then we have the slightly different monster variety

  type: MonsterArmorClassType!
  desc: String
  value: Int!
  armor: [Armor]
  spell: Spell
  condition: Condition
}

in the case of alignments, we have a defnition that returns an Alignment object, but if we look at the Monster schema in the database or the rest api, we can see that the Monster.alignment field is only a primitive string.

i think there's a couple options

  • add an enum definition for MonsterAlignment, and have the gql query for monster.monster-alignment return the simple string val
    OR
  • update the monster schema to return an Alignment

i'm going to try and get a PR up for that first option this afternoon

@fergcb
Copy link
Member

fergcb commented Jul 21, 2023

The second option definitely seems more appealing to me! There seems almost no point in having an Alignments collection if we don't reference it from Monster documents.

@ecshreve
Copy link
Contributor

i lied a little bit by accident.

in the gql schema , we currently only return the Alignment object on root root queries, and from the IdealOption type.

in the current schema the alignment field on the Race type, a primitive string 🤔 going to think about this a little more

@ecshreve
Copy link
Contributor

oh yeah i made it way more complicated than it was. we should be able to get the string exposed real easy, but if we want to return an Alignment object instead it's a bit more work

localhost
Screenshot 2023-07-21 at 2 57 31 PM

@mlupton100
Copy link

It looks like the swagger openapi spec never caught up with this change which is tripping up a generated typescript client I've made 😢

If it's a simple as updating monster.yml I'm happy to raise a PR to get this merged?

        alignments:
          description: "A creature's general moral and personal attitudes."
          type: string
          enum:
            - chaotic neutral
            - chaotic evil
            - chaotic good
            - lawful neutral
            - lawful evil
            - lawful good
            - neutral
            - neutral evil
            - neutral good
            - any alignment
            - unaligned

@bagelbits
Copy link
Collaborator

@mlupton100 Please feel free to open a PR!

@ecshreve
Copy link
Contributor

ecshreve commented Nov 9, 2024

@mlupton100 where are you generating your typescript client from? Where does the spec not match reality?


The alignment stuff is a bit unique because on the Race it's a flavor description, on the Monster it's one of those enum items, and then there's also the base Alignment which has a name, abbreviation, and general description.

The Monster.alignment probably shouldn't be an enum 🤔 it isn't strictly one of the defined Alignments, it can be any of these

      "lawful evil",
      "any alignment",
      "chaotic evil",
      "chaotic good",
      "lawful good",
      "neutral",
      "lawful neutral",
      "unaligned",
      "any non-good alignment",
      "any non-lawful alignment",
      "neutral evil",
      "any chaotic alignment",
      "neutral good",
      "chaotic neutral",
      "neutral good (50%) or neutral evil (50%)",
      "any evil alignment"

@mlupton100
Copy link

@ecshreve in this case I'm only referring to the flavour text on a monster, which comes from monster-model. This doesn't match up with what we actually receive from the API.

In the schema this property is called alignments, whereas in reality we receive alignment.

So to be clear the only issue is with the name of the property, not the types themselves.

It just leaves the generated client in a place where it shouts at me for trying to access monster.alignment, even through this is a perfectly valid property.

FYI the generator I was using was typescript-axios using the following command if you wanted to try it out.

# bundle the swagger file
npm run bundle-swagger

# generate the a typescript client with axios
npx @openapitools/openapi-generator-cli generate -i ./src/swagger/dist/openapi.json -g typescript-axios -o ./5e-srd-api-client/generated

Also apologies for not raising a PR for this earlier this week. Life got in the way and I forgot all about this. If you're happy that what I've said actually makes sense I can go ahead and do that this evening.

@ecshreve
Copy link
Contributor

ecshreve commented Nov 9, 2024

@mlupton100 ah! that totally makes sense. Fixing that typo should do the trick.

While we're here, I think we should either update the enum to include all the possible values, or remove the enum entirely and just leave it as a string. Thoughts?

It's just a plain string in the API typedef.

alignment: string;

@mlupton100
Copy link

@ecshreve In my use case it doesn't really matter, but perhaps other people might want to pivot on this value. In which case capturing the possible values in an enum might be handy.

if (monster.alignment === MonsterAlignmentEnum.ChaoticNeutral) {
   // do a thing
}

That's the only thing that comes to mind.

@ecshreve
Copy link
Contributor

ecshreve commented Nov 9, 2024

I think in that case the MonsterAlignmentEnum would still be defined by the consumer somewhere, because the actual endpoint implementation does not return an enum for the Monster.alignment. The enum-ization is only happening in our documentation.

For example, this monster returns "alignment": "any non-lawful alignment"
curl -L 'https://www.dnd5eapi.co/api/monsters/bandit' -H 'Accept: application/json'


Removing the enum in our swagger docs shouldn't break any consumers because the API behavior isn't changing, we're only changing the docs (and generated openapi spec).

@bagelbits
Copy link
Collaborator

I think just aligment: string makes the most sense for now.

@ecshreve
Copy link
Contributor

ecshreve commented Dec 4, 2024

Tackling this and some other openapi schema things over in #619

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

No branches or pull requests

5 participants