Add type hints to src/palace/manager/api/admin/ code. by dbernstein · Pull Request #3046 · ThePalaceProject/circulation

dbernstein · 2026-02-12T17:40:32Z

Description

As part of the ongoing effort to add type hints everywhere. This PR adds type hints to everything below src/palace/manager/api/admin/ and tightens up an implementations that require changes due to type hint additions.

Motivation and Context

Long term team-wide effort.

How Has This Been Tested?

Tests pass. Mypy passes.

Checklist

I have updated the documentation accordingly.
All new and existing tests passed.

codecov · 2026-02-13T20:49:47Z

Codecov Report

❌ Patch coverage is 79.82833% with 47 lines in your changes missing coverage. Please review.
✅ Project coverage is 93.12%. Comparing base (0822a6a) to head (3b9f135).
⚠️ Report is 3 commits behind head on main.

Files with missing lines	Patch %	Lines
src/palace/manager/api/admin/validator.py	74.28%	6 Missing and 3 partials ⚠️
...anager/api/admin/controller/collection_settings.py	30.00%	6 Missing and 1 partial ⚠️
.../manager/api/admin/controller/metadata_services.py	41.66%	6 Missing and 1 partial ⚠️
...nager/api/admin/controller/patron_auth_services.py	36.36%	6 Missing and 1 partial ⚠️
src/palace/manager/api/admin/controller/lanes.py	60.00%	6 Missing ⚠️
...alace/manager/api/admin/controller/custom_lists.py	50.00%	4 Missing ⚠️
...e/manager/api/admin/controller/catalog_services.py	71.42%	2 Missing ⚠️
...manager/api/admin/controller/discovery_services.py	71.42%	2 Missing ⚠️
...pi/admin/password_admin_authentication_provider.py	75.00%	1 Missing and 1 partial ⚠️
src/palace/manager/api/admin/routes.py	98.85%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3046      +/-   ##
==========================================
- Coverage   93.20%   93.12%   -0.09%     
==========================================
  Files         491      491              
  Lines       45248    45338      +90     
  Branches     6222     6228       +6     
==========================================
+ Hits        42172    42219      +47     
- Misses       1992     2029      +37     
- Partials     1084     1090       +6

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jonathangreen

This looks good! Need to address some of the Any type issues here though before it gets merged.

jonathangreen · 2026-02-19T14:48:27Z

src/palace/manager/api/admin/password_admin_authentication_provider.py

    @staticmethod
