refactor(services): un seul endroit pour annoncer un match planifie

Seconde moitie d ARCH-003.

Les memes vingt lignes vivaient dans trois routes -- matches.create_match,
matches.edit_match et team_matches.create_match : formater la date et
l heure, puis parcourir deux listes paralleles pour apparier un joueur avec
la ligne de participation qu une reaction Discord doit pouvoir retrouver.

Trois copies, donc trois occasions de diverger. Elles avaient deja diverge :

  create_match  lisait les heures des variables qu il venait d analyser, et
                affichait 'TBD' des qu une des deux manquait ;
  edit_match    les relisait depuis la ligne enregistree et substituait
                l heure de debut a une heure de fin absente.

Un match avec une heure de debut et pas de fin annoncait donc une heure
dans une route et 'TBD' dans l autre, pour la meme donnee.
app/services/scheduling.py retient la regle la plus soigneuse des deux.

zip_participants() isole l appariement par index, qui n est correct que
tant que les deux listes sont construites en phase -- desormais un seul
endroit a relire au lieu de trois, et une liste plus courte donne None,
converti en reference vers le match lui-meme.

matches.py 711 -> 687 lignes, team_matches.py 347 -> 337.

9 tests sur le service seul, sans base ni requete.

Co-Authored-By: Claude Opus 5 <[email protected]>

app/routes/matches.py

@@ -25,7 +25,7 @@ from app.models import (
     PersonalNote,
 )
 from datetime import datetime, timedelta
-from app.discord_bot import send_schedule_notification
+from app.services.scheduling import notify_participants, zip_participants
 
 matches_bp = Blueprint('matches', __name__, url_prefix='/matches')
 
@@ -375,25 +375,14 @@ def create_match(tryout_id):
 
         db.session.commit()
 
