From b365eb9bfff4e42fcb8a7a489cd2f369c8b588ee Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Oct 2025 06:52:07 +0000 Subject: [PATCH 1/3] Initial plan From 7fdb4820f7043988b84d94d05b87e990227b5192 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Oct 2025 07:06:52 +0000 Subject: [PATCH 2/3] Add user role enum and users command for course management - Created UserRole enum (teacher, amanuensis, hourly) in courses and hr modules - Added 'users' command with add, list, and rm subcommands - Updated prep_factor to handle teacher role (prep time 3 for lectures, 1 for TAs) - Added set_course_config function to courses module - Added add_user and remove_user methods to StorageRoot - Teachers now differentiated from TAs with higher lecture prep time Co-authored-by: dbosk <237222+dbosk@users.noreply.github.com> --- src/nytid/cli/courses.nw | 160 ++++++++++++++++++++++++++++++++++++++ src/nytid/courses/init.nw | 31 ++++++++ src/nytid/signup/hr/hr.nw | 70 +++++++++++++---- src/nytid/storage/init.nw | 21 +++++ 4 files changed, 265 insertions(+), 17 deletions(-) diff --git a/src/nytid/cli/courses.nw b/src/nytid/cli/courses.nw index 6d22944..9524268 100644 --- a/src/nytid/cli/courses.nw +++ b/src/nytid/cli/courses.nw @@ -45,6 +45,7 @@ from typing_extensions import Annotated <> <> +<> cli = typer.Typer(name="courses", help="Manage courses") @@ -824,6 +825,27 @@ cli.add_typer(minecli) MINE = "mine" @ +\section{User roles}\label{userroles} + +We define an enum for the different user roles in a course. +There are three types of users: +\begin{itemize} +\item Teachers: responsible for the course, always treated like amanuenses, + have higher prep time factor (3 for lectures instead of 0 for TAs). +\item Amanuenses: TAs with amanuensis contracts, have slightly different + prep time calculation. +\item Hourly: TAs paid by the hour. +\end{itemize} +<>= +from enum import Enum + +class UserRole(str, Enum): + """User role in a course""" + TEACHER = "teacher" + AMANUENSIS = "amanuensis" + HOURLY = "hourly" +@ + The subcommands of [[mine]] will be similar to those of [[registry]]. We want to list them, add them and remove them. We will do this by maintaining a special register named [[MINE]] that will @@ -1131,3 +1153,141 @@ def complete_course_regex(ctx: typer.Context, regex: str) -> typing.List[str]: registers = registers_regex(register_regex) return courses_regex(regex, registers) +@ + + +\section{Managing course users, the \texttt{users} subcommands}\label{users} + +We want to manage users (teachers and TAs) for each course. +This allows us to control access to course data and differentiate between +teachers and TAs in terms of prep time and employment contracts. + +<>= +userscli = typer.Typer(name="users", + help="Manage course users (teachers and TAs)") + +cli.add_typer(userscli) + +<> +@ + +\subsection{Adding a user to a course} + +We add a user to a course by storing their username and role in the course +configuration. +<>= +@userscli.command(name="add") +def users_add(course: Annotated[str, course_arg_autocomplete], + username: Annotated[str, username_arg], + role: Annotated[UserRole, role_opt] = UserRole.HOURLY, + register: Annotated[str, register_arg] = None): + """ + Adds a user to a course with a specific role. + """ + <> + <> + <> + logging.info(f"Added {username} as {role.value} to {course}") +<>= +username_arg = typer.Argument(help="Username (e.g., KTH username)") +role_opt = typer.Option(help="User role: teacher, amanuensis, or hourly") +@ + +To get the course config, we need to determine which register to use. +<>= +if not register: + <> + +try: + course_config = courses.get_course_config(course, register) +except Exception as err: + logging.error(f"Can't access course {course}: {err}") + sys.exit(1) +@ + +Now we add the user to the course configuration. +We store users in a dictionary with username as key and role as value. +<>= +try: + users_dict = course_config.get("users") + if not isinstance(users_dict, dict): + users_dict = {} +except (KeyError, TypeError): + users_dict = {} + +users_dict[username] = role.value +courses.set_course_config(course, register, "users", users_dict) +@ + +We also need to grant the user access to the course's storage. +This is handled by the storage module. +<>= +try: + register_path = registry.get(register) + with storage.open_root(f"{register_path}/{course}") as root: + root.add_user(username) +except Exception as err: + logging.warning(f"Couldn't grant storage access to {username}: {err}") +@ + +\subsection{Listing users in a course} + +We list all users and their roles for a course. +<>= +@userscli.command(name="list") +def users_list(course: Annotated[str, course_arg_autocomplete], + register: Annotated[str, register_arg] = None): + """ + Lists all users and their roles for a course. + """ + <> + + try: + users_dict = course_config.get("users") + if not users_dict or not isinstance(users_dict, dict): + logging.info(f"No users configured for {course}") + return + except KeyError: + logging.info(f"No users configured for {course}") + return + + print(f"Users in {course}:") + for username, role in users_dict.items(): + print(f" {username}\t{role}") +@ + +\subsection{Removing a user from a course} + +We remove a user from a course by removing them from the course configuration. +<>= +@userscli.command(name="rm") +def users_rm(course: Annotated[str, course_arg_autocomplete], + username: Annotated[str, username_arg], + register: Annotated[str, register_arg] = None): + """ + Removes a user from a course. + """ + <> + + try: + users_dict = course_config.get("users") + if not users_dict or username not in users_dict: + logging.error(f"User {username} not found in {course}") + sys.exit(1) + except KeyError: + logging.error(f"No users configured for {course}") + sys.exit(1) + + del users_dict[username] + courses.set_course_config(course, register, "users", users_dict) + + <> + logging.info(f"Removed {username} from {course}") +<>= +try: + register_path = registry.get(register) + with storage.open_root(f"{register_path}/{course}") as root: + root.remove_user(username) +except Exception as err: + logging.warning(f"Couldn't revoke storage access from {username}: {err}") +@ diff --git a/src/nytid/courses/init.nw b/src/nytid/courses/init.nw index 7a5dc27..3ba5eb0 100644 --- a/src/nytid/courses/init.nw +++ b/src/nytid/courses/init.nw @@ -464,4 +464,35 @@ def get_course_data(course: str, """ course_conf = get_course_config(course, register, config) return storage.open_root(course_conf.get("data_path")) +@ + + +\section{Setting course configuration values} +We want to be able to set values in a course's configuration. +<>= +def set_course_config(course: str, + register: str = None, + key: str = None, + value = None, + <>): + """ + Sets a value in the course's configuration. + + `course` identifies the course in the `register`. If `register` is None, + search through all registers in the registry, use the first one found + (undefined in which order duplicates are sorted). + + `key` is the configuration key to set. + `value` is the value to set for the key. + + The default `config` is the default config of the `typerconf` package. + """ + course_conf = get_course_config(course, register, config) + course_conf.set(key, value) + + # Force write back by accessing the config's internal file path + if hasattr(course_conf, 'conf_file') and course_conf.conf_file: + with open(course_conf.conf_file, 'w') as f: + course_conf.write_config(f) +@ diff --git a/src/nytid/signup/hr/hr.nw b/src/nytid/signup/hr/hr.nw index 4cec373..790fcab 100644 --- a/src/nytid/signup/hr/hr.nw +++ b/src/nytid/signup/hr/hr.nw @@ -5,14 +5,28 @@ The module is constructed as follows. <>= import arrow import datetime +from enum import Enum from nytid.signup import sheets from nytid.signup.sheets import SIGNUP_SHEET_HEADER import typerconf as config +<> <> <> @ +\section{User roles} + +We define an enum for the different user roles. +This enum matches the one in [[nytid.cli.courses]]. +<>= +class UserRole(str, Enum): + """User role in a course""" + TEACHER = "teacher" + AMANUENSIS = "amanuensis" + HOURLY = "hourly" +@ + \section{Rounding time and adding prep time correctly} The KTH policy on prep time for different kinds of events depends on what kind @@ -24,16 +38,20 @@ We provide a function that computes the time for an event according to the policy: \label{HRaddprep} <>= -def time_for_event(event, amanuensis=False): +def time_for_event(event, role=UserRole.HOURLY): """ - Input: an event of type ics.event.Event and an optional bool amanuensis - specifying whether the computation is for an amanuensis or not. + Input: an event of type ics.event.Event and an optional role + specifying whether the computation is for a teacher, amanuensis or hourly TA. Output: Returns a datetime.timedelta corresponding to the time including prep time for the event. """ + # Convert role to UserRole if it's a string + if isinstance(role, str): + role = UserRole(role) + return add_prep_time( - round_time(event.end-event.begin), event.name, event.begin, amanuensis) + round_time(event.end-event.begin), event.name, event.begin, role) @ KTH uses some rounding procedure for worked time. @@ -73,7 +91,7 @@ including prep time. <>= def add_prep_time(time, event_type, date=datetime.date.today(), - amanuensis=False): + role=UserRole.HOURLY): """ Input: - a datetime.timedelta object time, @@ -81,11 +99,16 @@ def add_prep_time(time, event_type, - an optional date (datetime or arrow object) indicating the date of the event. If no date is given, today's date is assumed, meaning the latest prep-time policy will be used. - - an optional bool indicating amanuensis employment or hourly. + - an optional role (UserRole enum or string) indicating teacher, amanuensis + or hourly employment. Output: the time object rounded according to KTH rules. """ - time *= prep_factor(event_type, date=date, amanuensis=amanuensis) + # Convert role to UserRole if it's a string + if isinstance(role, str): + role = UserRole(role) + + time *= prep_factor(event_type, date=date, role=role) return round_time(time) @ @@ -96,7 +119,7 @@ This function must be adapted to current policy\footnote{% Currently, lab session time should increase by 1.5 online and 1.8 on campus; tutorial session time should double. <>= -def prep_factor(event_type, date, amanuensis): +def prep_factor(event_type, date, role): """ Computes the prep time factor for the event type and date. This should implement the rules found at @@ -108,9 +131,13 @@ def prep_factor(event_type, date, amanuensis): "seminar", "laboration", etc. - date: a datetime.date or arrow.Arrow object indicating the date of the event. - - amanuensis: a bool indicating whether the event is for an amanuensis or - hourly. + - role: a UserRole enum or string indicating whether the event is for + a teacher, amanuensis or hourly TA. """ + # Convert role to UserRole if it's a string + if isinstance(role, str): + role = UserRole(role) + if isinstance(date, arrow.Arrow): date = date.datetime if isinstance(date, datetime.datetime): @@ -118,10 +145,16 @@ def prep_factor(event_type, date, amanuensis): event_type = event_type.casefold() + # Teachers do prep for lectures (factor 3), TAs help during lectures (no prep) + if "föreläsning" in event_type or "lecture" in event_type: + if role == UserRole.TEACHER: + return 3 + else: + return 1 # TAs just help during lectures, no prep + + # For exercises, everyone gets prep time if "övning" in event_type or "exercise" in event_type: return 2 - elif "föreläsning" in event_type or "lecture" in event_type: - return 3 tutoring = [ "laboration", @@ -132,16 +165,19 @@ def prep_factor(event_type, date, amanuensis): "question" ] + # Teachers and amanuenses are treated the same for tutoring + is_amanuensis_or_teacher = role in (UserRole.AMANUENSIS, UserRole.TEACHER) + if ( date < datetime.date(2023, 1, 1) or (date < datetime.date(2022, 10, 1) - and not amanuensis) + and not is_amanuensis_or_teacher) ): if check_substrings(tutoring, event_type): return 1.33 else: if check_substrings(tutoring, event_type): - if "online" in event_type or amanuensis: + if "online" in event_type or is_amanuensis_or_teacher: return 1.5 return 1.8 @@ -443,12 +479,12 @@ except (KeyError, ValueError): \subsection{Fix prep time function for amanuensis} We need to adjust the default argument for [[add_prep_time]]. -By default [[amanuensis]] is set to false, -but in this case we want it to true. +By default [[role]] is set to hourly, +but in this case we want it to amanuensis. <>= amanuensis_prep_time = lambda time, event_type, date: \ add_prep_time(time, event_type, - date, amanuensis=True) + date, role=UserRole.AMANUENSIS) @ \subsection{Compute the earliest and latest dates} diff --git a/src/nytid/storage/init.nw b/src/nytid/storage/init.nw index c7fd09e..19fcd4e 100644 --- a/src/nytid/storage/init.nw +++ b/src/nytid/storage/init.nw @@ -297,6 +297,27 @@ def grant_access(user): def revoke_access(user): <> + +def add_user(self, username): + """ + Grant read and write access to the storage root for a user. + This is a convenience method that calls grant_access with + appropriate permissions. + """ + try: + self.grant_access(username, "rl") + except NotImplementedError: + pass + +def remove_user(self, username): + """ + Revoke access to the storage root for a user. + This is a convenience method that calls revoke_access. + """ + try: + self.revoke_access(username) + except NotImplementedError: + pass <>= """ This method is not implemented for `nytid.cli.storage.StorageRoot`. From 0ecf9fcb484bc7a090ef8e98d23f17a19ced8523 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 14 Oct 2025 07:08:40 +0000 Subject: [PATCH 3/3] Add user role lookup helper and verify functionality - Added get_user_role helper function to hr CLI - Verified prep time calculations work correctly for all roles - Teachers get 3x prep time for lectures, TAs get 1x - Teachers and amanuenses treated the same for tutoring (1.5x) - All tests passing (10 expected failures for AFS/LADOK) Co-authored-by: dbosk <237222+dbosk@users.noreply.github.com> --- src/nytid/cli/hr.nw | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/src/nytid/cli/hr.nw b/src/nytid/cli/hr.nw index 7a4c6f1..5f9b933 100644 --- a/src/nytid/cli/hr.nw +++ b/src/nytid/cli/hr.nw @@ -198,6 +198,42 @@ except Exception as err: _canvas_session = None @ +\subsection{Getting user roles from course config} + +We need a helper function to get a user's role from the course configuration. +This allows us to determine if they are a teacher, amanuensis, or hourly TA. +<>= +def get_user_role(username, course, register=None): + """ + Gets the role for a user from the course configuration. + + Returns a UserRole enum value, or None if the user is not in the course config. + Falls back to checking contracts if no explicit role is configured. + """ + try: + from nytid.signup.hr import UserRole + course_config = courseutils.get_course_config(course, register) + users_dict = course_config.get("users") + + if users_dict and isinstance(users_dict, dict): + if username in users_dict: + role_str = users_dict[username] + return UserRole(role_str) if isinstance(role_str, str) else role_str + except (KeyError, ValueError) as err: + logging.debug(f"Couldn't get role for {username} from config: {err}") + + # Fallback: check if they have amanuensis contracts + try: + contracts = get_user_contracts(username) + if contracts: + return UserRole.AMANUENSIS + except Exception: + pass + + # Default to hourly if no information found + return UserRole.HOURLY +@ + \subsection{Looking up usernames in Canvas and LADOK} Now that we have a (hopefully) working [[canvas_session]] and