Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
2bbc35a82c | ||
|
|
49061c1105 | ||
|
|
29904c10f6 |
+4
-2
@@ -1,7 +1,9 @@
|
||||
# Copy to .env and fill in. .env is ignored by git; never commit it.
|
||||
|
||||
# Sheety: the endpoint of the "prices" sheet and the Bearer token of the project
|
||||
SHEETY_ENDPOINT=https://api.sheety.co/<your-id>/<your-project>/prices
|
||||
# Sheety: the endpoints of the "prices" sheet (destinations) and the "users" sheet
|
||||
# (the club's customers, filled by a Google Form), and the Bearer token of the project
|
||||
SHEETY_PRICES_ENDPOINT=https://api.sheety.co/<your-id>/<your-project>/prices
|
||||
SHEETY_USERS_ENDPOINT=https://api.sheety.co/<your-id>/<your-project>/users
|
||||
SHEETY_TOKEN=
|
||||
|
||||
# Flight provider: "serpapi" or "mock" (the course's mock flight service)
|
||||
|
||||
@@ -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 | 010 |
|
||||
| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 011 |
|
||||
|
||||
## Languages
|
||||
|
||||
|
||||
@@ -0,0 +1,61 @@
|
||||
# SQA Review Record
|
||||
|
||||
## Metadata
|
||||
| Key | Value |
|
||||
| --- | --- |
|
||||
| ID | RC-010 |
|
||||
| CrossReference | [MIL-003], [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 | [49061c1] |
|
||||
|
||||
---
|
||||
|
||||
## Artifact Under Review
|
||||
|
||||
- Instance reviewed: [MIL-003] (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 (`ENV_SHEETY_USERS_ENDPOINT`, `extract_emails`). |
|
||||
| 2 | Names state purpose in the domain's language; no unexplained abbreviations, no single-letter names outside tiny scopes | Pass | Names state purpose (`get_customer_emails`, `extract_emails`, `EMAIL_PATTERN`). |
|
||||
| 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 | No bare except; HTTP and shape problems raise `DataManagerError` with specific messages. |
|
||||
| 6 | No mutable default arguments and no shadowed builtins | Pass | No mutable defaults and no shadowed builtins; `_read_rows` is shared by both sheets. |
|
||||
| 7 | Files, locks and connections are managed with context managers | Pass | No files or connections opened by the new code; the HTTP session is unchanged. |
|
||||
| 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 | Console output uses `print` as the assignment requires; recorded in [ADR-0001]. No secrets are printed. |
|
||||
| 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: users read, errors, missing users endpoint, email extraction, legacy endpoint name. Fakes only; no network or `.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 | One private `_read_rows` serves both sheets; endpoint names, keys and the email pattern 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 | `get_destination_data` and `update_lowest_price` keep their contracts; the users endpoint is checked only when used, so Day 39 `.env` files still run. |
|
||||
| 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-003]: ../../milestones/mil-003-customer-emails.md
|
||||
[QC-PY-001]: ../../../framework/qc/code/qc-programming-python.md
|
||||
[ADR-0001]: ../../adr/adr-0001-flat-module-layout-and-console-output.md
|
||||
[49061c1]: https://git.tirsystem.com/Tirsvad-Udemy-100-days-of-code/040-personal-flight-club/commit/49061c1105f9874e073f83048fd0020217a1600a
|
||||
@@ -48,7 +48,13 @@ PROVIDER_SERPAPI = "serpapi"
|
||||
## @brief Name of the mock flight provider.
|
||||
PROVIDER_MOCK = "mock"
|
||||
|
||||
## @brief Environment variable: Sheety sheet endpoint.
|
||||
## @brief Environment variable: Sheety endpoint of the "prices" sheet.
|
||||
ENV_SHEETY_PRICES_ENDPOINT = "SHEETY_PRICES_ENDPOINT"
|
||||
|
||||
## @brief Environment variable: Sheety endpoint of the "users" sheet (the club's customers).
|
||||
ENV_SHEETY_USERS_ENDPOINT = "SHEETY_USERS_ENDPOINT"
|
||||
|
||||
## @brief Environment variable: the Day 39 name of the prices endpoint, still accepted.
|
||||
ENV_SHEETY_ENDPOINT = "SHEETY_ENDPOINT"
|
||||
|
||||
## @brief Environment variable: Sheety Bearer token.
|
||||
@@ -78,6 +84,18 @@ WHATSAPP_WAIT_SECONDS = 15
|
||||
## @brief Seconds pywhatkit waits after sending before it closes the browser tab.
|
||||
WHATSAPP_CLOSE_SECONDS = 3
|
||||
|
||||
## @brief Key that holds the rows in the Sheety response for the "prices" sheet.
|
||||
PRICES_KEY = "prices"
|
||||
|
||||
## @brief Key that holds the rows in the Sheety response for the "users" sheet.
|
||||
USERS_KEY = "users"
|
||||
|
||||
## @brief Field of a "users" row that holds the customer's email address.
|
||||
EMAIL_FIELD = "email"
|
||||
|
||||
## @brief Pattern of an accepted email address: text, an at sign and a dotted domain.
|
||||
EMAIL_PATTERN = r"[^@\s]+@[^@\s]+\.[^@\s]+"
|
||||
|
||||
## @brief Seconds to wait for an HTTP response before giving up.
|
||||
REQUEST_TIMEOUT_SECONDS = 30
|
||||
|
||||
|
||||
@@ -4,14 +4,21 @@
|
||||
"""
|
||||
|
||||
import os
|
||||
import re
|
||||
from typing import Any
|
||||
|
||||
import requests
|
||||
|
||||
from flight_club.constants import (
|
||||
EMAIL_FIELD,
|
||||
EMAIL_PATTERN,
|
||||
ENV_SHEETY_ENDPOINT,
|
||||
ENV_SHEETY_PRICES_ENDPOINT,
|
||||
ENV_SHEETY_TOKEN,
|
||||
ENV_SHEETY_USERS_ENDPOINT,
|
||||
PRICES_KEY,
|
||||
REQUEST_TIMEOUT_SECONDS,
|
||||
USERS_KEY,
|
||||
)
|
||||
|
||||
|
||||
@@ -20,15 +27,27 @@ class DataManagerError(RuntimeError):
|
||||
pass
|
||||
|
||||
|
||||
## @brief Talks to the Sheety sheet that holds the destinations.
|
||||
## @brief Talks to the Sheety sheets: the destinations ("prices") and the customers ("users").
|
||||
class DataManager:
|
||||
## @brief Reads the endpoint and token from the environment.
|
||||
## @brief Reads every setting from the environment, in this one place.
|
||||
#
|
||||
# The prices endpoint comes from SHEETY_PRICES_ENDPOINT, or from the Day 39 name
|
||||
# SHEETY_ENDPOINT. The users endpoint is optional here and checked when it is used,
|
||||
# so a run without customers still works.
|
||||
# @param session Optional HTTP session; a new one is made per call if omitted.
|
||||
def __init__(self, session: requests.Session | None = None) -> None:
|
||||
self._endpoint = self._require_env(ENV_SHEETY_ENDPOINT)
|
||||
self._prices_endpoint = self._prices_endpoint_from_env()
|
||||
self._users_endpoint = os.environ.get(ENV_SHEETY_USERS_ENDPOINT) or None
|
||||
self._headers = {"Authorization": f"Bearer {self._require_env(ENV_SHEETY_TOKEN)}"}
|
||||
self._session = session
|
||||
|
||||
@classmethod
|
||||
def _prices_endpoint_from_env(cls) -> str:
|
||||
legacy = os.environ.get(ENV_SHEETY_ENDPOINT)
|
||||
if legacy and not os.environ.get(ENV_SHEETY_PRICES_ENDPOINT):
|
||||
return legacy
|
||||
return cls._require_env(ENV_SHEETY_PRICES_ENDPOINT)
|
||||
|
||||
@staticmethod
|
||||
def _require_env(name: str) -> str:
|
||||
value = os.environ.get(name)
|
||||
@@ -45,18 +64,43 @@ class DataManager:
|
||||
if not 200 <= response.status_code < 300:
|
||||
raise DataManagerError(f"Sheety {action} failed with HTTP {response.status_code}.")
|
||||
|
||||
## @brief Fetch every destination row from the Google Sheet through the Sheety API.
|
||||
# @return Rows such as {'city': 'Paris', 'iataCode': 'CDG', 'lowestPrice': 54, 'id': 2}.
|
||||
def get_destination_data(self) -> list[dict[str, Any]]:
|
||||
def _read_rows(self, endpoint: str, key: str) -> list[dict[str, Any]]:
|
||||
response = self._get_session().get(
|
||||
self._endpoint, headers=self._headers, timeout=REQUEST_TIMEOUT_SECONDS
|
||||
endpoint, headers=self._headers, timeout=REQUEST_TIMEOUT_SECONDS
|
||||
)
|
||||
self._check(response, "read")
|
||||
body = response.json()
|
||||
prices = body.get("prices") if isinstance(body, dict) else None
|
||||
if not isinstance(prices, list):
|
||||
raise DataManagerError("Sheety response has no 'prices' list.")
|
||||
return prices
|
||||
rows = body.get(key) if isinstance(body, dict) else None
|
||||
if not isinstance(rows, list):
|
||||
raise DataManagerError(f"Sheety response has no '{key}' list.")
|
||||
return rows
|
||||
|
||||
## @brief Fetch every destination row from the Google Sheet through the Sheety API.
|
||||
# @return Rows such as {'city': 'Paris', 'iataCode': 'CDG', 'lowestPrice': 54, 'id': 2}.
|
||||
def get_destination_data(self) -> list[dict[str, Any]]:
|
||||
return self._read_rows(self._prices_endpoint, PRICES_KEY)
|
||||
|
||||
## @brief Fetch every customer row from the "users" sheet.
|
||||
# @return Rows such as {'firstName': 'Ann', 'email': 'ann@example.com', 'id': 2}.
|
||||
def get_customer_emails(self) -> list[dict[str, Any]]:
|
||||
if self._users_endpoint is None:
|
||||
raise DataManagerError(f"Environment variable {ENV_SHEETY_USERS_ENDPOINT} is not set.")
|
||||
return self._read_rows(self._users_endpoint, USERS_KEY)
|
||||
|
||||
## @brief Picks the usable email addresses out of the customer rows.
|
||||
# @param customers Rows from get_customer_emails().
|
||||
# @return The addresses that look like an email address, in sheet order, without duplicates.
|
||||
@staticmethod
|
||||
def extract_emails(customers: list[dict[str, Any]]) -> list[str]:
|
||||
emails: list[str] = []
|
||||
for customer in customers:
|
||||
email = customer.get(EMAIL_FIELD)
|
||||
if not isinstance(email, str):
|
||||
continue
|
||||
email = email.strip()
|
||||
if re.fullmatch(EMAIL_PATTERN, email) and email not in emails:
|
||||
emails.append(email)
|
||||
return emails
|
||||
|
||||
## @brief Stores a new lowest price for one row.
|
||||
# @param row_id Sheety row id.
|
||||
@@ -64,7 +108,7 @@ class DataManager:
|
||||
# @return None
|
||||
def update_lowest_price(self, row_id: int, new_price: float) -> None:
|
||||
response = self._get_session().put(
|
||||
f"{self._endpoint}/{row_id}",
|
||||
f"{self._prices_endpoint}/{row_id}",
|
||||
json={"price": {"lowestPrice": new_price}},
|
||||
headers=self._headers,
|
||||
timeout=REQUEST_TIMEOUT_SECONDS,
|
||||
|
||||
@@ -7,12 +7,15 @@ from flight_club.constants import REQUEST_TIMEOUT_SECONDS
|
||||
from flight_club.data_manager import DataManager, DataManagerError
|
||||
|
||||
ENDPOINT = "https://api.sheety.co/abc/flights/prices"
|
||||
USERS_ENDPOINT = "https://api.sheety.co/abc/flights/users"
|
||||
TOKEN = "super-secret-token"
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _env(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("SHEETY_ENDPOINT", ENDPOINT)
|
||||
monkeypatch.delenv("SHEETY_ENDPOINT", raising=False)
|
||||
monkeypatch.setenv("SHEETY_PRICES_ENDPOINT", ENDPOINT)
|
||||
monkeypatch.setenv("SHEETY_USERS_ENDPOINT", USERS_ENDPOINT)
|
||||
monkeypatch.setenv("SHEETY_TOKEN", TOKEN)
|
||||
|
||||
|
||||
@@ -75,10 +78,70 @@ def test_errors_never_contain_token() -> None:
|
||||
assert TOKEN not in str(write_error.value)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("name", ["SHEETY_ENDPOINT", "SHEETY_TOKEN"])
|
||||
@pytest.mark.parametrize("name", ["SHEETY_PRICES_ENDPOINT", "SHEETY_TOKEN"])
|
||||
def test_missing_env_variable_names_the_variable(
|
||||
monkeypatch: pytest.MonkeyPatch, name: str
|
||||
) -> None:
|
||||
monkeypatch.delenv(name)
|
||||
with pytest.raises(DataManagerError, match=name):
|
||||
DataManager(_session())
|
||||
|
||||
|
||||
USERS = [
|
||||
{"firstName": "Ann", "lastName": "Lee", "email": "ann@example.com", "id": 2},
|
||||
{"firstName": "Bo", "lastName": "Kim", "email": " bo@example.com ", "id": 3},
|
||||
]
|
||||
|
||||
|
||||
def test_get_customer_emails_returns_users() -> None:
|
||||
session = _session(body={"users": USERS})
|
||||
assert DataManager(session).get_customer_emails() == USERS
|
||||
session.get.assert_called_once_with(
|
||||
USERS_ENDPOINT,
|
||||
headers={"Authorization": f"Bearer {TOKEN}"},
|
||||
timeout=REQUEST_TIMEOUT_SECONDS,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("body", [{}, {"users": "x"}, [1]])
|
||||
def test_get_customer_emails_raises_without_users_list(body: Any) -> None:
|
||||
with pytest.raises(DataManagerError, match="users"):
|
||||
DataManager(_session(body=body)).get_customer_emails()
|
||||
|
||||
|
||||
def test_get_customer_emails_raises_on_http_error() -> None:
|
||||
with pytest.raises(DataManagerError, match="401"):
|
||||
DataManager(_session(status=401)).get_customer_emails()
|
||||
|
||||
|
||||
def test_get_customer_emails_needs_the_users_endpoint(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.delenv("SHEETY_USERS_ENDPOINT")
|
||||
manager = DataManager(_session(body={"users": USERS}))
|
||||
with pytest.raises(DataManagerError, match="SHEETY_USERS_ENDPOINT"):
|
||||
manager.get_customer_emails()
|
||||
|
||||
|
||||
def test_extract_emails_keeps_valid_unique_addresses() -> None:
|
||||
customers: list[dict[str, Any]] = [
|
||||
*USERS,
|
||||
{"email": "ann@example.com"},
|
||||
{"email": "not-an-email"},
|
||||
{"email": None},
|
||||
{"firstName": "No", "lastName": "Mail"},
|
||||
]
|
||||
assert DataManager.extract_emails(customers) == ["ann@example.com", "bo@example.com"]
|
||||
|
||||
|
||||
def test_legacy_endpoint_name_is_still_accepted(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.delenv("SHEETY_PRICES_ENDPOINT")
|
||||
monkeypatch.setenv("SHEETY_ENDPOINT", "https://api.sheety.co/old/prices")
|
||||
session = _session(body={"prices": []})
|
||||
DataManager(session).get_destination_data()
|
||||
assert session.get.call_args.args == ("https://api.sheety.co/old/prices",)
|
||||
|
||||
|
||||
def test_prices_endpoint_wins_over_the_legacy_name(monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
monkeypatch.setenv("SHEETY_ENDPOINT", "https://api.sheety.co/old/prices")
|
||||
session = _session(body={"prices": []})
|
||||
DataManager(session).get_destination_data()
|
||||
assert session.get.call_args.args == (ENDPOINT,)
|
||||
|
||||
Reference in New Issue
Block a user