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

Create strongly typed PlanCommand class #6

Merged
merged 4 commits into from
Feb 21, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ repos:
- id: pydocstyle
exclude: |
(?x)(
migrations/.*
tests.py
)$
additional_dependencies: [tomli]

Expand Down
3 changes: 3 additions & 0 deletions canvas_sdk/commands/__init__.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
from .plan import PlanCommand, PlanCommandAttributes

__all__ = ("PlanCommand", "PlanCommandAttributes")
69 changes: 69 additions & 0 deletions canvas_sdk/commands/base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
from typing import Any, TypedDict


class _BaseCommandAttributes(TypedDict):
note_id: int | None
command_uuid: str | None
user_id: int


class _BaseCommand:
note_id: int | None
command_uuid: str | None
user_id: int

def __init__(
self,
user_id: int | None = None,
note_id: int | None = None,
command_uuid: str | None = None,
) -> None:
if not user_id:
raise AttributeError("user_id is required")
self.user_id = user_id
self.note_id = note_id
self.command_uuid = command_uuid

def __setattr__(self, name: str, value: Any) -> None:
if name not in _BaseCommandAttributes.__annotations__:
return super().__setattr__(name, value)

expected_type = _BaseCommandAttributes.__annotations__[name]
if issubclass(type(value), expected_type):
super().__setattr__(name, value)
else:
raise TypeError(f"'{name}' requires a type of '{expected_type}'")
mbiannaccone marked this conversation as resolved.
Show resolved Hide resolved

@property
def values(self) -> dict:
return {}

def originate(self) -> dict:
"""Originate a new command in the note body."""
if not self.note_id:
raise AttributeError("Note id is required to originate a command")
return {"note_id": self.note_id, "user_id": self.user_id, "values": self.values}

def update(self) -> dict:
"""Update the command."""
if not self.command_uuid:
raise AttributeError("Command uuid is required to update a command")
return {"command_uuid": self.command_uuid, "user_id": self.user_id, "values": self.values}

def delete(self) -> dict:
"""Delete the command."""
if not self.command_uuid:
raise AttributeError("Command uuid is required to delete a command")
return {"command_uuid": self.command_uuid, "user_id": self.user_id, "delete": True}

def commit(self) -> dict:
"""Commit the command."""
if not self.command_uuid:
raise AttributeError("Command uuid is required to commit a command")
return {"command_uuid": self.command_uuid, "user_id": self.user_id, "commit": True}

def enter_in_error(self) -> dict:
"""Enter in error the command."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Phrasing suggestion:

Suggested change
"""Enter in error the command."""
"""Mark the command as entered-in-error."""

if not self.command_uuid:
raise AttributeError("Command uuid is required to enter in error a command")
return {"command_uuid": self.command_uuid, "user_id": self.user_id, "enter_in_error": True}
38 changes: 38 additions & 0 deletions canvas_sdk/commands/plan.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
from typing import Any

from canvas_sdk.commands.base import _BaseCommand, _BaseCommandAttributes


class PlanCommandAttributes(_BaseCommandAttributes):
"""Attributes speciic to the Plan Command."""

narrative: str | None
Copy link
Member

Choose a reason for hiding this comment

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

Let's steer right into this question: how do we keep this in sync with the platform? Suppose for example that we decided to add a new field, called timeframe, to the definition of the plan command on the platform, a duration of time within which the plan should be accomplished. How do we protect our customers from being the ones to discover the issues that might be entailed from getting out of sync?

Copy link
Member

Choose a reason for hiding this comment

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

Idea: integration test? Is there something in home-app exposed via HTTP that returns the command schema? Would it serve our purpose to require that an SDK command class field of a given type exists if and only if it is present in the schema returned in the home-app integration test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think there is an endpoint exactly like that in home-app, i'll dig it up and make an integration test. we could also have a test that runs on a cron and checks daily or something. great idea to call this out



class PlanCommand(_BaseCommand):
"""A class for managing a Plan command within a specific note."""

narrative: str | None

def __init__(
self,
narrative: str | None = None,
**kwargs: Any,
mbiannaccone marked this conversation as resolved.
Show resolved Hide resolved
) -> None:
super().__init__(**kwargs)
self.narrative = narrative

def __setattr__(self, name: str, value: Any) -> None:
if name not in PlanCommandAttributes.__annotations__:
return super().__setattr__(name, value)

expected_type = PlanCommandAttributes.__annotations__[name]
if issubclass(type(value), expected_type):
super().__setattr__(name, value)
else:
raise TypeError(f"'{name}' requires a type of '{expected_type}'")

@property
def values(self) -> dict:
"""The Plan command's field values."""
return {"narrative": self.narrative}
106 changes: 106 additions & 0 deletions canvas_sdk/commands/tests.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,106 @@
import pytest

