feat: required fields

[ffMathy]

Supersedes #34.

I am a bit unsure if arrays are always set. I mean, it certainly makes sense to me, but I am not sure how the Ftrack API handles it. In my opinion, arrays should never be null or undefined, but instead empty. Let me know if this behavior does not reflect what's actually happening.

• [X] I have added automatic tests where applicable
• [X] The PR title is suitable as a release note
• [X] The PR contains a description of what has been changed
• [X] The description contains manual test instructions

Changes

Test

[ffMathy]

Callin' @jimmycallin ❤️

[jimmycallin]

I'm of two minds around this and will need to sleep on this a bit.

Queries

Take this query for instance:

session.query<Task>("select id from Task limit 1");

Here I will get back an object saying that name will be available, which isn't true. What the required field really means is that if you ask for a value back, it won't be null. For this case, I think we should only list the primary keys as non-optional. To mark more fields as non-optional based on the expression, I can see us providing a generic that helps with this, something like:

type WithRequired<T, K extends keyof T> = T & { [P in K]-?: T[P] }

session.query<WithRequired<Task, "name">>("select name from Task limit 1");

Mutations

Thinking about it a bit more, I'm uncertain even if primary_key should be part of non-optional fields. It makes sense from a query standpoint, but take this update action for instance:

session.create<Task>("Task", [id], { name: "Test" } );

Here, I expect data to not need an entity key, as it will be generated if omitted. I don't even need all required fields, as some of them will have a default value on creation. Unfortunately we don't have a way in our schema today to see which fields are necessary to provide on creation or not.

Even further, when we have update events:

session.update<Task>("Task", [id], { status_id: "1234" });

I don't even need fields that are necessary for creation, as it is more like a PATCH event.

Looking at graphql, they split out Input types from query types, so perhaps the solution is to do something similar here and introduce TaskInput etc for mutations. In that case we can ignore the mutation questions above for now.

Just loose thoughts :) Let me know if you have any ideas!

[ffMathy]

Here I will get back an object saying that name will be available, which isn't true. What the required field really means is that if you ask for a value back, it won't be null. For this case, I think we should only list the primary keys as non-optional.

Indeed, but as I mentioned in my other PR, I think it should be the Ftrack API's responsibility to wrap the type in a Partial instead.

The type should represent the truth of the datamodel. The fact that some fields may not be required is specific to the call that the Ftrack client makes, and hence, the partial should be in its return type instead. For instance:

class ApiClient {
  function create<T>(foo: T): Partial<T> { ... } //now even if some fields in T are required, they are optional in this case
}

Here, I expect data to not need an entity key, as it will be generated if omitted. I don't even need all required fields, as some of them will have a default value on creation. Unfortunately we don't have a way in our schema today to see which fields are necessary to provide on creation or not.

Again, same applies here. It should be the responsibility of the create method's type to wrap that in a partial.

I don't even need fields that are necessary for creation, as it is more like a PATCH event.

Same answer here.

Those are my thoughts. In my opinion, we're breaking the encapsulation principle by defining types that are actually not what the data represents, but instead represent how they look when they are used.

A class should not be able to know how it is used. That violates that principle.

[jimmycallin]

that makes a lot of sense! thanks for explaining your thoughts. i think we can merge this and try it out :)

[ffMathy]

Great stuff! ❤️ When will it typically be released? :pray:

[jimmycallin]

Great stuff! ❤️ When will it typically be released? 🙏

i have already created a 2.0.0-beta.1, try it out!

[ffMathy]

Ah okay - I think we'll wait until it's out of beta :heart:

[ffMathy]

Nevermind, had some time to spare. We're now on the beta and all our tests pass, and the types look great.