From dbef3bda5238f3fdbcaf321f5a2116a0f8aba65d Mon Sep 17 00:00:00 2001 From: Adam Moussa <166072409+amoussa1229@users.noreply.github.com> Date: Thu, 10 Sep 2026 19:58:59 +0000 Subject: [PATCH] chore(pay): remove weekly-post payroll email code, env, and failure alarm (#249) PR #248 disabled the SES send by blanking PAYROLL_RECIPIENTS and dropping the ses:SendEmail grant. This removes the now-dead path: _send_pay_email and _build_pay_email_html, the SES_SENDER and PAYROLL_RECIPIENTS env, and the PayrollEmailFailure metric filter and alarm that only fired on that path. Slack schedule post, pay-summary DM, and checkcomponents enqueue unchanged. --- src/weekly-post/app.py | 99 +----------------------------- template.yaml | 40 ------------ tests/weekly_post/test_handler.py | 49 +++------------ tests/weekly_post/test_pay_math.py | 61 ------------------ 4 files changed, 9 insertions(+), 240 deletions(-) diff --git a/src/weekly-post/app.py b/src/weekly-post/app.py index cb9dd2d..43d6301 100644 --- a/src/weekly-post/app.py +++ b/src/weekly-post/app.py @@ -1,5 +1,5 @@ """Lambda handler — posts the weekly on-call schedule and previous week's pay summary -to Slack every Monday at 7am ET, and emails the pay summary to payroll.""" +to Slack every Monday at 7am ET, and enqueues after-hours dollars for Flex.""" import json import logging @@ -164,92 +164,6 @@ def _calculate_weekly_pay(schedule: ShiftSchedule, week_start: datetime) -> dict } -def _build_pay_email_html(week_label: str, pay_record: dict) -> str: - """Build an HTML email body for the weekly pay summary. - - Holiday shifts are rendered distinctly: a per-shift breakdown section - highlights holiday rows (shaded, showing the multiplier and label) and the - totals table flags any employee who worked a holiday during the week. - """ - breakdown = pay_record.get("breakdown", []) - totals = pay_record.get("totals", {}) - - holiday_names = {line["name"] for line in breakdown if line.get("is_holiday")} - - totals_rows = "" - for name, info in sorted(totals.items()): - holiday_tag = ( - ' ★ holiday' - if name in holiday_names - else "" - ) - totals_rows += ( - f"{name}{holiday_tag}" - f"${info.get('rate', 0):.2f}${info['total']:.2f}\n" - ) - - breakdown_rows = "" - for line in breakdown: - if line.get("is_holiday"): - mult = line.get("multiplier", Decimal("1")) - label = line.get("holiday_label", "") or "Holiday" - base = line.get("base_rate", line["rate"]) - row_style = ' style="background:#fff8e1;"' - detail = f"{label} — ${base:.2f} × {mult:.2f}x" - else: - row_style = "" - detail = f"${line['rate']:.2f}" - breakdown_rows += ( - f"{line['date_label']}" - f"{line['day']}" - f"{line['name']}" - f"{detail}" - f"${line['amount']:.2f}\n" - ) - - return f""" - -

Bonus Pay Summary — {week_label}

- - - -{totals_rows}
NameRateTotal
- -

Shift Breakdown

- - -{breakdown_rows}
DateDayNameDetailAmount
-

Holiday shifts are shaded and paid at the listed multiplier.

- -

This is an automated report from Sea Haven Industries.

- -""" - - -def _send_pay_email(week_label: str, pay_record: dict) -> None: - """Send the weekly pay summary email via SES.""" - sender = os.environ.get("SES_SENDER", "noreply@seahaven.com") - recipients = os.environ.get("PAYROLL_RECIPIENTS", "").split(",") - recipients = [r.strip() for r in recipients if r.strip()] - - if not recipients: - logger.warning("No PAYROLL_RECIPIENTS configured — skipping email") - return - - ses = boto3.client("ses") - html_body = _build_pay_email_html(week_label, pay_record) - - ses.send_email( - Source=sender, - Destination={"ToAddresses": recipients}, - Message={ - "Subject": {"Data": f"Bonus Pay Summary — {week_label}"}, - "Body": {"Html": {"Data": html_body}}, - }, - ) - logger.info("Sent pay email to %s", recipients) - - def build_checkcomponents_payload(pay_record: dict) -> dict: """Sibling dollar lines only. No Flex OAuth, no payPeriodId invention.""" week_start = datetime.strptime(pay_record["week_start"], "%Y-%m-%d").date() @@ -343,17 +257,6 @@ def handler(event, context): else: logger.warning("PAY_REPORT_USER not set — skipping Slack pay summary") - # Email pay summary to payroll. Isolated so a delivery failure (e.g. - # an SES permission/identity issue) can never abort the rest of the - # handler — the Slack schedule post below must still go out. - try: - _send_pay_email(week_label, pay_record) - except Exception: - logger.exception( - "Failed to send pay summary email to payroll for week of %s", - week_key, - ) - # --- Two-week schedule (always starts on Monday of this week) --- this_monday = now - timedelta(days=now.weekday()) week_start = this_monday.strftime("%Y-%m-%d") diff --git a/template.yaml b/template.yaml index 9799205..7616ba3 100644 --- a/template.yaml +++ b/template.yaml @@ -165,8 +165,6 @@ Resources: SHIFT_TABLE: !Ref ShiftTable SLACK_BOT_TOKEN_SECRET: afterhours-shift-manager/slack-bot-token SHIFT_CHANNEL: !Ref ShiftChannel - SES_SENDER: noreply@seahaven.com - PAYROLL_RECIPIENTS: "" PAY_REPORT_USER: U0A3SC48T47 TZ: !Ref Timezone CHECKCOMPONENTS_QUEUE_URL: !Ref CheckcomponentsQueueUrl @@ -600,44 +598,6 @@ Resources: AlarmActions: - !Sub "arn:aws:sns:${AWS::Region}:${AWS::AccountId}:site-alerts" - # --- Payroll email delivery failure (log-metric alarm) --- - # The pay-summary email send is wrapped in try/except so a delivery failure - # never aborts the schedule post — which means it does NOT surface on the - # Lambda Errors metric above (the handler exits "success"). This filter turns - # the "Failed to send pay summary" log line into a metric so a silent payroll - # delivery failure still pages site-alerts. DefaultValue 0 keeps the metric - # reporting on every run so the alarm sits in OK, not INSUFFICIENT_DATA. - PayrollEmailFailureMetricFilter: - Type: AWS::Logs::MetricFilter - Properties: - # !Ref the stack-managed log group (not a literal name) so CloudFormation - # orders this filter after the group exists. - LogGroupName: !Ref WeeklyPostLogGroup - FilterPattern: '"Failed to send pay summary"' - MetricTransformations: - - MetricName: PayrollEmailFailures - MetricNamespace: AfterHours/WeeklyPost - MetricValue: "1" - # DefaultValue 0 means non-matching runs emit 0, so the metric stays - # populated and the alarm rests in OK rather than INSUFFICIENT_DATA. - DefaultValue: 0 - - PayrollEmailFailureAlarm: - Type: AWS::CloudWatch::Alarm - Properties: - AlarmName: PayrollEmailFailure-afterhours-weekly-post - AlarmDescription: "Weekly-post failed to email the pay summary to payroll (SES send error)" - Namespace: AfterHours/WeeklyPost - MetricName: PayrollEmailFailures - Statistic: Sum - Period: 300 - EvaluationPeriods: 1 - Threshold: 1 - ComparisonOperator: GreaterThanOrEqualToThreshold - TreatMissingData: notBreaching - AlarmActions: - - !Sub "arn:aws:sns:${AWS::Region}:${AWS::AccountId}:site-alerts" - # --- Lambda Duration alarms (Maximum, ms; 2-of-3 evaluation) --- # Thresholds set to ~80% of each function's timeout, with 2-of-3 evaluation. # Timeouts: slack-bot/weekly-post/release-notifier/roster-api = 30s (global diff --git a/tests/weekly_post/test_handler.py b/tests/weekly_post/test_handler.py index f718ff4..526a5aa 100644 --- a/tests/weekly_post/test_handler.py +++ b/tests/weekly_post/test_handler.py @@ -28,7 +28,6 @@ def env(monkeypatch): "SLACK_BOT_TOKEN_SECRET", "afterhours-shift-manager/slack-bot-token" ) monkeypatch.setenv("PAY_REPORT_USER", "U_BOSS") - monkeypatch.delenv("PAYROLL_RECIPIENTS", raising=False) # skip SES email monkeypatch.setenv( "CHECKCOMPONENTS_QUEUE_URL", "https://sqs.us-east-1.amazonaws.com/011934824531/paychex-checkcomponents", @@ -89,27 +88,6 @@ def test_calculates_and_dms_pay(weeklypost_app, schedule, seed, slack, env, sqs) assert schedule.get_pay_record("2026-06-01")["checkcomponents_sent"] is True -@freeze_time(MON_0800) -def test_pay_email_failure_does_not_block_schedule_post( - weeklypost_app, schedule, seed, slack, env, monkeypatch, sqs -): - # Previous week has an assigned shift, so the pay/email path runs. - seed.config(shift_rate="50") - seed.weekly("Monday", "114", "Alice") - # SES delivery blows up (e.g. a permission/identity issue). - monkeypatch.setattr( - weeklypost_app, - "_send_pay_email", - MagicMock(side_effect=Exception("SES AccessDenied")), - ) - - result = weeklypost_app.handler({"force": True}, None) - - # The email failure is swallowed; the schedule post still goes out. - assert result["posted"] is True - assert schedule.get_schedule_post("C_TEST")["message_ts"] == "999.000" - - @freeze_time(MON_0800) def test_rolls_existing_post_forward(weeklypost_app, schedule, seed, slack, env, sqs): # An existing post is deleted + reposted so it lands at the bottom every @@ -256,22 +234,11 @@ def test_payload_skips_fallback_and_zero(weeklypost_app): assert "payPeriodId" not in payload -def test_send_pay_email_skips_when_recipients_empty(weeklypost_app, monkeypatch): - monkeypatch.setenv("PAYROLL_RECIPIENTS", "") - ses = MagicMock(name="ses") - - def client(svc, **kw): - if svc == "ses": - return ses - return MagicMock(name=svc) - - monkeypatch.setattr(weeklypost_app.boto3, "client", client) - weeklypost_app._send_pay_email("Jun 1 to Jun 7", {"totals": {}, "breakdown": []}) - ses.send_email.assert_not_called() - - -def test_weekly_post_payroll_recipients_empty_and_no_ses_grant(): - text = (Path(__file__).resolve().parents[2] / "template.yaml").read_text() - assert 'PAYROLL_RECIPIENTS: ""' in text - assert "PAYROLL_RECIPIENTS: payroll@seahaven.com" not in text - assert "ses:SendEmail" not in text +def test_weekly_post_has_no_payroll_email_path(): + root = Path(__file__).resolve().parents[2] + template = (root / "template.yaml").read_text() + for token in ("PAYROLL_RECIPIENTS", "SES_SENDER", "ses:", "PayrollEmailFailure"): + assert token not in template, token + source = (root / "src" / "weekly-post" / "app.py").read_text() + for token in ("_send_pay_email", "_build_pay_email_html", 'boto3.client("ses")'): + assert token not in source, token diff --git a/tests/weekly_post/test_pay_math.py b/tests/weekly_post/test_pay_math.py index 4dfc4fe..c6cb7c0 100644 --- a/tests/weekly_post/test_pay_math.py +++ b/tests/weekly_post/test_pay_math.py @@ -207,64 +207,3 @@ class TestHolidayPay: assert breakdown[0]["name"] == "Alice" assert breakdown[0].get("is_holiday") is None assert breakdown[0]["amount"] == Decimal("50") - - -class TestBuildPayEmailHtml: - def test_renders_totals_rows(self, weeklypost_app): - pay_record = { - "totals": { - "Alice": {"total": Decimal("100"), "rate": Decimal("50"), "shifts": 2} - } - } - html = weeklypost_app._build_pay_email_html("Jun 1 to Jun 7", pay_record) - assert "Jun 1 to Jun 7" in html - assert "Alice" in html - assert "$100.00" in html - - def test_holiday_rows_rendered_distinctly(self, weeklypost_app): - pay_record = { - "totals": { - "Alice": { - "total": Decimal("75"), - "rate": Decimal("50"), - "shifts": 1, - "holiday_shifts": 1, - }, - "Bob": { - "total": Decimal("50"), - "rate": Decimal("50"), - "shifts": 1, - "holiday_shifts": 0, - }, - }, - "breakdown": [ - { - "date_label": "Jun 3", - "day": "Wed (Day)", - "name": "Alice", - "rate": Decimal("75"), - "base_rate": Decimal("50"), - "amount": Decimal("75"), - "is_holiday": True, - "multiplier": Decimal("1.5"), - "holiday_label": "Founders Day", - }, - { - "date_label": "Jun 6", - "day": "Sat (Day)", - "name": "Bob", - "rate": Decimal("50"), - "base_rate": Decimal("50"), - "amount": Decimal("50"), - }, - ], - } - html = weeklypost_app._build_pay_email_html("Jun 1 to Jun 7", pay_record) - # Holiday label, multiplier, and shading appear for the holiday row. - assert "Founders Day" in html - assert "1.50x" in html - assert "background:#fff8e1" in html - # Holiday worker is flagged in the totals table; non-holiday isn't. - assert "holiday" in html - assert "$75.00" in html - assert "$50.00" in html