from canvas_sdk.commands import PlanCommand


def test_plan_command_init_raises_error_without_user_id() -> None:
with pytest.raises(AttributeError) as e:
PlanCommand()
assert "user_id is required" in repr(e.value)


def test_plan_command_init_sets_all_kwargs_with_correct_type() -> None:
p = PlanCommand(user_id=1, note_id=100, command_uuid="123", narrative="hello in there")
assert p.__dict__ == {
"user_id": 1,
"note_id": 100,
"command_uuid": "123",
"narrative": "hello in there",
}


def test_plan_command_init_sets_all_kwargs_with_correct_none_type() -> None:
p = PlanCommand(user_id=1)
assert p.__dict__ == {
"user_id": 1,
"note_id": None,
"command_uuid": None,
"narrative": None,
}


def test_plan_command_raises_error_when_wrong_type_is_set_on_user_id() -> None:
with pytest.raises(TypeError) as e:
PlanCommand(user_id="1", note_id=100, command_uuid="123", narrative="hello in there")
assert "'user_id' requires a type of '<class 'int'>'" in repr(e.value)


def test_plan_command_raises_error_when_wrong_type_is_set_on_note_id() -> None:
with pytest.raises(TypeError) as e:
PlanCommand(user_id=1, note_id="100", command_uuid="123", narrative="hello in there")
assert "'note_id' requires a type of 'int | None'" in repr(e.value)


def test_plan_command_raises_error_when_wrong_type_is_set_on_command_uuid() -> None:
with pytest.raises(TypeError) as e:
PlanCommand(user_id=1, note_id=100, command_uuid=123, narrative="hello in there")
assert "'command_uuid' requires a type of 'str | None'" in repr(e.value)


def test_plan_command_raises_error_when_wrong_type_is_set_on_narrative() -> None:
with pytest.raises(TypeError) as e:
PlanCommand(user_id=1, note_id=100, command_uuid="123", narrative=143) # type: ignore
assert "'narrative' requires a type of 'str | None'" in repr(e.value)


def test_plan_command_only_allows_defined_type_to_be_set_on_user_id() -> None:
p = PlanCommand(user_id=1)
with pytest.raises(TypeError) as e1:
p.user_id = "1" # type: ignore
assert "'user_id' requires a type of '<class 'int'>'" in repr(e1.value)

with pytest.raises(TypeError) as e2:
p.user_id = None # type: ignore
assert "'user_id' requires a type of '<class 'int'>'" in repr(e2.value)

p.user_id = 100
assert p.user_id == 100


def test_plan_command_only_allows_defined_type_to_be_set_on_note_id() -> None:
p = PlanCommand(user_id=1)
with pytest.raises(TypeError) as e:
p.note_id = "1" # type: ignore
assert "'note_id' requires a type of 'int | None'" in repr(e.value)

p.note_id = 100
assert p.note_id == 100

p.note_id = None
assert p.note_id is None


def test_plan_command_only_allows_defined_type_to_be_set_on_command_uuid() -> None:
p = PlanCommand(user_id=1)
with pytest.raises(TypeError) as e:
p.command_uuid = 1 # type: ignore
assert "'command_uuid' requires a type of 'str | None'" in repr(e.value)

p.command_uuid = "100"
assert p.command_uuid == "100"

p.command_uuid = None
assert p.command_uuid is None


def test_plan_command_only_allows_defined_type_to_be_set_on_narrative() -> None:
p = PlanCommand(user_id=1)
with pytest.raises(TypeError) as e:
p.narrative = 1 # type: ignore
assert "'narrative' requires a type of 'str | None'" in repr(e.value)

p.narrative = "yeah"
assert p.narrative == "yeah"

p.narrative = None
assert p.narrative is None
Loading