feat: add reject/accept to Activity class#35
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Code Review
This pull request adds an ActivityMixin to the core schema and implements it in a new Python module to provide helper methods for creating Accept and Reject activities. Feedback suggests making the id parameter optional, allowing the actor to be a string, and adding future annotations for better type handling.
| from typing import TYPE_CHECKING | ||
|
|
||
| if TYPE_CHECKING: | ||
| from apmodel.activity import Accept, Reject | ||
| from apmodel.core import Activity | ||
| from apmodel.objects import Actor | ||
|
|
||
|
|
||
| class ActivityMixin: | ||
| def accept(self: "Activity", id: str, actor: "Actor") -> "Accept": | ||
| from apmodel.activity.accept import Accept | ||
|
|
||
| return Accept(id=id, actor=actor, object=self) | ||
|
|
||
| def reject(self: "Activity", id: str, actor: "Actor") -> "Reject": | ||
| from apmodel.activity.reject import Reject | ||
|
|
||
| return Reject(id=id, actor=actor, object=self) |
There was a problem hiding this comment.
The current implementation of accept and reject requires an id as a positional argument. In many ActivityPub scenarios, the ID of a new activity might not be known at creation time (e.g., it's assigned by the server later) or should be optional.
Additionally, the actor parameter is typed strictly as Actor, but in practice, it is often provided as a URI string.
I suggest:
- Making
idoptional with a default ofNone. - Swapping the order of
actorandidto allowidto be optional while keepingactorrequired. - Allowing
actorto be astr. - Adding
from __future__ import annotationsto simplify type hints and avoid circular evaluation issues at runtime.
| from typing import TYPE_CHECKING | |
| if TYPE_CHECKING: | |
| from apmodel.activity import Accept, Reject | |
| from apmodel.core import Activity | |
| from apmodel.objects import Actor | |
| class ActivityMixin: | |
| def accept(self: "Activity", id: str, actor: "Actor") -> "Accept": | |
| from apmodel.activity.accept import Accept | |
| return Accept(id=id, actor=actor, object=self) | |
| def reject(self: "Activity", id: str, actor: "Actor") -> "Reject": | |
| from apmodel.activity.reject import Reject | |
| return Reject(id=id, actor=actor, object=self) | |
| from __future__ import annotations | |
| from typing import TYPE_CHECKING | |
| if TYPE_CHECKING: | |
| from apmodel.activity import Accept, Reject | |
| from apmodel.core import Activity | |
| from apmodel.objects import Actor | |
| class ActivityMixin: | |
| def accept(self: Activity, actor: Actor | str, id: str | None = None) -> Accept: | |
| from apmodel.activity.accept import Accept | |
| return Accept(id=id, actor=actor, object=self) | |
| def reject(self: Activity, actor: Actor | str, id: str | None = None) -> Reject: | |
| from apmodel.activity.reject import Reject | |
| return Reject(id=id, actor=actor, object=self) |
No description provided.