From 36c04d2d7e11e03d933aef437b1011ee75bb6778 Mon Sep 17 00:00:00 2001 From: Marcos Prieto Date: Fri, 12 Jul 2024 13:43:28 +0200 Subject: [PATCH] Enable mypy's check_untyped_defs Make necessary changes to allow a clean mypy run after the change --- lms/models/assignment.py | 4 ++-- lms/models/oauth2_token.py | 10 ++++------ lms/product/plugin/__init__.py | 1 + lms/product/plugin/course_copy.py | 4 ++-- lms/product/plugin/plugin.py | 1 + lms/product/product.py | 2 +- lms/services/application_instance.py | 4 ++-- lms/services/digest.py | 5 +++-- lms/services/group_info.py | 2 +- lms/services/moodle.py | 4 ++-- lms/validation/__init__.py | 2 +- lms/views/admin/assignment.py | 2 +- lms/views/admin/organization.py | 3 ++- lms/views/api/blackboard/files.py | 6 +++--- lms/views/api/canvas/authorize.py | 2 +- lms/views/api/canvas/files.py | 1 + lms/views/api/canvas/pages.py | 1 + lms/views/api/d2l/authorize.py | 2 +- lms/views/api/d2l/files.py | 6 +++--- lms/views/api/moodle/files.py | 6 ++++-- lms/views/api/moodle/pages.py | 1 + lms/views/lti/basic_launch.py | 2 +- pyproject.toml | 2 +- 23 files changed, 40 insertions(+), 33 deletions(-) diff --git a/lms/models/assignment.py b/lms/models/assignment.py index 2992bfdc63..6e23bc5c44 100644 --- a/lms/models/assignment.py +++ b/lms/models/assignment.py @@ -1,7 +1,7 @@ import sqlalchemy as sa from sqlalchemy.dialects.postgresql import JSONB from sqlalchemy.ext.mutable import MutableDict -from sqlalchemy.orm import Mapped, mapped_column +from sqlalchemy.orm import DynamicMapped, Mapped, mapped_column from lms.db import Base from lms.models._mixins import CreatedUpdatedMixin @@ -78,7 +78,7 @@ class Assignment(CreatedUpdatedMixin, Base): deep_linking_uuid: Mapped[str | None] = mapped_column(sa.Unicode, nullable=True) """UUID that identifies the deep linking that created this assignment.""" - groupings: Mapped[list[Grouping]] = sa.orm.relationship( + groupings: DynamicMapped[Grouping] = sa.orm.relationship( secondary="assignment_grouping", viewonly=True, lazy="dynamic" ) diff --git a/lms/models/oauth2_token.py b/lms/models/oauth2_token.py index 42148c6739..1d92d8f120 100644 --- a/lms/models/oauth2_token.py +++ b/lms/models/oauth2_token.py @@ -4,6 +4,7 @@ from enum import Enum, unique import sqlalchemy as sa +from sqlalchemy.orm import Mapped, mapped_column from lms.db import Base, varchar_enum @@ -43,7 +44,7 @@ class OAuth2Token(Base): sa.UniqueConstraint("user_id", "application_instance_id", "service"), ) - id = sa.Column(sa.Integer(), autoincrement=True, primary_key=True) + id: Mapped[int] = mapped_column(autoincrement=True, primary_key=True) #: The LTI user_id of the LMS user who this access token belongs to. user_id = sa.Column(sa.UnicodeText(), nullable=False) @@ -91,11 +92,8 @@ class OAuth2Token(Base): #: inserting new rows) then whenever `access_token` and `expires_in` #: are updated with new values from the authorization server, `received_at` #: should also be updated to the current time. - received_at = sa.Column( - sa.DateTime, - default=datetime.datetime.utcnow, - server_default=sa.func.now(), - nullable=False, + received_at: Mapped[datetime.datetime] = mapped_column( + default=datetime.datetime.utcnow, server_default=sa.func.now() ) #: The API that this token is used with. In OAuth 2.0 parlance, this diff --git a/lms/product/plugin/__init__.py b/lms/product/plugin/__init__.py index 2aa0e73bc6..fe74edd0d5 100644 --- a/lms/product/plugin/__init__.py +++ b/lms/product/plugin/__init__.py @@ -1,3 +1,4 @@ +# type: ignore from lms.product.plugin.course_copy import CourseCopyPlugin from lms.product.plugin.grouping import GroupingPlugin from lms.product.plugin.misc import MiscPlugin diff --git a/lms/product/plugin/course_copy.py b/lms/product/plugin/course_copy.py index 32b14d4e10..2f293df1b1 100644 --- a/lms/product/plugin/course_copy.py +++ b/lms/product/plugin/course_copy.py @@ -1,4 +1,4 @@ -from typing import Callable +from typing import Any, Callable from lms.models import File from lms.services.exceptions import ExternalRequestError, OAuth2TokenError @@ -20,7 +20,7 @@ def is_file_in_course(self, course_id, file_id, type_) -> bool: def find_matching_file_in_course( self, - store_new_course_files: Callable[[str], None], + store_new_course_files: Callable[[str], Any] | Callable[[int], Any], file_type: str, original_file_id, new_course_id, diff --git a/lms/product/plugin/plugin.py b/lms/product/plugin/plugin.py index 747bf69106..e4204b0de6 100644 --- a/lms/product/plugin/plugin.py +++ b/lms/product/plugin/plugin.py @@ -1,3 +1,4 @@ +# type: ignore from dataclasses import dataclass from typing import cast diff --git a/lms/product/product.py b/lms/product/product.py index 28e968edac..213ab8ec0e 100644 --- a/lms/product/product.py +++ b/lms/product/product.py @@ -3,7 +3,7 @@ from dataclasses import InitVar, dataclass from lms.product.family import Family # pylint:disable=unused-import -from lms.product.plugin import PluginConfig, Plugins +from lms.product.plugin import PluginConfig, Plugins # type: ignore @dataclass(frozen=True) diff --git a/lms/services/application_instance.py b/lms/services/application_instance.py index 31174df43e..28925e9816 100644 --- a/lms/services/application_instance.py +++ b/lms/services/application_instance.py @@ -52,14 +52,14 @@ def __init__(self, application_instance: ApplicationInstance): ) -def _email_or_domain_match(columns, email): +def _email_or_domain_match(columns, email: str): """ Get an SQL comparator for matching emails. This will match the full email if it contains '@' or interpret the text as a domain if not. This will search over all the provided fields. """ - return sa.or_( + return sa.or_( # type: ignore ( sa.func.lower(column) == email.lower() if "@" in email diff --git a/lms/services/digest.py b/lms/services/digest.py index 94525f0607..60761822f9 100644 --- a/lms/services/digest.py +++ b/lms/services/digest.py @@ -400,7 +400,7 @@ def course_infos(self): for row in self._db.execute(query): # SQLAlchemy returns None instead of []. - authority_provided_ids = row.authority_provided_ids or [] + row_authority_provided_ids = row.authority_provided_ids or [] instructor_h_userids = row.instructor_h_userids or [] self._course_infos.append( @@ -411,7 +411,8 @@ def course_infos(self): learner_annotations=tuple( annotation for annotation in self.annotations - if annotation.authority_provided_id in authority_provided_ids + if annotation.authority_provided_id + in row_authority_provided_ids and annotation.userid not in instructor_h_userids ), ) diff --git a/lms/services/group_info.py b/lms/services/group_info.py index e13c162526..15ed8bca06 100644 --- a/lms/services/group_info.py +++ b/lms/services/group_info.py @@ -46,7 +46,7 @@ def upsert_group_info(self, grouping: Grouping, params: dict): # AI should be many:many, and we reflect that wrongness here. group_info.application_instance = grouping.application_instance - group_info.type = self._GROUPING_TYPES[grouping.type] # type: ignore + group_info.type = self._GROUPING_TYPES[grouping.type] for field in [ # Most (all) of these are duplicated elsewhere, we'll keep updating for now diff --git a/lms/services/moodle.py b/lms/services/moodle.py index 20b9eb5ab2..6cb77f4fe4 100644 --- a/lms/services/moodle.py +++ b/lms/services/moodle.py @@ -255,7 +255,7 @@ def _construct_file_tree(course_id, files): "children": [], } folders[component] = new_folder - current_node["children"].append(new_folder) + current_node["children"].append(new_folder) # type: ignore current_node = folders[component] file_node = { @@ -266,7 +266,7 @@ def _construct_file_tree(course_id, files): "lms_id": file_data["url"], "updated_at": file_data["updated_at"], } - current_node["children"].append(file_node) + current_node["children"].append(file_node) # type: ignore return root["children"] diff --git a/lms/validation/__init__.py b/lms/validation/__init__.py index 48fe3d2cfe..1ee9190f5f 100644 --- a/lms/validation/__init__.py +++ b/lms/validation/__init__.py @@ -97,5 +97,5 @@ def wrapper_view(context, request): def includeme(config): - _validated_view.options = ["schema"] + _validated_view.options = ["schema"] # type: ignore config.add_view_deriver(_validated_view) diff --git a/lms/views/admin/assignment.py b/lms/views/admin/assignment.py index 41fa0a0b6e..613c311fb3 100644 --- a/lms/views/admin/assignment.py +++ b/lms/views/admin/assignment.py @@ -55,7 +55,7 @@ def assignment_dashboard(self): "dashboard.assignment", public_id=assignment.groupings[ 0 - ].application_instance.organization.public_id, + ].application_instance.organization.public_id, # type: ignore assignment_id=assignment.id, ), ) diff --git a/lms/views/admin/organization.py b/lms/views/admin/organization.py index aeb66e9e22..45c3b5c228 100644 --- a/lms/views/admin/organization.py +++ b/lms/views/admin/organization.py @@ -50,7 +50,7 @@ def __init__(self, request) -> None: self.organization_service: OrganizationService = request.find_service( OrganizationService ) - self.organization_usage_report_service: OrganizationService = ( + self.organization_usage_report_service: OrganizationUsageReportService = ( request.find_service(OrganizationUsageReportService) ) @@ -155,6 +155,7 @@ def toggle_organization_enabled(self): request_org_id = self.request.matchdict["id_"] for org_id in self.organization_service.get_hierarchy_ids(request_org_id): org = self.organization_service.get_by_id(org_id) + assert org, "Organization {org_id} not found" self.organization_service.update_organization( org, enabled=self.request.params.get("enabled", "") == "on" ) diff --git a/lms/views/api/blackboard/files.py b/lms/views/api/blackboard/files.py index e91b4698e8..a683ec7a1f 100644 --- a/lms/views/api/blackboard/files.py +++ b/lms/views/api/blackboard/files.py @@ -75,9 +75,9 @@ def via_url(self): ) document_url = self.request.params["document_url"] - file_id = course.get_mapped_file_id( - DOCUMENT_URL_REGEX.search(document_url)["file_id"] # type: ignore - ) + document_url_match = DOCUMENT_URL_REGEX.search(document_url) + assert document_url_match + file_id = course.get_mapped_file_id(document_url_match["file_id"]) try: if self.request.lti_user.is_instructor: if not self.course_copy_plugin.is_file_in_course(course_id, file_id): diff --git a/lms/views/api/canvas/authorize.py b/lms/views/api/canvas/authorize.py index a4e1ff0324..989b58dcc0 100644 --- a/lms/views/api/canvas/authorize.py +++ b/lms/views/api/canvas/authorize.py @@ -152,7 +152,7 @@ def oauth2_redirect_error(request): ) if error_description := request.params.get("error_description"): - kwargs["error_details"] = {"error_description": error_description} + kwargs["error_details"] = {"error_description": error_description} # type: ignore request.context.js_config.enable_oauth2_redirect_error_mode(**kwargs) diff --git a/lms/views/api/canvas/files.py b/lms/views/api/canvas/files.py index 60bd42e26e..5aaaecfec2 100644 --- a/lms/views/api/canvas/files.py +++ b/lms/views/api/canvas/files.py @@ -46,6 +46,7 @@ def via_url(self): ) document_url_match = DOCUMENT_URL_REGEX.search(assignment.document_url) + assert document_url_match public_url = self.canvas.public_url_for_file( assignment, document_url_match["file_id"], diff --git a/lms/views/api/canvas/pages.py b/lms/views/api/canvas/pages.py index ac11592a46..59e7017076 100644 --- a/lms/views/api/canvas/pages.py +++ b/lms/views/api/canvas/pages.py @@ -168,6 +168,7 @@ def proxy(self): @staticmethod def _parse_document_url(document_url): document_url_match = DOCUMENT_URL_REGEX.search(document_url) + assert document_url_match course_id = document_url_match["course_id"] page_id = document_url_match["page_id"] diff --git a/lms/views/api/d2l/authorize.py b/lms/views/api/d2l/authorize.py index 86d1d9d2da..04ffd27819 100644 --- a/lms/views/api/d2l/authorize.py +++ b/lms/views/api/d2l/authorize.py @@ -17,7 +17,7 @@ permission=Permissions.API, ) def authorize(request): - scopes = [] + scopes: list[str] = [] if request.product.settings.files_enabled: scopes += FILES_SCOPES diff --git a/lms/views/api/d2l/files.py b/lms/views/api/d2l/files.py index c74e1cffbf..8138f72cf5 100644 --- a/lms/views/api/d2l/files.py +++ b/lms/views/api/d2l/files.py @@ -40,9 +40,9 @@ def via_url(_context, request): course_id, raise_on_missing=True ) - file_id = course.get_mapped_file_id( - DOCUMENT_URL_REGEX.search(document_url)["file_id"] - ) + document_url_match = DOCUMENT_URL_REGEX.search(document_url) + assert document_url_match + file_id = course.get_mapped_file_id(document_url_match["file_id"]) try: if request.lti_user.is_instructor: if not course_copy_plugin.is_file_in_course(course_id, file_id): diff --git a/lms/views/api/moodle/files.py b/lms/views/api/moodle/files.py index 5dc89f0450..c07fcddaad 100644 --- a/lms/views/api/moodle/files.py +++ b/lms/views/api/moodle/files.py @@ -44,8 +44,10 @@ def via_url(_context, request): ) document_url = request.params["document_url"] - document_course_id = DOCUMENT_URL_REGEX.search(document_url)["course_id"] - document_file_id = DOCUMENT_URL_REGEX.search(document_url)["url"] + document_url_match = DOCUMENT_URL_REGEX.search(document_url) + assert document_url_match + document_course_id = document_url_match["course_id"] + document_file_id = document_url_match["url"] effective_file_id = _effective_file_id( course_copy_plugin, course, document_url, document_course_id, document_file_id ) diff --git a/lms/views/api/moodle/pages.py b/lms/views/api/moodle/pages.py index 9198c39390..979343e2c8 100644 --- a/lms/views/api/moodle/pages.py +++ b/lms/views/api/moodle/pages.py @@ -113,6 +113,7 @@ def proxy(self): @staticmethod def _parse_document_url(document_url): document_url_match = DOCUMENT_URL_REGEX.search(document_url) + assert document_url_match course_id = document_url_match["course_id"] page_id = document_url_match["page_id"] diff --git a/lms/views/lti/basic_launch.py b/lms/views/lti/basic_launch.py index d87b1e941a..f157870fe0 100644 --- a/lms/views/lti/basic_launch.py +++ b/lms/views/lti/basic_launch.py @@ -266,7 +266,7 @@ def _configure_assignment(self, assignment): ) def _configure_js_for_file_picker( - self, assignment: Assignment, route: str = "configure_assignment" + self, assignment: Assignment | None, route: str = "configure_assignment" ) -> dict: """ Show the file-picker for the user to choose a document. diff --git a/pyproject.toml b/pyproject.toml index 35e3ff3b3c..4c45f84b40 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -225,7 +225,7 @@ python_version = 3.11 warn_unused_configs = true warn_redundant_casts = true warn_unused_ignores = true - +check_untyped_defs = true disable_error_code = [ "import-untyped",