-    def forgot_password_template(redirect):
+    def forgot_password_template(
+        redirect: str | None | Response | WerkzeugResponse,


I think this should be str | None, we don't want to pass a Response into a jinja template do we?

jonathangreen · 2026-02-19T14:48:47Z

src/palace/manager/api/admin/password_admin_authentication_provider.py

+    def reset_password_template(
+        reset_password_token: str,
+        admin_id: int,
+        redirect: str | None | Response | WerkzeugResponse,


Same as: https://github.com/ThePalaceProject/circulation/pull/3046/changes#r2828319615

jonathangreen · 2026-02-19T14:49:55Z

src/palace/manager/api/admin/routes.py

@@ -1,7 +1,9 @@
+# Decorators from palace.manager.api.routes and core.app_server are untyped.
+# mypy: disallow_untyped_decorators=false


Instead of this, lets add types to these decorators

jonathangreen · 2026-02-19T14:50:30Z

src/palace/manager/api/admin/routes.py

+) -> Callable[..., Any]:
    @wraps(f)
-    def decorated(*args, **kwargs):
+    def decorated(*args: Any, **kwargs: Any) -> Any:


For all of these decorators we should add actual types instead of defaulting to 'Any'.

circulation/src/palace/manager/api/admin/routes.py

Lines 47 to 65 in d4e43d6

def requires_basic_auth[**P, T](func: Callable[P, T]) -> Callable[P, T | ProblemDetail]:

"""Basic auth for stateless system admin only API calls."""

@wraps(func)

def wrapper(*args: P.args, **kwargs: P.kwargs) -> T | ProblemDetail:

auth = request.authorization

if not auth or auth.username is None or auth.password is None:

return INVALID_ADMIN_CREDENTIALS

admin = Admin.authenticate(app.manager._db, auth.username, auth.password)

if not admin:

return INVALID_ADMIN_CREDENTIALS

if not admin.is_system_admin():

return ADMIN_NOT_AUTHORIZED

return func(*args, **kwargs)

return wrapper

provides an example of how these should be typed.

jonathangreen · 2026-02-19T14:51:00Z

src/palace/manager/api/admin/routes.py

 @app.route("/admin/sign_in_with_password", methods=["POST"])
 @returns_problem_detail
-def password_auth():
+def password_auth() -> Any:


For all these routes, we should add types instead of defaulting to Any.

jonathangreen · 2026-02-19T15:01:34Z

src/palace/manager/api/admin/validator.py

@@ -1,4 +1,10 @@
+from __future__ import annotations
+
+# mypy: warn_unreachable=false


We shouldn't add this at the file level. If we need this ignore, lets add it line by line, so that we get warnings when its unnecessary. Ideally though, we wouldn't need this at all. usually this is a sign that something is incorrectly typed.

jonathangreen · 2026-02-19T15:07:33Z

src/palace/manager/api/admin/validator.py

+        self,
+        settings: list[dict[str, Any]],
+        value: str,
+        form: dict[str, Any] | None,


Minor: The form = form or {} on line 46 handles None, but the callers in validate_email (line 72) already do content.get("form") or {}, so None can never actually reach here. The | None is harmless but misleading.

jonathangreen · 2026-02-19T15:08:21Z

src/palace/manager/api/admin/template_styles.py

@@ -1,4 +1,4 @@
-body_style = """
+body_style: str = """


Minor: Adding : str to every module-level string constant adds noise. Mypy already infers these as str. This isn't wrong, just unnecessary.

jonathangreen · 2026-02-19T15:09:49Z

src/palace/manager/api/admin/validator.py

        return [_f for _f in result if _f]

-    def _value(self, field, form):
+    def _value(self, field: dict[str, Any], form: Any) -> Any:


Typing form as Any defeats the purpose. Any means mypy won't catch misuse.

jonathangreen · 2026-02-19T15:09:58Z

src/palace/manager/api/admin/validator.py


-    def _list_of_values(self, fields, form):
-        result = []
+    def _list_of_values(self, fields: list[dict[str, Any]], form: Any) -> list[Any]:


Typing form as Any defeats the purpose. Any means mypy won't catch misuse.

dbernstein force-pushed the add-type-hints-to-api-admin branch 2 times, most recently from 168c3c1 to d4e43d6 Compare February 13, 2026 20:42

dbernstein marked this pull request as ready for review February 17, 2026 01:34

dbernstein requested a review from a team February 17, 2026 01:34

jonathangreen reviewed Feb 19, 2026

View reviewed changes

dbernstein force-pushed the add-type-hints-to-api-admin branch from d4e43d6 to 60aee91 Compare February 24, 2026 21:15

dbernstein added 3 commits March 3, 2026 11:35

Add type hints to src/palace/manager/api/admin/ code.

d187da0

Fix broken tests.

14b6342

Address PR review comments.

3b9f135

dbernstein force-pushed the add-type-hints-to-api-admin branch from 60aee91 to 3b9f135 Compare March 3, 2026 20:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add type hints to src/palace/manager/api/admin/ code.#3046

Add type hints to src/palace/manager/api/admin/ code.#3046
dbernstein wants to merge 3 commits intomainfrom
add-type-hints-to-api-admin

dbernstein commented Feb 12, 2026 •

edited

Loading

Uh oh!

codecov bot commented Feb 13, 2026 •

edited

Loading

Uh oh!

jonathangreen left a comment

Uh oh!

jonathangreen Feb 19, 2026

Uh oh!

jonathangreen Feb 19, 2026

Uh oh!

jonathangreen Feb 19, 2026

Uh oh!

jonathangreen Feb 19, 2026

Uh oh!

jonathangreen Feb 19, 2026

Uh oh!

jonathangreen Feb 19, 2026

Uh oh!

jonathangreen Feb 19, 2026

Uh oh!

jonathangreen Feb 19, 2026

Uh oh!

jonathangreen Feb 19, 2026

Uh oh!

jonathangreen Feb 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		@@ -1,7 +1,9 @@
		# Decorators from palace.manager.api.routes and core.app_server are untyped.
		# mypy: disallow_untyped_decorators=false

	def requires_basic_auth[**P, T](func: Callable[P, T]) -> Callable[P, T \| ProblemDetail]:
	"""Basic auth for stateless system admin only API calls."""

	@wraps(func)
	def wrapper(args: P.args, *kwargs: P.kwargs) -> T \| ProblemDetail:
	auth = request.authorization
	if not auth or auth.username is None or auth.password is None:
	return INVALID_ADMIN_CREDENTIALS

	admin = Admin.authenticate(app.manager._db, auth.username, auth.password)

	if not admin:
	return INVALID_ADMIN_CREDENTIALS

	if not admin.is_system_admin():
	return ADMIN_NOT_AUTHORIZED
	return func(args, *kwargs)

	return wrapper

		@@ -1,4 +1,10 @@
		from __future__ import annotations

		# mypy: warn_unreachable=false

Conversation

dbernstein commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Motivation and Context

How Has This Been Tested?

Checklist

Uh oh!

codecov bot commented Feb 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

jonathangreen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

dbernstein commented Feb 12, 2026 •

edited

Loading

codecov bot commented Feb 13, 2026 •

edited

Loading