-
Notifications
You must be signed in to change notification settings - Fork 12
refactor: [BREAKING] update Task and Project schemas to match API objects #286
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
Conversation
color: z.string(), | ||
createdAt: z.string(), | ||
createdAt: z.string().nullable(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really? createdAt
can be null?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it can be null
in the current iteration. As weird as it may sound, the created_at
attribute is new to the system. We can see the tombstones returning null
here.
I know we are discussing introducing created_at
everywhere, and we should have a date as default for those we can't really determine, but not sure what is the state of that.
| { | ||
canAssignTasks: boolean; | ||
childOrder: number; | ||
color: string; | ||
createdAt: null | string; | ||
defaultOrder: number; | ||
description: string; | ||
id: string; | ||
inboxProject: boolean; | ||
isArchived: boolean; | ||
isCollapsed: boolean; | ||
isDeleted: boolean; | ||
isFavorite: boolean; | ||
isFrozen: boolean; | ||
isShared: boolean; | ||
name: string; | ||
parentId: null | string; | ||
updatedAt: null | string; | ||
url: string; | ||
viewStyle: string; | ||
} | ||
| { | ||
canAssignTasks: boolean; | ||
childOrder: number; | ||
collaboratorRoleDefault: string; | ||
color: string; | ||
createdAt: null | string; | ||
defaultOrder: number; | ||
description: string; | ||
folderId: null | boolean; | ||
id: string; | ||
isArchived: boolean; | ||
isCollapsed: boolean; | ||
isDeleted: boolean; | ||
isFavorite: boolean; | ||
isFrozen: boolean; | ||
isInviteOnly: null | boolean; | ||
isLinkSharingEnabled: boolean; | ||
isShared: boolean; | ||
name: string; | ||
role: null | string; | ||
status: string; | ||
updatedAt: null | string; | ||
url: string; | ||
viewStyle: string; | ||
workspaceId: string; | ||
}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this return type not look so unwieldly? Shouldn't this just be PersonalProject | WorkspaceProject
? This applies to all the added docs in this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is something that I had to battle with when I first set up docusaurus/typedoc. Types are automatically expanded, while interfaces aren't. Because Project
can only be a type because it is a union of two interfaces, it appears expanded.
I've now revisited typedoc/docusaurus documentation looking for some config where we can disable auto expanding but once again I haven't found one.
That said, as a solution we can use directly the PersonalProject
and WorkspaceProject
interfaces. In fact, I think it's better than having a Project
union as it makes the project distinction very explicit.
Done in a922ba6 .
@scottlovegrove , do I have your 🟢 to merge this? |
🗺 Overview
While #283 served as a patch to a bug in
4.0.0
, introducing tasks and project converter helpers to map Todoist API v1 objects to the expected local schemas, the long term goal is to have the SDK entities fully match the objects returned by the API. For this reason, in this PR we're introducing a breaking change that brings the Task and Project schemas closer to their API objects. "Closer" and not equal as we're keeping theurl
field, as done in the Python SDK.🧪 Test plan
Changes introduced here have been manually tested by someone other than the PR author.
The task and project endpoints return objects in the updated schemas.
No validation errors are thrown.