-        # Discord notifications
+        notify_participants(
-        event_date_str = date_obj.strftime('%Y-%m-%d')
+            title=match.title,
-        event_time_str = (
+            date=date_obj,
-            f"{start_time.strftime('%I:%M %p')} - {end_time.strftime('%I:%M %p')}"
+            start_time=start_time,
-            if start_time and end_time
+            end_time=end_time,
-            else 'TBD'
+            participants=zip_participants(notified_player_ids, notified_participant_ids),
+            fallback_id=match.id,
         )
-        for i, player_id in enumerate(notified_player_ids):
-            reference_id = (
-                notified_participant_ids[i] if i < len(notified_participant_ids) else match.id
-            )
-            send_schedule_notification(
-                user_id=player_id,
-                event_type='match',
-                event_title=match.title,
-                event_date=event_date_str,
-                event_time=event_time_str,
-                reference_id=reference_id,
-            )
 
         flash(_('Match scheduled successfully!'), 'success')
         return redirect(url_for('tryouts.view_tryout', tryout_id=tryout_id))
@@ -553,27 +542,14 @@ def edit_match(match_id):
 
         db.session.commit()
 
-        # Discord notifications
+        notify_participants(
-        end_time_val = match.end_time or (match.start_time if match.start_time else None)
+            title=match.title,
-        if match.start_time and end_time_val:
+            date=match.date,
-            event_time_str = (
+            start_time=match.start_time,
-                f"{match.start_time.strftime('%I:%M %p')} - {end_time_val.strftime('%I:%M %p')}"
+            end_time=match.end_time,
-            )
+            participants=zip_participants(notified_player_ids, notified_participant_ids),
-        else:
+            fallback_id=match.id,
-            event_time_str = 'TBD'
+        )
-        event_date_str = match.date.strftime('%Y-%m-%d')
-        for i, player_id in enumerate(notified_player_ids):
-            reference_id = (
-                notified_participant_ids[i] if i < len(notified_participant_ids) else match.id
-            )
-            send_schedule_notification(
-                user_id=player_id,
-                event_type='match',
-                event_title=match.title,
-                event_date=event_date_str,
-                event_time=event_time_str,
-                reference_id=reference_id,
-            )
 
         flash(_('Match updated successfully!'), 'success')
         return redirect(url_for('tryouts.view_tryout', tryout_id=tryout.id))

app/routes/team_matches.py

@@ -19,7 +19,7 @@ from app.models import (
 )
 from app.permissions import can_manage_org_team, coach_org_teams, visible_org_teams
 from datetime import datetime, timedelta
-from app.discord_bot import send_schedule_notification
+from app.services.scheduling import notify_participants, zip_participants
 
 team_matches_bp = Blueprint('team_matches', __name__, url_prefix='/team-matches')
 
@@ -204,27 +204,17 @@ def create_match(team_id):
 
         db.session.commit()
 
-        # Discord notifications
+        notify_participants(
-        event_date_str = date_obj.strftime('%Y-%m-%d')
+            title=team_match.title,
-        event_time_str = (
+            date=date_obj,
-            f"{start_time.strftime('%I:%M %p')} - {end_time.strftime('%I:%M %p')}"
+            start_time=start_time,
-            if start_time and end_time
+            end_time=end_time,
-            else 'TBD'
+            participants=zip_participants(
+                [tp.player_id for tp in team_players], notified_participant_ids
+            ),
+            fallback_id=team_match.id,
         )
 
-        for i, tp in enumerate(team_players):
-            reference_id = (
-                notified_participant_ids[i] if i < len(notified_participant_ids) else team_match.id
-            )
-            send_schedule_notification(
-                user_id=tp.player_id,
-                event_type='match',
-                event_title=team_match.title,
-                event_date=event_date_str,
-                event_time=event_time_str,
-                reference_id=reference_id,
-            )
-
         flash(_('Team match "%(title)s" scheduled successfully!', title=title), 'success')
         return redirect(url_for('team_matches.list_matches'))
 

app/services/scheduling.py

@@ -0,0 +1,96 @@
+"""Announcing a scheduled match to the people who have to be there.
+
+The same twenty lines appeared three times — matches.create_match,
+matches.edit_match and team_matches.create_match — each formatting the date
+and time itself, then walking two parallel lists in lockstep to pair a
+player with the participant row that a Discord reaction has to find again
+(ARCH-003).
+
+Three copies meant three chances to drift, and they had:
+create_match read the times from local variables it had just parsed, while
+edit_match re-derived them from the saved row and substituted the start
+time for a missing end time. The rule kept here is the more careful of the
+two.
+"""
+
+import logging
+
+from app.discord_bot import send_schedule_notification
+
+logger = logging.getLogger(__name__)
+
+#: What the bot shows when a match has no usable time.
+TIME_UNKNOWN = 'TBD'
+
+
+def format_event_time(start_time, end_time):
+    """Render a match's time range the way the Discord message expects.
+
+    Args:
+        start_time: A time, or None.
+        end_time: A time, or None. Falls back to start_time, so a match with
+            only a start still announces something useful.
+
+    Returns:
+        str: 'HH:MM AM - HH:MM PM', or TIME_UNKNOWN.
+    """
+    if not start_time:
+        return TIME_UNKNOWN
+    finish = end_time or start_time
+    return f'{start_time.strftime("%I:%M %p")} - {finish.strftime("%I:%M %p")}'
+
+
+def zip_participants(player_ids, participant_ids):
+    """Pair each player with the participant row that was created for them.
+
+    The routes build these as two parallel lists, appended in step. Pairing
+    them by index is what the original code did, and it is only correct as
+    long as they stay in step — hence one place to look at rather than
+    three. A short participant list yields None, which
+    :func:`notify_participants` turns into the fallback reference.
+
+    Args:
+        player_ids: Player primary keys, in creation order.
+        participant_ids: Participant row ids, in the same order.
+
+    Yields:
+        tuple[int, int | None]: (player_id, participant_id).
+    """
+    for index, player_id in enumerate(player_ids):
+        yield player_id, participant_ids[index] if index < len(participant_ids) else None
+
+
+def notify_participants(*, title, date, start_time, end_time, participants, fallback_id):
+    """Tell each participant that a match has been scheduled or changed.
+
+    Args:
+        title: Match title, shown in the message.
+        date: The match date.
+        start_time: Start time, or None.
+        end_time: End time, or None.
+        participants: Iterable of (player_id, participant_id) pairs.
+            participant_id is what a Discord reaction resolves back to, so
+            attendance lands on the right row.
+        fallback_id: Reference to use when a participant row has no id —
+            the match's own, which the bot can still act on.
+
+    Returns:
+        int: How many notifications were handed to the bot.
+    """
+    event_date = date.strftime('%Y-%m-%d')
+    event_time = format_event_time(start_time, end_time)
+
+    sent = 0
+    for player_id, participant_id in participants:
+        if not player_id:
+            continue
+        send_schedule_notification(
+            user_id=player_id,
+            event_type='match',
+            event_title=title,
+            event_date=event_date,
+            event_time=event_time,
+            reference_id=participant_id or fallback_id,
+        )
+        sent += 1
+    return sent

tests/test_scheduling_service.py

@@ -0,0 +1,100 @@
+"""Announcing a scheduled match — ARCH-003.
+
+The same twenty lines lived in three routes. They had already drifted:
+create_match formatted the time from the values it had just parsed and
+printed 'TBD' whenever either end was missing, while edit_match re-derived
+them from the saved row and substituted the start time for a missing end.
+A match with a start and no end therefore announced a time in one route and
+'TBD' in the other, for the same data.
+
+app/services/scheduling.py keeps the more careful of the two rules.
+"""
+
+from datetime import date, time
+
+import pytest
+
+from app.services import scheduling
+from app.services.scheduling import TIME_UNKNOWN, format_event_time, zip_participants
+
+
+class TestTimeFormatting:
+    def test_a_full_range_is_rendered(self):
+        assert format_event_time(time(18, 0), time(20, 30)) == '06:00 PM - 08:30 PM'
+
+    def test_a_missing_end_falls_back_to_the_start(self):
+        """This is the behaviour create_match did not have: it printed TBD
+        and lost the start time it knew perfectly well."""
+        assert format_event_time(time(18, 0), None) == '06:00 PM - 06:00 PM'
+
+    def test_no_start_means_nothing_useful_to_say(self):
+        assert format_event_time(None, time(20, 0)) == TIME_UNKNOWN
+        assert format_event_time(None, None) == TIME_UNKNOWN
+
+
+class TestParticipantPairing:
+    def test_players_pair_with_their_participant_rows(self):
+        assert list(zip_participants([7, 8, 9], [70, 80, 90])) == [(7, 70), (8, 80), (9, 90)]
+
+    def test_a_short_participant_list_yields_none(self):
+        """The routes append to two lists in step; if they ever fall out of
+        step the notification still goes out, against the match itself."""
+        assert list(zip_participants([7, 8], [70])) == [(7, 70), (8, None)]
+
+    def test_no_players_means_no_pairs(self):
+        assert list(zip_participants([], [])) == []
+
+
+class TestNotifying:
+    @pytest.fixture
+    def sent(self, monkeypatch):
+        calls = []
+        monkeypatch.setattr(
+            scheduling, 'send_schedule_notification', lambda **kwargs: calls.append(kwargs)
+        )
+        return calls
+
+    def test_each_participant_is_told(self, sent):
+        count = scheduling.notify_participants(
+            title='Scrim',
+            date=date(2030, 4, 1),
+            start_time=time(19, 0),
+            end_time=time(21, 0),
+            participants=[(1, 11), (2, 22)],
+            fallback_id=99,
+        )
+
+        assert count == 2
+        assert [call['user_id'] for call in sent] == [1, 2]
+        assert [call['reference_id'] for call in sent] == [11, 22]
+        assert {call['event_date'] for call in sent} == {'2030-04-01'}
+        assert {call['event_time'] for call in sent} == {'07:00 PM - 09:00 PM'}
+
+    def test_a_participant_without_a_row_falls_back_to_the_match(self, sent):
+        """A Discord reaction resolves the reference back to a record; the
+        match id is the one thing that always exists."""
+        scheduling.notify_participants(
+            title='Scrim',
+            date=date(2030, 4, 1),
+            start_time=None,
+            end_time=None,
+            participants=[(1, None)],
+            fallback_id=99,
+        )
+
+        assert sent[0]['reference_id'] == 99
+        assert sent[0]['event_time'] == TIME_UNKNOWN
+
+    def test_a_missing_player_is_skipped(self, sent):
+        assert (
+            scheduling.notify_participants(
+                title='Scrim',
+                date=date(2030, 4, 1),
+                start_time=None,
+                end_time=None,
+                participants=[(None, 11), (2, 22)],
+                fallback_id=99,
+            )
+            == 1
+        )
+        assert [call['user_id'] for call in sent] == [2]