diff --git a/.env.example b/.env.example index ca990af..b1e9c71 100644 --- a/.env.example +++ b/.env.example @@ -17,3 +17,9 @@ MOCK_FLIGHTS_APP_KEY= # WhatsApp: your own number with country code, for example +441234567890 # Needs the optional extra: python -m pip install -e ".[whatsapp]" WHATSAPP_NUMBER= + +# Email: the SMTP server, the sender and an app password (not your normal password). +# Every customer in the "users" sheet gets the deal by email. +SMTP_ADDRESS=smtp.gmail.com +EMAIL_ADDRESS= +EMAIL_PASSWORD= diff --git a/docs/artifact-registry.md b/docs/artifact-registry.md index cf5e482..85bae39 100644 --- a/docs/artifact-registry.md +++ b/docs/artifact-registry.md @@ -16,7 +16,7 @@ document of a type. `Primary File` may contain a glob (e.g. | PP | Project Plan | docs/project-plan.md | 002 | | MIL | Milestone | docs/milestones/mil-*.md | 006 | | ADR | Architecture Decision Record | docs/adr/adr-*.md | 0002 | -| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 011 | +| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 012 | ## Languages diff --git a/docs/sqa/reviews/rc-011-code-mil-004.md b/docs/sqa/reviews/rc-011-code-mil-004.md new file mode 100644 index 0000000..1c94af7 --- /dev/null +++ b/docs/sqa/reviews/rc-011-code-mil-004.md @@ -0,0 +1,61 @@ +# SQA Review Record + +## Metadata +| Key | Value | +| --- | --- | +| ID | RC-011 | +| CrossReference | [MIL-004], [QC-PY-001], [ADR-0001] | + +## Version History +| Date | Status | Author | Reviewer | Change | Commit | +| --- | --- | --- | --- | --- | --- | +| 2026-10-24 | Accepted | Jens Tirsvad Nielsen | AI as stand-in for S01 | Initial version | [d94a68a] | + +--- + +## Artifact Under Review + +- Instance reviewed: [MIL-004] (the Python code and tests of its tasks 1 to 4: `src/flight_club/`, `tests/`) +- Checklist used: [QC-PY-001] +- Scope: full review of the code changed by this milestone +- Mode: autonomous, not independent (the reviewer is `AI as stand-in for S01`) + +## Checklist Results + +| # | Criterion | Status | Evidence/Notes | +| --- | --- | --- | --- | +| 1 | Packages, modules, functions, variables, classes and constants follow PEP 8 casing (`snake_case`, `PascalCase`, `UPPER_SNAKE`) | Pass | Names use snake_case, PascalCase and UPPER_SNAKE (`send_emails`, `ENV_SMTP_ADDRESS`, `SMTP_PORT`). | +| 2 | Names state purpose in the domain's language; no unexplained abbreviations, no single-letter names outside tiny scopes | Pass | Names state purpose (`send_emails`, `_build_email`, `EMAIL_SUBJECT`). | +| 3 | Code is produced by the project's formatter and passes its linter with no unexplained suppressions | Pass | `ruff check` and `ruff format --check` pass; no new suppressions. | +| 4 | Every function and method signature is type-annotated, including `-> None` | Pass | Every new function and test has annotations including `-> None`. | +| 5 | No bare `except:`, no swallowed exceptions; specific exceptions are raised and the cause is kept (`raise ... from`) | Pass | Only `smtplib.SMTPException` and `OSError` are caught, in one place; the error type is reported and the loop result is returned, nothing is swallowed silently. | +| 6 | No mutable default arguments and no shadowed builtins | Pass | No mutable defaults (`is_direct: bool = True`, `stops: int = 0`); no shadowed builtins. | +| 7 | Files, locks and connections are managed with context managers | Pass | The SMTP connection is opened with a `with` block, so it is always closed. | +| 8 | Public modules, classes and functions have docstrings that say what, not how | Pass | Public classes and functions carry Doxygen comments (what, not how). | +| 9 | Logging uses `logging`, not `print`; no secrets or personal data in log output | Pass | Output uses `print` as the program's console output, recorded in [ADR-0001]. The SMTP error text and the app password are never printed (test `never_raises_or_prints_the_login`). | +| 10 | Classes and operations trace to the Design Class Diagram they implement; deviations are recorded | Pass | No DCD exists; the assignment's classes are the design; deviation recorded in [ADR-0001]. | +| 11 | Tests exist for new behaviour, are named for the behaviour, and do not depend on order or the network | Pass | New behaviour is tested with a fake `smtplib.SMTP`: login and STARTTLS, one mail per address, UTF-8 body, missing settings, SMTP failure, both message texts, GBP and no pound sign, customers read once, WhatsApp still sent when customers cannot be read. No mail is sent; no `.env`. | +| 12 | Type checker runs in strict mode without errors; `Any` is justified in a comment | Pass | `mypy --strict` passes on `src` and `tests`; the only `Any` is the JSON response shape. | +| 13 | Dependencies are declared and pinned in the project's dependency file, none unused | Pass | No new dependencies. | +| 14 | Each piece of knowledge (a business rule, constant, format, validation or query) is defined in one place; code is merged only where it expresses the same knowledge, not where it merely looks alike | Pass | The message text is built in one place (`build_message`) and reused for WhatsApp and email; env names, port and subject are constants. | +| 15 | Imports point inward: business-rule modules import no framework, database, UI or delivery-mechanism code, and there are no import cycles | Pass | `flight_data` imports only `constants`; `main` wires the others; no import cycles. | +| 16 | SOLID holds: a class or module has one reason to change; new behaviour is added by extension, not by editing a type test in several places; subclasses honour the contract of their base; protocols are narrow; high-level code depends on protocols or abstract types and receives its implementations | Pass | `send_emails` is added next to `send_whatsapp` without changing it; the SMTP class is the standard one and is replaced in tests by monkeypatching. | +| 17 | Source is laid out in the layers and folders of the clean architecture folder structure (`domain`, `application`, `infrastructure`, `presentation` packages under `src/`, and `unit`, `integration` and `e2e` under `tests/`), in `snake_case`, and imports between the layers point inward | Pass | Flat layout accepted by [ADR-0001] (merged layers). | +| 18 | File and folder names are English, contain no space and no character outside letters, digits, `-`, `_` and `.`, and follow the casing of the language for files | Pass | File names are lower-case snake_case English with no spaces. | + +## Overall Verdict + +Go — all mandatory criteria pass; deviations are recorded in ADR-0001. + +## Action Items + +| Action | Owner | Due | +| --- | --- | --- | +| None | - | - | + +--- + +[MIL-004]: ../../milestones/mil-004-email-notifications-and-main.md +[QC-PY-001]: ../../../framework/qc/code/qc-programming-python.md +[ADR-0001]: ../../adr/adr-0001-flat-module-layout-and-console-output.md +[d94a68a]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/040-personal-flight-club/commit/d94a68ac10edbcbd44b59f8f1f30c5aa14a9edb8 diff --git a/src/flight_club/constants.py b/src/flight_club/constants.py index cd1a58c..a86ae3a 100644 --- a/src/flight_club/constants.py +++ b/src/flight_club/constants.py @@ -78,6 +78,21 @@ ENV_MOCK_FLIGHTS_URL = "MOCK_FLIGHTS_URL" ## @brief Environment variable: WhatsApp recipient number with country code. ENV_WHATSAPP_NUMBER = "WHATSAPP_NUMBER" +## @brief Environment variable: address of the SMTP server that sends the emails. +ENV_SMTP_ADDRESS = "SMTP_ADDRESS" + +## @brief Environment variable: the sender's email address, also the SMTP login name. +ENV_EMAIL_ADDRESS = "EMAIL_ADDRESS" + +## @brief Environment variable: the app password of the sender's email account. +ENV_EMAIL_PASSWORD = "EMAIL_PASSWORD" + +## @brief Port of the SMTP server; the connection is upgraded with STARTTLS. +SMTP_PORT = 587 + +## @brief Subject line of the deal emails. +EMAIL_SUBJECT = "New Low Price Flight!" + ## @brief Seconds pywhatkit waits for WhatsApp Web to load before it sends the message. WHATSAPP_WAIT_SECONDS = 15 diff --git a/src/flight_club/main.py b/src/flight_club/main.py index 15a9ca7..37caa39 100644 --- a/src/flight_club/main.py +++ b/src/flight_club/main.py @@ -32,6 +32,14 @@ def _is_checkable(row: dict[str, object]) -> bool: ) +def _get_customer_emails(data_manager: DataManager) -> list[str]: + try: + return data_manager.extract_emails(data_manager.get_customer_emails()) + except DataManagerError as error: + print(f"No emails sent: {error}") + return [] + + ## @brief Checks every sheet row and reports each flight cheaper than the stored price. # @param data_manager Source of the destination rows and target of price updates. # @param flight_search Provider of flight offers. @@ -49,6 +57,7 @@ def run( six_month_from_today = today + timedelta(days=SEARCH_WINDOW_DAYS) return_date = six_month_from_today.strftime(DATE_FORMAT) found: list[FlightData] = [] + customer_emails: list[str] | None = None sheet_data = data_manager.get_destination_data() for row in sheet_data: @@ -70,7 +79,11 @@ def run( if cheapest.price != NOT_AVAILABLE and cheapest.price < row["lowestPrice"]: print(f"Lower price flight found to {row['city']}!") data_manager.update_lowest_price(row["id"], cheapest.price) # type: ignore[arg-type] - notification_manager.send_whatsapp(notification_manager.build_message(cheapest)) + message = notification_manager.build_message(cheapest) + notification_manager.send_whatsapp(message) + if customer_emails is None: + customer_emails = _get_customer_emails(data_manager) + notification_manager.send_emails(customer_emails, message) found.append(cheapest) return found diff --git a/src/flight_club/notification_manager.py b/src/flight_club/notification_manager.py index 38e6930..b19470a 100644 --- a/src/flight_club/notification_manager.py +++ b/src/flight_club/notification_manager.py @@ -1,14 +1,21 @@ """ @file -@brief Builds low price alerts and sends them through WhatsApp. +@brief Builds low price alerts and sends them through email and WhatsApp. """ import os +import smtplib from collections.abc import Callable +from email.message import EmailMessage from flight_club.constants import ( CURRENCY, + EMAIL_SUBJECT, + ENV_EMAIL_ADDRESS, + ENV_EMAIL_PASSWORD, + ENV_SMTP_ADDRESS, ENV_WHATSAPP_NUMBER, + SMTP_PORT, WHATSAPP_CLOSE_SECONDS, WHATSAPP_WAIT_SECONDS, ) @@ -16,30 +23,78 @@ from flight_club.flight_data import FlightData _INSTALL_HINT = 'Install the optional extra with: pip install -e ".[whatsapp]"' _NUMBER_HINT = f"Set {ENV_WHATSAPP_NUMBER} (with country code) to receive WhatsApp messages." +_EMAIL_HINT = ( + f"Set {ENV_SMTP_ADDRESS}, {ENV_EMAIL_ADDRESS} and {ENV_EMAIL_PASSWORD} to send emails." +) -## @brief Formats flight alerts and delivers them to the owner's WhatsApp. +## @brief Formats flight alerts and delivers them by email to the customers and to the owner's WhatsApp. class NotificationManager: - ## @brief Creates the manager with an optional replacement for pywhatkit. + ## @brief Reads every setting from the environment, in this one place. # @param sender Callable taking (number, message); used instead of pywhatkit when given. def __init__(self, sender: Callable[[str, str], None] | None = None) -> None: self._sender = sender + self._smtp_address = os.environ.get(ENV_SMTP_ADDRESS) + self._email_address = os.environ.get(ENV_EMAIL_ADDRESS) + self._email_password = os.environ.get(ENV_EMAIL_PASSWORD) + self._whatsapp_number = os.environ.get(ENV_WHATSAPP_NUMBER) ## @brief Builds the alert text for one cheap flight. + # + # A direct flight gets the short text; a flight with stopovers says how many. + # The price uses the currency code (GBP), never a currency symbol. # @param flight The cheapest flight found for a destination. # @return A one line message with price, airports and both dates. def build_message(self, flight: FlightData) -> str: - return ( + message = ( f"Low price alert! Only {CURRENCY} {flight.price} to fly " f"from {flight.origin_airport} to {flight.destination_airport}, " f"from {flight.out_date} to {flight.return_date}." ) + if flight.stops == 0: + return message + noun = "stopover" if flight.stops == 1 else "stopovers" + return f"{message} Flight has {flight.stops} {noun}." + + ## @brief Sends the message to every address; never raises. + # + # The mail is encoded as UTF-8, so non-ASCII characters do not fail. + # @param email_list Recipient addresses. + # @param email_body The text of the mail. + # @return The number of mails that were sent; 0 when the SMTP settings are missing. + def send_emails(self, email_list: list[str], email_body: str) -> int: + if not email_list: + return 0 + if not (self._smtp_address and self._email_address and self._email_password): + print(_EMAIL_HINT) + print(email_body) + return 0 + sent = 0 + try: + with smtplib.SMTP(self._smtp_address, SMTP_PORT) as connection: + connection.starttls() + connection.login(self._email_address, self._email_password) + for address in email_list: + connection.send_message(self._build_email(address, email_body)) + sent += 1 + except (smtplib.SMTPException, OSError) as error: + # Only the type is printed: the text of the error might echo the login. + print(f"Sending email failed: {type(error).__name__}.") + return sent + + def _build_email(self, address: str, email_body: str) -> EmailMessage: + message = EmailMessage() + message["Subject"] = EMAIL_SUBJECT + message["From"] = self._email_address + message["To"] = address + message.set_content(email_body, charset="utf-8") + return message ## @brief Sends a message to the number in the environment; never raises. # @param message The text to send. # @return True when sent, False when it was only printed. def send_whatsapp(self, message: str) -> bool: - number = os.environ.get(ENV_WHATSAPP_NUMBER) + number = self._whatsapp_number if not number: return self._print_instead(_NUMBER_HINT, message) if self._sender is not None: diff --git a/tests/test_main.py b/tests/test_main.py index 1f21ac4..6612b43 100644 --- a/tests/test_main.py +++ b/tests/test_main.py @@ -15,6 +15,8 @@ class FakeDataManager: def __init__(self, rows: list[dict[str, Any]]) -> None: self.rows = rows self.updates: list[tuple[int, float]] = [] + self.customers_error: Exception | None = None + self.customer_reads = 0 def get_destination_data(self) -> list[dict[str, Any]]: return self.rows @@ -22,6 +24,16 @@ class FakeDataManager: def update_lowest_price(self, row_id: int, new_price: float) -> None: self.updates.append((row_id, new_price)) + def get_customer_emails(self) -> list[dict[str, Any]]: + if self.customers_error is not None: + raise self.customers_error + self.customer_reads += 1 + return [{"email": "ann@example.test"}, {"email": "bo@example.test"}] + + @staticmethod + def extract_emails(customers: list[dict[str, Any]]) -> list[str]: + return [str(c["email"]) for c in customers] + class FakeFlightSearch: def __init__(self, price: float | None, indirect_price: float | None = None) -> None: @@ -54,6 +66,11 @@ class FakeFlightSearch: class FakeNotificationManager: def __init__(self) -> None: self.sent: list[str] = [] + self.emails: list[tuple[list[str], str]] = [] + + def send_emails(self, email_list: list[str], email_body: str) -> int: + self.emails.append((email_list, email_body)) + return len(email_list) def build_message(self, flight: FlightData) -> str: return f"alert {flight.destination_airport} {flight.price}" @@ -165,3 +182,39 @@ def test_run_reports_nothing_when_neither_search_finds_a_flight() -> None: assert search.direct_flags == [True, False] assert found == [] assert data.updates == [] + + +def test_run_emails_every_customer_when_a_cheaper_flight_is_found() -> None: + _, _, _, notifier = _run([_row(100)], 54) + assert notifier.emails == [(["ann@example.test", "bo@example.test"], "alert CDG 54.0")] + + +def test_run_sends_no_email_when_nothing_is_cheaper() -> None: + _, _, _, notifier = _run([_row(100)], 150) + assert notifier.emails == [] + + +def test_run_reads_the_customers_once_for_several_deals() -> None: + rows = [{**_row(), "iataCode": code, "id": i} for i, code in enumerate(["CDG", "BER"])] + data, search, notifier = FakeDataManager(rows), FakeFlightSearch(54), FakeNotificationManager() + run(data, search, notifier, today=TODAY) # type: ignore[arg-type] + assert data.customer_reads == 1 + assert len(notifier.emails) == 2 + + +def test_run_still_sends_whatsapp_when_the_customers_cannot_be_read( + capsys: pytest.CaptureFixture[str], +) -> None: + data, search, notifier = ( + FakeDataManager([_row(100)]), + FakeFlightSearch(54), + FakeNotificationManager(), + ) + data.customers_error = DataManagerError( + "Environment variable SHEETY_USERS_ENDPOINT is not set." + ) + found = run(data, search, notifier, today=TODAY) # type: ignore[arg-type] + assert [f.price for f in found] == [54] + assert notifier.sent == ["alert CDG 54.0"] + assert notifier.emails == [([], "alert CDG 54.0")] + assert "SHEETY_USERS_ENDPOINT" in capsys.readouterr().out diff --git a/tests/test_notification_manager.py b/tests/test_notification_manager.py index 2018683..603d568 100644 --- a/tests/test_notification_manager.py +++ b/tests/test_notification_manager.py @@ -1,11 +1,18 @@ import builtins +import smtplib import sys import types -from typing import Any +from typing import Any, ClassVar, Self import pytest -from flight_club.constants import ENV_WHATSAPP_NUMBER +from flight_club.constants import ( + ENV_EMAIL_ADDRESS, + ENV_EMAIL_PASSWORD, + ENV_SMTP_ADDRESS, + ENV_WHATSAPP_NUMBER, + SMTP_PORT, +) from flight_club.flight_data import FlightData from flight_club.notification_manager import NotificationManager @@ -89,3 +96,123 @@ def test_send_whatsapp_calls_pywhatkit_with_documented_arguments( monkeypatch.setitem(sys.modules, "pywhatkit", fake) assert NotificationManager().send_whatsapp("hello") is True assert received == [(NUMBER, "hello", 15, True, 3)] + + +SMTP_HOST = "smtp.example.test" +SENDER = "club@example.test" +APP_PASSWORD = "app-password-secret" + + +class FakeSMTP: + """Stands in for smtplib.SMTP; records what the manager does and sends nothing.""" + + instances: ClassVar[list["FakeSMTP"]] = [] + fail_with: ClassVar[Exception | None] = None + + def __init__(self, host: str, port: int) -> None: + if FakeSMTP.fail_with is not None: + raise FakeSMTP.fail_with + self.host = host + self.port = port + self.started_tls = False + self.login_args: tuple[str, str] | None = None + self.messages: list[Any] = [] + FakeSMTP.instances.append(self) + + def __enter__(self) -> Self: + return self + + def __exit__(self, *args: object) -> None: + return None + + def starttls(self) -> None: + self.started_tls = True + + def login(self, user: str, password: str) -> None: + self.login_args = (user, password) + + def send_message(self, message: Any) -> None: + self.messages.append(message) + + +@pytest.fixture +def smtp(monkeypatch: pytest.MonkeyPatch) -> type[FakeSMTP]: + FakeSMTP.instances = [] + FakeSMTP.fail_with = None + monkeypatch.setenv(ENV_SMTP_ADDRESS, SMTP_HOST) + monkeypatch.setenv(ENV_EMAIL_ADDRESS, SENDER) + monkeypatch.setenv(ENV_EMAIL_PASSWORD, APP_PASSWORD) + monkeypatch.setattr(smtplib, "SMTP", FakeSMTP) + return FakeSMTP + + +def test_send_emails_logs_in_and_sends_one_mail_per_address(smtp: type[FakeSMTP]) -> None: + sent = NotificationManager().send_emails(["a@example.test", "b@example.test"], "deal") + connection = smtp.instances[0] + assert sent == 2 + assert (connection.host, connection.port) == (SMTP_HOST, SMTP_PORT) + assert connection.started_tls + assert connection.login_args == (SENDER, APP_PASSWORD) + assert [m["To"] for m in connection.messages] == ["a@example.test", "b@example.test"] + assert all(m["From"] == SENDER for m in connection.messages) + assert connection.messages[0].get_content().strip() == "deal" + + +def test_send_emails_encodes_non_ascii_text_as_utf8(smtp: type[FakeSMTP]) -> None: + NotificationManager().send_emails(["a@example.test"], "Only £54 to Zürich") + message = smtp.instances[0].messages[0] + assert "£54 to Zürich" in message.get_content() + assert message.get_content_charset() == "utf-8" + assert "£54".encode() in message.as_bytes() + + +def test_send_emails_without_recipients_does_not_connect(smtp: type[FakeSMTP]) -> None: + assert NotificationManager().send_emails([], "deal") == 0 + assert smtp.instances == [] + + +@pytest.mark.parametrize("name", [ENV_SMTP_ADDRESS, ENV_EMAIL_ADDRESS, ENV_EMAIL_PASSWORD]) +def test_send_emails_prints_instead_when_a_setting_is_missing( + smtp: type[FakeSMTP], + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], + name: str, +) -> None: + monkeypatch.delenv(name) + assert NotificationManager().send_emails(["a@example.test"], "deal") == 0 + output = capsys.readouterr().out + assert "deal" in output + assert name in output + assert smtp.instances == [] + + +def test_send_emails_never_raises_or_prints_the_login_when_smtp_fails( + smtp: type[FakeSMTP], capsys: pytest.CaptureFixture[str] +) -> None: + smtp.fail_with = smtplib.SMTPAuthenticationError(535, f"bad login {APP_PASSWORD}".encode()) + assert NotificationManager().send_emails(["a@example.test"], "deal") == 0 + output = capsys.readouterr().out + assert "SMTPAuthenticationError" in output + assert APP_PASSWORD not in output + + +def test_build_message_for_a_direct_flight_has_no_stopover_text() -> None: + assert "stopover" not in NotificationManager().build_message(_flight()) + + +@pytest.mark.parametrize(("stops", "text"), [(1, "1 stopover."), (2, "2 stopovers.")]) +def test_build_message_for_a_stopover_flight_says_how_many(stops: int, text: str) -> None: + flight = FlightData(540, "LHR", "DPS", "2026-10-25", "2027-04-20", stops) + message = NotificationManager().build_message(flight) + assert message.endswith(f"Flight has {text}") + assert message != NotificationManager().build_message(_flight()) + + +def test_messages_use_gbp_and_not_the_pound_sign() -> None: + flight = FlightData(540, "LHR", "DPS", "2026-10-25", "2027-04-20", 1) + for message in ( + NotificationManager().build_message(flight), + NotificationManager().build_message(_flight()), + ): + assert "GBP" in message + assert "£" not in message