Standardise APIResource structures

[fergcb]

Throughout the database, objects contain references to other objects in the database. Most commonly, references have the following structure:

{
    "name": "Name of Referenced Resource",
    "url": "/api/.../..."
}

However, elsewhere, we several variations on this structure, for example, in some attributes of Class objects, we have cases like this:

"starting_equipment": {
    "url": "/api/starting-equipment/1",
    "class": "Barbarian"
},

In this case, the class attribute is pointless, as starting_equipment is a property of the Class object, which already contains it's own name. Only the url attribute is useful here.

When querying collections using the API, we get an array of references of the following format:

{
    "index": "resource-index",
    "name": "Resource Name",
    "url": "/api/resource/url"
}

I believe this is the ideal structure for references, and should be implemented across the database, for these reasons:

• The url is provided in order to fetch the resource from the API - this is the most important part of the reference - it needs to stay
• index is useful for managing the data client-side. For example, it can be used to fetch the resource from a local cache, as a key for the resource in a collection, etc.
  • This is useful as the index does not always correspond with the final element of the url, so cannot be reliably parsed from the url attribute. e.g. /api/classes/monk/levels - the index of the referenced resource is nowhere to be seen in the URL.
• The name is provided as a human-readable identifier of the resource. This comes in handy if displaying a list of objects to the user in a meaningful way, without having to fetch each object from the API.
  • name should be optional, as there are scenarios where it is less useful or non-applicable, for example Starting Equipment and Level strctures lack a name attribute.

[bagelbits]

I think that's a reasonable proposal.

[bagelbits]

It'll probably take a while to implement though. However, it doesn't necessarily need to be done all at once and can be one file at a time per PR.

[fergcb]

I've actually started on this one locally. Have written a quick script to automate the rewrite, but there are a few edge cases that still need seeing to (e.g. ones with a "class" attribute instead of "name")

[bagelbits]

nods Awesome! Nice to hear you're making headway! Let me know if there's anyway I can help out!

[fergcb]

keep your eyes peeled for the PR!

[fergcb]

In some places, we have references where it doesn't make sense to use the full NamedAPIResource structure, e.g. because the linked URL returns a list, or an unnamed object, like starting equipment. Would it make sense to have these simply be strings containing the url?

Currently, in Class resources, we have examples of both situations:

    "starting_equipment": {
      "url": "/api/starting-equipment/cleric",
      "class": "Cleric"
    },
    "class_levels": {
      "url": "/api/classes/cleric/levels",
      "class": "Cleric"
    },

This is weird, because these are attributes of the class object, so the name of the class is already known to any client viewing this object, and as part of the same request. Additionally, the class attribute doesn't correspond to a top-level attribute of the referenced resource, but in fact a nested attribute, name, of the class object in the referenced object(s).

I propose replacing this with the following, to remove the redundant and potentially misleading class attribute, and to simplify the structure:

    "starting_equipment": "/api/starting-equipment/cleric",
    "class_levels": "/api/classes/cleric/levels",

Thoughts @bagelbits?

[bagelbits]

Starting equipment and levels both have indices though? I still think it should include the index. Then the shape is the same as everywhere else instead of being a string in this one special case, but a map in every other case.

[bagelbits]

Consistency is still important. I can see the name not necessarily being useful though? I can go either way on that one.

[fergcb]

Individual levels have indices, but the URL provided by class_levels doesn't return an object that has an index, it returns an array.

I can see the point with starting equipment, regarding the shape, but I would add that the indices of starting equipment objects are identical to those of the classes they correspond to, making it equally redundant as the name originally provided. Additionally, they don't have names, like every other APIResource structure.

Edit RE consistency: the key issue with these structures is that they don't have names. They're not the same kind of structures as every other NamedAPIResource, so I don't feel they necessarily warrant the same kind of reference.

[bagelbits]

@ogregoire, thoughts?

[ogregoire]

My quick personal take here (because I have a lot on my plate in private life at the moment) is that I don't see why levels and starting_equipment are separate from the class. They're strongly linked to the classes and they should probably be better in the class.json file. So far I was always thinking that if we can make it work like this, why not, if we can't, we should merge. In my view, it was always a nonsense that they're in their own file.

So if someone finds there's too much inconsistency, I'm of the advice that we merge class_levels.json and starting_equipment.json into class.json (and maybe include the spells_list.json for good measure).

[fergcb]

Over the past few days, I've been thinking this might be the better solution, too. It would simplify both the API and the digestion of the data by the client.

There's absolutely no reason not to include starting equipment and class levels in the actual class. It's not as though different classes will ever share the same instances of these structures.

[fergcb]

Leaving the references as URL strings will suffice for now. I've opened a new issue (#251) to discuss how we should move forward with the Class object reference structure.

[bagelbits]

I'm going to keep this open for now. @fergcb, can you update the docs to match these changes?

[fergcb]

Yep, will do