Merge branch 'lib_user_nullability' into lib_user_explode

pycroft/helpers/printing/__init__.py

@@ -100,7 +100,7 @@ def generate_user_sheet(
     new_user: bool,
     wifi: bool,
     bank_account: BankAccount,
-    user: User | None = None,
+    user: User = None,
     user_id: str | None = None,
     plain_user_password: str | None = None,
     generation_purpose: str = "",

pycroft/lib/mail.py

@@ -18,13 +18,13 @@
 
 from pycroft.lib.exc import PycroftLibException
 
-mail_envelope_from = os.environ.get('PYCROFT_MAIL_ENVELOPE_FROM')
-mail_from = os.environ.get('PYCROFT_MAIL_FROM')
-mail_reply_to = os.environ.get('PYCROFT_MAIL_REPLY_TO')
-smtp_host = os.environ.get('PYCROFT_SMTP_HOST')
+mail_envelope_from = os.environ["PYCROFT_MAIL_ENVELOPE_FROM"]
+mail_from = os.environ["PYCROFT_MAIL_FROM"]
+mail_reply_to = os.environ["PYCROFT_MAIL_REPLY_TO"]
+smtp_host = os.environ["PYCROFT_SMTP_HOST"]
 smtp_port = int(os.environ.get('PYCROFT_SMTP_PORT', 465))
-smtp_user = os.environ.get('PYCROFT_SMTP_USER')
-smtp_password = os.environ.get('PYCROFT_SMTP_PASSWORD')
+smtp_user = os.environ["PYCROFT_SMTP_USER"]
+smtp_password = os.environ["PYCROFT_SMTP_PASSWORD"]
 smtp_ssl = os.environ.get('PYCROFT_SMTP_SSL', 'ssl')
 template_path_type = os.environ.get('PYCROFT_TEMPLATE_PATH_TYPE', 'filesystem')
 template_path = os.environ.get('PYCROFT_TEMPLATE_PATH', 'pycroft/templates')
@@ -68,12 +68,12 @@ def render(self, **kwargs: t.Any) -> tuple[str, str]:
 
 
 def compose_mail(mail: Mail) -> MIMEMultipart:
-    msg = MIMEMultipart('alternative', _charset='utf-8')
-    msg['Message-Id'] = make_msgid()
-    msg['From'] = mail_from
-    msg['To'] = Header(mail.to_address)
-    msg['Subject'] = mail.subject
-    msg['Date'] = formatdate(localtime=True)
+    msg = MIMEMultipart("alternative", _charset="utf-8")
+    msg["Message-Id"] = make_msgid()
+    msg["From"] = mail_from
+    msg["To"] = str(Header(mail.to_address))
+    msg["Subject"] = mail.subject
+    msg["Date"] = formatdate(localtime=True)
 
     msg.attach(MIMEText(mail.body_plain, 'plain', _charset='utf-8'))
 

pycroft/lib/task.py

@@ -197,7 +197,7 @@ def schedule_user_task(
     due: DateTimeTz,
     user: User,
     parameters: TaskParams,
-    processor: User,
+    processor: User | None,
 ) -> UserTask:
     if due < session.utcnow():
         raise ValueError("the due date must be in the future")

pycroft/lib/user/blocking.py

@@ -29,7 +29,7 @@ def block(
     user: User,
     reason: str,
     processor: User,
-    during: Interval[DateTimeTz] = None,
+    during: Interval[DateTimeTz] | None = None,
     violation: bool = True,
 ) -> User:
     """Suspend a user during a given interval.

pycroft/lib/user/edit.py

@@ -98,7 +98,7 @@ def edit_email(
 
 
 @with_transaction
-def edit_birthdate(user: User, birthdate: date, processor: User) -> User:
+def edit_birthdate(user: User, birthdate: date | None, processor: User) -> User:
     """
     Changes the birthdate of a user and creates a log entry.
 

pycroft/lib/user/lifecycle.py

@@ -58,9 +58,9 @@ def create_user(
     groups: t.Iterable[PropertyGroup],
     processor: User | None,
     address: Address,
-    passwd_hash: str = None,
+    passwd_hash: str | None = None,
     send_confirm_mail: bool = False,
-) -> tuple[User, str]:
+) -> tuple[User, str | None]:
     """Create a new member
 
     Create a new user with a generated password, finance- and unix account, and make him member
@@ -139,7 +139,7 @@ def login_available(login: str, session: Session) -> bool:
             .add_columns(1)
         )
     )
-    return session.scalar(stmt)
+    return session.scalars(stmt).one()
 
 
 @with_transaction
@@ -150,7 +150,7 @@ def move_in(
     room_number: str,
     mac: str | None,
     processor: User | None = None,
-    birthdate: date = None,
+    birthdate: date | None = None,
     host_annex: bool = False,
     begin_membership: bool = True,
     when: DateTimeTz | None = None,
@@ -180,6 +180,7 @@ def move_in(
 
     :return: The user object.
     """
