Skip to content

Commit

Permalink
Annotate event.data type as Mapping instead of dict
Browse files Browse the repository at this point in the history
We had a recent bug where LTIEvent.from_request modified the passed in
data parameter.

Annotating the type as Mapping raises an type error trying to reassign
keys with the following error:

Unsupported target for indexed assignment ("Mapping[Any, Any] | None")  [index]

While this doesn't prevent the issue in the whole codebase it's a good
practice to annotate types with the most restrictive one.
  • Loading branch information
marcospri committed Dec 13, 2024
1 parent 0d1248c commit bd47735
Showing 1 changed file with 3 additions and 2 deletions.
5 changes: 3 additions & 2 deletions lms/events/event.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
from dataclasses import asdict, dataclass, field, fields
from typing import Mapping

from pyramid.request import Request
from sqlalchemy import inspect
Expand Down Expand Up @@ -28,7 +29,7 @@ class BaseEvent:
assignment_id: int | None = None
grouping_id: int | None = None

data: dict | None = None
data: Mapping | None = None
"""Extra data to associate with this event"""

Type = EventType.Type
Expand All @@ -49,7 +50,7 @@ class LTIEvent(BaseEvent):

@classmethod
def from_request(
cls, request: Request, type_: EventType.Type, data: dict | None = None
cls, request: Request, type_: EventType.Type, data: Mapping | None = None
):
if not request.lti_user:
return cls(request=request, type=type_, data=data)
Expand Down

0 comments on commit bd47735

Please sign in to comment.