feat: improve function of adding students to mm team (#61)

This reverts commit 3dc6667716.

.pre-commit-config.yaml

@@ -1,41 +1,41 @@
 repos:
   - repo: https://github.com/pre-commit/pre-commit-hooks
-    rev: v4.4.0
+    rev: v6.0.0
     hooks:
       - id: check-yaml
       - id: end-of-file-fixer
       - id: trailing-whitespace
       - id: requirements-txt-fixer
   - repo: https://github.com/pre-commit/mirrors-mypy
-    rev: "v1.4.1"
+    rev: "v1.18.2"
     hooks:
       - id: mypy
         additional_dependencies:
           - pydantic
   - repo: https://github.com/asottile/pyupgrade
-    rev: v3.9.0
+    rev: v3.20.0
     hooks:
       - id: pyupgrade
   - repo: https://github.com/hadialqattan/pycln
-    rev: v2.4.0
+    rev: v2.5.0
     hooks:
       - id: pycln
         args: [-a]
   - repo: https://github.com/PyCQA/bandit
-    rev: '1.7.5'
+    rev: '1.8.6'
     hooks:
     - id: bandit
   - repo: https://github.com/PyCQA/isort
-    rev: 5.12.0
+    rev: 6.0.1
     hooks:
       - id: isort
         args: ["--profile", "black", "--filter-files"]
   - repo: https://github.com/psf/black
-    rev: 23.7.0
+    rev: 25.9.0
     hooks:
       - id: black
   - repo: https://github.com/Lucas-C/pre-commit-hooks
-    rev: v1.5.1
+    rev: v1.5.5
     hooks:
       - id: remove-crlf
       - id: remove-tabs

joint_teapot/app.py

@@ -6,7 +6,7 @@ from pathlib import Path
 from time import sleep
 from typing import TYPE_CHECKING, List, Optional
 
-# from filelock import FileLock
+from filelock import FileLock
 from git import Repo
 from typer import Argument, Exit, Option, Typer, echo
 
@@ -242,6 +242,13 @@ def create_personal_channels_on_mm(
 ) -> None:
     tea.pot.create_channels_for_individuals(invite_teaching_team)
 
+@app.command(
+    "invite-students-to-mm-team",
+    help="invite all canvas students to the mattermost team",
+)
+def invite_students_to_mattermost_team() -> None:
+    tea.pot.invite_students_to_mattermost_team()
+
 
 @app.command(
     "create-webhooks-for-mm",
@@ -349,6 +356,7 @@ def joj3_all_env(
     submitter_repo_name = env.github_repository.split("/")[-1]
     penalty_factor = joj3.get_penalty_factor(end_time, penalty_config)
     total_score = joj3.get_total_score(env.joj3_output_path)
+    total_score = round(total_score - abs(total_score) * (1 - penalty_factor))
     res = {
         "totalScore": total_score,
         "cappedTotalScore": (
@@ -388,13 +396,12 @@ def joj3_all_env(
     lock_file_path = os.path.join(
         settings.repos_dir, grading_repo_name, settings.joj3_lock_file_path
     )
-    logger.info(
+    logger.debug(
         f"try to acquire lock, file path: {lock_file_path}, "
         + f"timeout: {settings.joj3_lock_file_timeout}"
     )
-    if True:  # disable the file lock temporarily
-        # with FileLock(lock_file_path, timeout=settings.joj3_lock_file_timeout).acquire():
-        logger.info("file lock acquired")
+    with FileLock(lock_file_path, timeout=settings.joj3_lock_file_timeout).acquire():
+        logger.debug("file lock acquired")
         retry_interval = 1
         git_push_ok = False
         while not git_push_ok:
@@ -424,7 +431,9 @@ def joj3_all_env(
                     os.path.join(repo_path, scoreboard_filename),
                     exercise_name,
                     submitter_repo_name,
+                    total_score,
                 )
+                failed_stage = joj3.get_failed_stage_from_file(env.joj3_output_path)
                 tea.pot.git.add_commit(
                     grading_repo_name,
                     [scoreboard_filename],
@@ -434,6 +443,7 @@ def joj3_all_env(
                         f"gitea actions link: {gitea_actions_url}\n"
                         f"gitea issue link: {gitea_issue_url}\n"
                         f"groups: {env.joj3_groups}\n"
+                        f"failed stage: {failed_stage}\n"
                     ),
                 )
             if not skip_failed_table:
@@ -502,6 +512,9 @@ def joj3_check_env(
             "Example: --penalty-config 24=0.75,48=0.5"
         ),
     ),
+    ignore_submitter: bool = Option(
+        False, help="ignore submitter when checking submission count"
+    ),
 ) -> None:
     app.pretty_exceptions_enable = False
     set_settings(Settings(_env_file=env_path))
@@ -520,7 +533,7 @@ def joj3_check_env(
         penalty_config,
     )
     count_msg, count_failed = tea.pot.joj3_check_submission_count(
-        env, grading_repo_name, group_config, scoreboard_filename
+        env, grading_repo_name, group_config, scoreboard_filename, ignore_submitter
     )
     echo(
         json.dumps(
@@ -533,6 +546,16 @@ def joj3_check_env(
     logger.info("joj3-check-env done")
 
 
+@app.command("joj3-check-gitea-token")
+def joj3_check_gitea_token(
+    env_path: str = Argument("", help="path to .env file")
+) -> None:
+    app.pretty_exceptions_enable = False
+    set_settings(Settings(_env_file=env_path))
+    set_logger(settings.stderr_log_level)
+    tea.pot.gitea.organization_api.org_list_repos(settings.gitea_org_name)
+
+
 if __name__ == "__main__":
     try:
         app()

joint_teapot/config.py

@@ -40,7 +40,7 @@ class Settings(BaseSettings):
     joj_sid: str = ""
 
     # joj3
-    joj3_lock_file_path: str = ".git/teapot.lock"
+    joj3_lock_file_path: str = ".git/teapot-joj3-all-env.lock"
     joj3_lock_file_timeout: int = 30
 
     # moss

joint_teapot/teapot.py

@@ -231,6 +231,9 @@ class Teapot:
             self.canvas.students, invite_teaching_teams
         )
 
+    def invite_students_to_mattermost_team(self) -> None:
+        return self.mattermost.invite_students_to_team(self.canvas.students)
+
     def joj3_post_issue(
         self,
         env: joj3.Env,
@@ -266,7 +269,6 @@ class Teapot:
         ):
             if issue.title.startswith(title_prefix):
                 joj3_issue = issue
-                logger.info(f"found joj3 issue: #{joj3_issue.number}")
                 break
         else:
             new_issue = True
@@ -293,9 +295,7 @@ class Teapot:
                 submitter_repo_name,
                 body={"title": title, "body": comment, "labels": [label_id]},
             )
-            logger.info(f"created joj3 issue: #{joj3_issue.number}")
         gitea_issue_url = joj3_issue.html_url
-        logger.info(f"gitea issue url: {gitea_issue_url}")
         if not new_issue:
             self.gitea.issue_api.issue_edit_issue(
                 self.gitea.org_name,
@@ -359,7 +359,9 @@ class Teapot:
         grading_repo_name: str,
         group_config: str,
         scoreboard_filename: str,
+        ignore_submitter: bool,
     ) -> Tuple[str, bool]:
+        submitter = env.github_actor
         submitter_repo_name = env.github_repository.split("/")[-1]
         repo: Repo = self.git.get_repo(grading_repo_name)
         now = datetime.now(timezone.utc)
@@ -386,11 +388,13 @@ class Teapot:
             time_windows.append(since)
             valid_items.append((name, max_count, time_period, since))
         logger.info(f"valid items: {valid_items}, time windows: {time_windows}")
-        all_commits = []
+        matched_commits = []
+        all_commits_length = 0
         if time_windows:
             earliest_since = min(time_windows).strftime("%Y-%m-%dT%H:%M:%S")
             commits = repo.iter_commits(paths=scoreboard_filename, since=earliest_since)
             for commit in commits:
+                all_commits_length += 1
                 lines = commit.message.strip().splitlines()
                 if not lines:
                     continue
@@ -400,25 +404,28 @@ class Teapot:
                 d = match.groupdict()
                 if (
                     env.joj3_conf_name != d["exercise_name"]
-                    or env.github_actor != d["submitter"]
                     or submitter_repo_name != d["submitter_repo_name"]
                 ):
                     continue
+                if not ignore_submitter and submitter != d["submitter"]:
+                    continue
                 groups_line = next((l for l in lines if l.startswith("groups: ")), None)
                 commit_groups = (
                     groups_line[len("groups: ") :].split(",") if groups_line else []
                 )
-                all_commits.append(
+                matched_commits.append(
                     {
                         "time": commit.committed_datetime,
                         "groups": [g.strip() for g in commit_groups],
                     }
                 )
-        logger.info(f"all commits length: {len(all_commits)}")
+        logger.info(
+            f"matched commits length: {len(matched_commits)}, all commits length: {all_commits_length}"
+        )
         for name, max_count, time_period, since in valid_items:
             submit_count = 0
             time_limit = now - timedelta(hours=time_period)
-            for commit in all_commits:
+            for commit in matched_commits:
                 if commit["time"] < time_limit:
                     continue
                 if name:
@@ -428,18 +435,19 @@ class Teapot:
                         continue
                 submit_count += 1
             logger.info(
-                f"submitter {env.github_actor} is submitting for the {submit_count + 1} time, "
+                f"submitter {submitter} is submitting for the {submit_count + 1} time, "
                 f"{min(0, max_count - submit_count - 1)} time(s) remaining, "
                 f"group={name}, "
                 f"time period={time_period} hour(s), "
                 f"max count={max_count}, submit count={submit_count}"
             )
             use_group = True
+            keyword_placeholder = ""
             if name:
-                comment += f"keyword `{name}` "
+                keyword_placeholder = f", with keyword `{name}`"
                 use_group = name.lower() in env.joj3_groups.lower()
             comment += (
-                f"In last {time_period} hour(s): "
+                f"In last {time_period} hour(s) {keyword_placeholder}: "
                 f"submit count {submit_count}, "
                 f"max count {max_count}"
             )

joint_teapot/utils/joj3.py

@@ -7,6 +7,7 @@ from typing import Any, Dict, List, Optional, Tuple
 
 from pydantic_settings import BaseSettings
 
+from joint_teapot.config import settings
 from joint_teapot.utils.logger import logger
 
 
@@ -41,6 +42,7 @@ def generate_scoreboard(
     scoreboard_file_path: str,
     exercise_name: str,
     submitter_repo_name: str,
+    exercise_total_score: int,
 ) -> None:
     if not scoreboard_file_path.endswith(".csv"):
         logger.error(
@@ -99,11 +101,6 @@ def generate_scoreboard(
         for row in data:
             row.insert(index, "")
 
-    exercise_total_score = 0
-    for stage in stages:
-        for result in stage["results"]:
-            exercise_total_score += result["score"]
-    exercise_total_score = exercise_total_score
     submitter_row[columns.index(exercise_name)] = str(exercise_total_score)
 
     total = 0
@@ -146,6 +143,18 @@ def get_failed_table_from_file(table_file_path: str) -> List[List[str]]:
     return data
 
 
+def get_failed_stage_from_file(score_file_path: str) -> str:
+    with open(score_file_path) as json_file:
+        stages: List[Dict[str, Any]] = json.load(json_file)
+
+    failed_stage = ""
+    for stage in stages:
+        if stage["force_quit"] == True:
+            failed_stage = stage["name"]
+            break
+    return failed_stage
+
+
 def update_failed_table_from_score_file(
     data: List[List[str]],
     score_file_path: str,
@@ -153,31 +162,23 @@ def update_failed_table_from_score_file(
     repo_link: str,
     action_link: str,
 ) -> None:
-    # get info from score file
-    with open(score_file_path) as json_file:
-        stages: List[Dict[str, Any]] = json.load(json_file)
-
-    failed_name = ""
-    for stage in stages:
-        if stage["force_quit"] == True:
-            failed_name = stage["name"]
-            break
+    failed_stage = get_failed_stage_from_file(score_file_path)
 
     # append to failed table
     now = datetime.now().strftime("%Y-%m-%d %H:%M")
     repo = f"[{repo_name}]({repo_link})"
-    failure = f"[{failed_name}]({action_link})"
+    failure = f"[{failed_stage}]({action_link})"
     row_found = False
     for i, row in enumerate(data[:]):
         if row[1] == repo:
             row_found = True
-            if failed_name == "":
+            if failed_stage == "":
                 data.remove(row)
             else:
                 data[i][0] = now
                 data[i][2] = failure
             break
-    if not row_found and failed_name != "":
+    if not row_found and failed_stage != "":
         data.append([now, repo, failure])
 
 
@@ -243,7 +244,7 @@ def generate_title_and_comment(
         f"Generated at {now} from [Gitea Actions #{run_number}]({action_link}), "
         f"commit {commit_hash}, "
         f"triggered by @{submitter}, "
-        f"run ID `{run_id}`.\n"
+        f"run ID [`{run_id}`](https://focs.ji.sjtu.edu.cn/joj-mon/d/{settings.gitea_org_name}?var-Filters=RunID%7C%3D%7C{run_id}).\n"
         "Powered by [JOJ3](https://github.com/joint-online-judge/JOJ3) and "
         "[Joint-Teapot](https://github.com/BoYanZh/Joint-Teapot) with ❤️.\n"
     )

joint_teapot/utils/logger.py

@@ -38,10 +38,7 @@ def set_logger(
 ) -> None:
     logging.basicConfig(handlers=[InterceptHandler()], level=0, force=True)
     logger.remove()
-    logger.add(
-        stderr,
-        level=stderr_log_level,
-    )
+    logger.add(stderr, level=stderr_log_level, colorize=stderr.isatty())
     logger.add(settings.log_file_path, level="DEBUG")
 
 

joint_teapot/workers/canvas.py

@@ -39,8 +39,19 @@ class Canvas:
             student.name = (
                 re.sub(r"[^\x00-\x7F]+", "", student.name).strip().title()
             )  # We only care english name
-            student.sis_id = student.login_id
-            student.login_id = student.email.split("@")[0]
+            # Some users (like system users, announcers) might not have login_id
+            if hasattr(student, 'login_id') and student.login_id:
+                student.sis_id = student.login_id
+                student.login_id = student.email.split("@")[0]
+            else:
+                # For users without login_id, use email prefix as both sis_id and login_id
+                if hasattr(student, 'email') and student.email:
+                    student.login_id = student.email.split("@")[0]
+                    student.sis_id = student.login_id
+                else:
+                    # Fallback for users without email
+                    student.login_id = f"user_{student.id}"
+                    student.sis_id = student.login_id
             return student
 
         self.students = [

joint_teapot/workers/git.py

@@ -90,28 +90,23 @@ class Git:
         retry_interval = 2
         while retry_interval and auto_retry:
             try:
-                current_branch = ""
-                if repo.head.is_detached:
-                    current_branch = repo.head.commit.hexsha
-                else:
-                    current_branch = repo.active_branch.name
                 if clean_git_lock:
-                    lock_files = [
-                        "index.lock",
-                        "HEAD.lock",
-                        "fetch-pack.lock",
-                        "logs/HEAD.lock",
-                        "packed-refs.lock",
-                        "config.lock",
-                        f"refs/heads/{current_branch}.lock",
-                        f"refs/remotes/origin/{current_branch}.lock",
-                        f"refs/heads/{checkout_dest}.lock",
-                        f"refs/remotes/origin/{checkout_dest}.lock",
-                    ]
-                    for lock_file in lock_files:
-                        lock_path = os.path.join(repo_dir, ".git", lock_file)
-                        if os.path.exists(lock_path):
-                            os.remove(lock_path)
+                    locks_removed_count = 0
+                    for root, _, files in os.walk(os.path.join(repo_dir, ".git")):
+                        for filename in files:
+                            if filename.endswith(".lock"):
+                                lock_file_path = os.path.join(root, filename)
+                                if (
+                                    os.path.relpath(lock_file_path, repo_dir)
+                                    == settings.joj3_lock_file_path
+                                ):
+                                    continue
+                                try:
+                                    os.remove(lock_file_path)
+                                    locks_removed_count += 1
+                                except OSError as e:
+                                    logger.warning(f"error removing lock file: {e}")
+                    logger.info(f"removed {locks_removed_count} lock files")
                 repo.git.fetch("--tags", "--all", "-f")
                 repo.git.reset("--hard", reset_target)
                 repo.git.clean("-d", "-f", "-x")
@@ -148,9 +143,7 @@ class Git:
             try:
                 repo.index.add(file)
             except OSError:
-                logger.warning(
-                    f'File path "{file}" does not exist. Skipping this file.'
-                )
+                logger.warning(f'file path "{file}" does not exist, skipped')
                 continue
         if repo.is_dirty(untracked_files=True) or repo.index.diff(None):
             repo.index.commit(commit_message)

joint_teapot/workers/gitea.py

@@ -494,6 +494,7 @@ class Gitea:
             self.repository_api.user_current_delete_subscription(
                 self.org_name, repo.name
             )
+            logger.info(f"Unwatched {repo.name}")
 
     def get_all_teams(self) -> Dict[str, List[str]]:
         res: Dict[str, List[str]] = {}

joint_teapot/workers/mattermost.py

@@ -7,6 +7,7 @@ from mattermostdriver import Driver
 from joint_teapot.config import settings
 from joint_teapot.utils.logger import logger
 from joint_teapot.workers.gitea import Gitea
+from joint_teapot.workers.canvas import User
 
 
 class Mattermost:
@@ -229,14 +230,21 @@ class Mattermost:
                 logger.warning(f"Error when creating outgoing webhook at Gitea: {e}")
 
     # unused since we can give students invitation links instead
-    def invite_students_to_team(self, students: List[str]) -> None:
+    def invite_students_to_team(self, students: List[User]) -> None:
         for student in students:
+            username = student.login_id
+
+            if not username:
+                student_id = getattr(student, 'id', 'unknown')
+                logger.warning(f"Student {student_id} has no login_id, skipping")
+                continue
+
             try:
-                mmuser = self.endpoint.users.get_user_by_username(student)
+                mmuser = self.endpoint.users.get_user_by_username(username)
             except Exception:
-                logger.warning(f"User {student} is not found on the Mattermost server")
+                logger.warning(f"User {username} is not found on the Mattermost server")
                 continue
             self.endpoint.teams.add_user_to_team(
                 self.team["id"], {"user_id": mmuser["id"], "team_id": self.team["id"]}
             )
-            logger.info(f"Added user {student} to team {self.team['name']}")
+            logger.info(f"Added user {username} to team {self.team['name']}")