+    processor = processor if processor is not None else user
 
     if when and when > session.utcnow():
         task_params = UserMoveInParams(
@@ -237,13 +238,15 @@ def move_in(
                 session.session.add(Interface(mac=mac, host=new_host))
                 setup_ipv4_networking(session.session, new_host)
 
-    user_send_mail(user, UserMovedInTemplate(), True)
+        msg = deferred_gettext("Moved in: {room}").format(room=room.short_name)
+    else:
+        msg = deferred_gettext("Moved in!")
 
-    msg = deferred_gettext("Moved in: {room}")
+    user_send_mail(user, UserMovedInTemplate(), True)
 
     log_user_event(
-        author=processor if processor is not None else user,
-        message=msg.format(room=room.short_name).to_json(),
+        author=processor,
+        message=msg.to_json(),
         user=user,
     )
 

pycroft/lib/user/mail.py

@@ -38,7 +38,7 @@ def format_user_mail(user: User, text: str) -> str:
         id=encode_type2_user_id(user.id),
         email=user.email if user.email else "-",
         email_internal=user.email_internal,
-        room_short=user.room.short_name if user.room_id is not None else "-",
+        room_short=user.room.short_name if user.room is not None else "-",
         swdd_person_id=user.swdd_person_id if user.swdd_person_id else "-",
     )
 
@@ -48,8 +48,8 @@ def user_send_mails(
     template: MailTemplate | None = None,
     soft_fail: bool = False,
     use_internal: bool = True,
-    body_plain: str = None,
-    subject: str = None,
+    body_plain: str | None = None,
+    subject: str | None = None,
     **kwargs: t.Any,
 ) -> None:
     """
@@ -94,6 +94,8 @@ def user_send_mails(
             # No template given, use formatted body_mail instead.
             if not isinstance(user, User):
                 raise ValueError("Plaintext email not supported for other User types.")
+            if body_plain is None:
+                raise ValueError("Must use either template or body_plain")
 
             html = None
             plaintext = format_user_mail(user, body_plain)

pycroft/lib/user/mail_confirmation.py

@@ -30,6 +30,7 @@ def confirm_mail_address(
     # else: one of {mr, user} is not None
 
     if user is None:
+        assert mr is not None
         if mr.email_confirmed:
             raise ValueError("E-Mail already confirmed")
 
@@ -51,6 +52,7 @@ def confirm_mail_address(
 
         return "pre_member", reg_result
     elif mr is None:
+        assert user is not None
         user.email_confirmed = True
         user.email_confirmation_key = None
 

pycroft/lib/user/member_request.py

@@ -2,8 +2,10 @@
 import typing as t
 from datetime import timedelta, date
 from difflib import SequenceMatcher
+from itertools import chain
 
-from sqlalchemy import func
+from sqlalchemy import func, select
+from sqlalchemy.orm import Session
 
 from pycroft import config
 from pycroft.helpers import utc
@@ -48,7 +50,6 @@
     user_send_mail,
 )
 from .user_id import (
-    check_user_id,
     decode_type1_user_id,
     decode_type2_user_id,
     encode_type2_user_id,
@@ -68,8 +69,6 @@ def create_member_request(
     previous_dorm: str | None,
 ) -> PreMember:
     check_new_user_data(
-        login,
-        email,
         name,
         swdd_person_id,
         room,
@@ -112,6 +111,9 @@ def create_member_request(
 def finish_member_request(
     prm: PreMember, processor: User | None, ignore_similar_name: bool = False
 ) -> User:
+    assert prm.email is not None, f"{prm!r} not persisted"
+    assert prm.move_in_date is not None, f"{prm!r} not persisted"
+
     if prm.room is None:
         raise ValueError("Room is None")
 
@@ -121,14 +123,17 @@ def finish_member_request(
         prm.move_in_date = utcnow.date()
 
     check_new_user_data(
-        prm.login,
-        prm.email,
         prm.name,
         prm.swdd_person_id,
         prm.room,
         prm.move_in_date,
         ignore_similar_name,
     )
+    check_new_user_data_unused(
+        login=prm.login,
+        email=prm.email,
+        swdd_person_id=prm.swdd_person_id,
+    )
 
     user = user_from_pre_member(prm, processor=processor)
     processor = processor or user
@@ -153,7 +158,9 @@ def finish_member_request(
     return user
 
 
-def user_from_pre_member(pre_member: PreMember, processor: User) -> User:
+def user_from_pre_member(pre_member: PreMember, processor: User | None) -> User:
+    assert pre_member.email is not None, f"{pre_member!r} not persisted"
+    assert pre_member.birthdate is not None, f"{pre_member!r} not persisted"
     user, _ = create_user(
         pre_member.name,
         pre_member.login,
@@ -229,8 +236,10 @@ def merge_member_request(
         )
 
     if merge_person_id:
+        assert prm.swdd_person_id is not None
         user = edit_person_id(user, prm.swdd_person_id, processor)
 
+    assert prm.move_in_date is not None
     move_in_datetime = utc.with_min_time(prm.move_in_date)
 
     if merge_room:
@@ -293,25 +302,27 @@ def merge_member_request(
 
 
 def get_possible_existing_users_for_pre_member(prm: PreMember) -> set[User]:
+    sess: Session = session.session  # TODO make parameter
+
+    assert prm.email is not None, f"{prm!r} not persisted!"
+
     user_swdd_person_id = get_user_by_swdd_person_id(prm.swdd_person_id)
-    user_login = User.q.filter_by(login=prm.login).first()
-    user_email = User.q.filter(func.lower(User.email) == prm.email.lower()).first()
+    user_login = sess.scalar(select(User).filter_by(login=prm.login))
+    user_email = sess.scalar(select(User).where(func.lower(User.email) == prm.email.lower()))
 
-    users_name = User.q.filter_by(name=prm.name).all()
+    users_name = sess.scalars(select(User).filter_by(name=prm.name)).all()
     users_similar = get_similar_users_in_room(prm.name, prm.room, 0.5)
 
     users = {
         user
-        for user in [user_swdd_person_id, user_login, user_email] + users_name + users_similar
+        for user in chain((user_swdd_person_id, user_login, user_email), users_name, users_similar)
         if user is not None
     }
 
     return users
 
 
 def check_new_user_data(
-    login: str,
-    email: str,
     name: str,
     swdd_person_id: int | None,
     room: Room | None,
@@ -327,7 +338,7 @@ def check_new_user_data(
             raise MoveInDateInvalidException
 
 
-def check_new_user_data_unused(login: str, email: str, swdd_person_id: int) -> None:
+def check_new_user_data_unused(login: str, email: str, swdd_person_id: int | None) -> None:
     """Check whether some user data from a member request is already used.
 
     :raises UserExistsException:
@@ -382,27 +393,20 @@ def get_name_from_first_last(first_name: str, last_name: str) -> str:
 
 
 def get_user_by_id_or_login(ident: str, email: str) -> User | None:
-    re_uid1 = r"^\d{4,6}-\d{1}$"
-    re_uid2 = r"^\d{4,6}-\d{2}$"
-
-    user = User.q.filter(func.lower(User.email) == email.lower())
-
-    if re.match(re_uid1, ident):
-        if not check_user_id(ident):
-            return None
-        user_id, _ = decode_type1_user_id(ident)
-        user = user.filter_by(id=user_id)
-    elif re.match(re_uid2, ident):
-        if not check_user_id(ident):
-            return None
-        user_id, _ = decode_type2_user_id(ident)
-        user = user.filter_by(id=user_id)
+    stmt = select(User).where(func.lower(User.email) == email.lower())
+
+    if (d := decode_type1_user_id(ident)) is not None:
+        user_id, _ = d
+        stmt = stmt.filter_by(id=user_id)
+    elif (d := decode_type2_user_id(ident)) is not None:
+        user_id, _ = d
+        stmt = stmt.filter_by(id=user_id)
     elif re.match(BaseUser.login_regex, ident):
-        user = user.filter_by(login=ident)
+        stmt = stmt.filter_by(login=ident)
     else:
         return None
 
-    return t.cast(User | None, user.one_or_none())
+    return session.session.scalar(stmt)
 
 
 def find_similar_users(name: str, room: Room, ratio: float) -> t.Iterable[User]:

pycroft/lib/user/user_sheet.py

@@ -13,9 +13,9 @@
 def store_user_sheet(
     new_user: bool,
     wifi: bool,
-    user: User | None = None,
+    user: User,
     timeout: int = 15,
-    plain_user_password: str = None,
+    plain_user_password: str | None = None,
     generation_purpose: str = "",
     plain_wifi_password: str = "",
 ) -> WebStorage:
@@ -65,7 +65,7 @@ def get_user_sheet(sheet_id: int) -> bytes | None:
 def generate_user_sheet(
     new_user: bool,
     wifi: bool,
-    user: User | None = None,
+    user: User,
     plain_user_password: str | None = None,
     generation_purpose: str = "",
     plain_wifi_password: str = "",

pyproject.toml

@@ -169,7 +169,11 @@ disallow_untyped_globals = true
 
 [[tool.mypy.overrides]]
 module = [
+    "pycroft.model.task_serialization",
     "pycroft.lib.finance",
+    "pycroft.lib.mail",
+    "pycroft.lib.user",
+    "pycroft.lib.user.*",
 ]
 strict_optional = true