From be042949a2762247de8c04dcdb6f9b811547010a Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Fri, 28 Aug 2026 21:10:31 +0200 Subject: [PATCH 1/3] Keep CCMA usable when the member store is mounted read-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The encrypted volume holding the member data can be mounted without write access, but starting against such a store failed: the housekeeper takes a lock file before doing anything, so its startup pass died with a PermissionError and took the whole start with it. The store is now probed with an actual write once at startup -- permissions, mount options and filesystem state all matter, and only an attempt covers them together -- and a read-only store opens as a read-only session. The housekeeper is skipped rather than attempted, every write inside the repository goes through one guard that reports ReadOnlyStoreError (a RepositoryError, so the dialogs already handle it) instead of letting an OS error surface, and the services that archive into the member file check before they start sending or rendering. The session says so permanently: a warning banner above the tabs, "NUR LESEN" in the window title and status bar, and refused actions explaining why. Program settings still save -- they live in the user's config directory -- while the store-backed ones are skipped with a notice. "Erneut prüfen" picks up a volume that was remounted writable without restarting. A store that was never initialized still fails, but says that creating one needs write access. Co-Authored-By: Claude Opus 5 --- README.md | 18 +++ src/ccma/app.py | 2 + src/ccma/assets/CHANGELOG.json | 3 +- src/ccma/services/documents.py | 1 + src/ccma/services/housekeeper.py | 4 + src/ccma/services/reminder_mail.py | 1 + src/ccma/services/sepa_mail.py | 1 + src/ccma/storage/repository.py | 97 +++++++++++++--- src/ccma/ui/asset_tab.py | 6 +- src/ccma/ui/main_window.py | 72 +++++++++++- src/ccma/ui/member_tab.py | 24 ++-- src/ccma/ui/options_dialog.py | 41 ++++--- src/ccma/ui/splash.py | 9 +- tests/test_read_only_store.py | 171 +++++++++++++++++++++++++++++ 14 files changed, 403 insertions(+), 47 deletions(-) create mode 100644 tests/test_read_only_store.py diff --git a/README.md b/README.md index a2a4112..73d55fa 100644 --- a/README.md +++ b/README.md @@ -183,6 +183,24 @@ marker may share the last cell. CCMA removes both markers and clones the whole row, including its formatting, once per item. With no items, the template row is removed. A loop that is not closed in the same row is rejected. +## Read-only stores + +The member store may be mounted read-only -- the encrypted volume holding the +member data does not have to be writable to look something up. CCMA detects this +at startup by probing the store with an actual write, skips the housekeeper (every +one of its passes writes) and opens in a read-only session: a permanent warning +banner above the tabs, a "NUR LESEN" marker in the window title and status bar, +and every write refused with one clear message instead of an operating system +error. + +Program settings still save normally -- they live in the user's config directory, +not in the store. Settings that belong to the store (club data, member numbers, +reminders, e-mail) are skipped with a notice. When the volume is remounted with +write access, "Erneut prüfen" in the banner picks that up without a restart. + +A store that was never initialized cannot be opened read-only: creating it needs +write access, and CCMA says so instead of failing obscurely. + ## Housekeeper rules The housekeeper runs every rule for every member. Built-in Python rules live in diff --git a/src/ccma/app.py b/src/ccma/app.py index b5f690d..89c7ef2 100644 --- a/src/ccma/app.py +++ b/src/ccma/app.py @@ -78,6 +78,8 @@ class CCMAApp(tk.Tk): def _startup_complete(self, result: StartupResult) -> None: self.deiconify() + if result.repository.read_only: + self.title(f"CCMA · v{__version__} · NUR LESEN (Store schreibgeschützt)") main = MainWindow( self, result.repository, diff --git a/src/ccma/assets/CHANGELOG.json b/src/ccma/assets/CHANGELOG.json index 3708993..63badea 100644 --- a/src/ccma/assets/CHANGELOG.json +++ b/src/ccma/assets/CHANGELOG.json @@ -38,7 +38,8 @@ "Eine überfällige Lastschrift, für die bereits eine Rücklastschrift-Mahnung versandt wurde und deren Zahlungsfrist noch läuft, wird vom Hausmeister nicht mehr weiterhin als überfällig gemeldet, sondern als niedrig priorisierter Hinweis, dass die Klärung noch läuft.", "Im SEPA-Export lassen sich einzelne Lastschriften per Mehrfachauswahl von der CSV-/XML-/Info-Mail-Erstellung ausschließen (z. B. während eine Rücklastschrift geklärt wird), statt immer alle offenen Lastschriften einziehen zu müssen; standardmäßig sind alle ausgewählt.", "Mahnungs- und SEPA-Info-Mails müssen nicht mehr zwingend als lokale Datei gespeichert und von Hand in Thunderbird importiert werden: unter Optionen → E-Mail-Versand kann ein Postfach mit SMTP- und IMAP-Zugangsdaten hinterlegt werden, wahlweise mit direktem Versand, Ablage als Entwurf im Postfach oder einer Rückfrage bei jeder Erzeugung; die bisherige lokale Ablage bleibt als Standard erhalten.", - "Direkt versandte Mahnungen und SEPA-Info-Mails können zusätzlich automatisch in einen Gesendet-Ordner des Postfachs abgelegt werden. Für den Entwürfe- und den Gesendet-Ordner lässt sich die tatsächliche Ordnerliste live vom Server abrufen und per Dropdown auswählen; ein noch nicht vorhandener, frei eingegebener Ordner wird bei Bedarf automatisch angelegt." + "Direkt versandte Mahnungen und SEPA-Info-Mails können zusätzlich automatisch in einen Gesendet-Ordner des Postfachs abgelegt werden. Für den Entwürfe- und den Gesendet-Ordner lässt sich die tatsächliche Ordnerliste live vom Server abrufen und per Dropdown auswählen; ein noch nicht vorhandener, frei eingegebener Ordner wird bei Bedarf automatisch angelegt.", + "CCMA startet jetzt auch, wenn der Mitglieder-Store schreibgeschützt eingebunden ist (z. B. ein nur lesend gemountetes Crypt-Laufwerk): Die Daten lassen sich wie gewohnt ansehen, der Hausmeister wird übersprungen und alle schreibenden Aktionen werden mit einer verständlichen Meldung abgelehnt statt mit einem Systemfehler. Ein dauerhaft sichtbarer Warnhinweis über den Tabs sowie Markierungen in Fenstertitel und Statusleiste weisen auf den Nur-Lese-Betrieb hin; wird der Store wieder mit Schreibrechten eingebunden, genügt „Erneut prüfen“ ohne Neustart." ] }, { diff --git a/src/ccma/services/documents.py b/src/ccma/services/documents.py index 42e2b87..1516bba 100644 --- a/src/ccma/services/documents.py +++ b/src/ccma/services/documents.py @@ -98,6 +98,7 @@ class DocumentService: if not template_path.is_file() or template_path.suffix.casefold() not in SUPPORTED_TEMPLATE_SUFFIXES: raise DocumentError("Das ausgewählte OpenDocument-Template ist nicht verfügbar.") + self.repository.assert_writable() member = self.repository.get_member(member_id) data = None claim = None diff --git a/src/ccma/services/housekeeper.py b/src/ccma/services/housekeeper.py index 555f9d1..4c054b6 100644 --- a/src/ccma/services/housekeeper.py +++ b/src/ccma/services/housekeeper.py @@ -77,6 +77,9 @@ class Housekeeper: overrides the configured setting for just this run without changing it permanently -- e.g. to recompute one member's full claim history on demand after a contribution change, without retroactively billing everyone else.""" + # The housekeeper exists to write: it refreshes tasks, creates claims and + # updates member records. On a read-only store it must not even take its lock. + self.repository.assert_writable() current_date = today or date.today() delay = _non_negative_delay(member_delay) effective_settings = ( @@ -189,6 +192,7 @@ class Housekeeper: return _open_findings(working["items"]) def delete_task(self, key: str) -> list[HousekeeperFinding]: + self.repository.assert_writable() selected_key = key.strip() if not selected_key: raise RepositoryError("Der Task hat keinen gültigen Key.") diff --git a/src/ccma/services/reminder_mail.py b/src/ccma/services/reminder_mail.py index a3a0e26..7743236 100644 --- a/src/ccma/services/reminder_mail.py +++ b/src/ccma/services/reminder_mail.py @@ -133,6 +133,7 @@ def generate_and_send_reminder_mail( ) -> GeneratedReminderMail: if delivery_mode not in {"local", "send", "drafts"}: raise RepositoryError(f"Unbekannter Versandmodus: {delivery_mode}") + repository.assert_writable() member = repository.get_member(member_id) data, claim = repository.get_claim(member_id, claim_id) reminder = repository._find_reminder(data, claim_id, reminder_id) diff --git a/src/ccma/services/sepa_mail.py b/src/ccma/services/sepa_mail.py index e581d58..8016442 100644 --- a/src/ccma/services/sepa_mail.py +++ b/src/ccma/services/sepa_mail.py @@ -109,6 +109,7 @@ def generate_debit_mails( sender_email: str, signature: str, ) -> tuple[list[GeneratedDebitMail], list[str]]: + repository.assert_writable() output: Path | None = None email_settings: dict | None = None smtp_ctx = nullcontext(None) diff --git a/src/ccma/storage/repository.py b/src/ccma/storage/repository.py index c15cfef..d2fae21 100644 --- a/src/ccma/storage/repository.py +++ b/src/ccma/storage/repository.py @@ -53,6 +53,12 @@ class RepositoryError(RuntimeError): pass +class ReadOnlyStoreError(RepositoryError): + """Raised instead of a bare PermissionError when the store is mounted read-only: + the encrypted volume holding the member data can be mounted without write access, + and CCMA stays usable for reading in that case.""" + + DEFAULT_MEMBER_NUMBER_PATTERN = "CCMA-{number:04d}" DEFAULT_CONTRIBUTION_RULES = [ @@ -167,8 +173,60 @@ class MemberRepository: self.root = Path(root).expanduser().resolve() self.members_root = self.root / "members" self.assets_root = self.root / "assets" + self._read_only: bool | None = None + + @property + def read_only(self) -> bool: + """Whether the store can be written to at all. Probed once and cached: the + answer only changes when the volume is remounted, which needs a restart (or + an explicit refresh_read_only()) anyway.""" + if self._read_only is None: + self._read_only = self.detect_read_only() + return self._read_only + + def refresh_read_only(self) -> bool: + self._read_only = self.detect_read_only() + return self._read_only + + def detect_read_only(self) -> bool: + # Probe by actually writing: permissions, mount options and filesystem state + # all matter, and only an attempt covers them together. A store directory that + # does not exist yet counts as writable when its nearest existing parent is -- + # that is an ordinary first start, not a read-only volume. + target = self.root + while not target.exists() and target != target.parent: + target = target.parent + probe = target / f".ccma-write-probe-{uuid4().hex}" + try: + probe.touch() + probe.unlink() + except OSError: + return True + return False + + def assert_writable(self) -> None: + if self.read_only: + raise ReadOnlyStoreError( + "Der Mitglieder-Store ist schreibgeschützt eingebunden. Änderungen sind " + "erst wieder möglich, wenn der Store mit Schreibrechten eingebunden ist." + ) + + def _write_json(self, path: Path, data) -> None: + self.assert_writable() + write_json_atomic(path, data) def initialize(self) -> None: + # A read-only store cannot be set up -- but it also does not need to be, as + # long as it was initialized while it was still writable. Opening one that was + # never initialized is the only case that has to fail here. + if self.read_only: + if not (self.root / "repository.json").is_file(): + raise ReadOnlyStoreError( + f"Der Mitglieder-Store {self.root} ist schreibgeschützt eingebunden und " + "enthält keine repository.json. Ein neuer Store kann nur mit " + "Schreibrechten angelegt werden." + ) + return self.members_root.mkdir(parents=True, exist_ok=True) self.assets_root.mkdir(parents=True, exist_ok=True) (self.root / "rules").mkdir(parents=True, exist_ok=True) @@ -185,7 +243,7 @@ class MemberRepository: shutil.copyfile(source, destination) config_path = self.root / "repository.json" if not config_path.exists(): - write_json_atomic(config_path, DEFAULT_CONFIGURATION) + self._write_json(config_path, DEFAULT_CONFIGURATION) def validate(self) -> list[str]: errors: list[str] = [] @@ -376,6 +434,7 @@ class MemberRepository: selected_number = self._allocate_member_number( policy["pattern"], policy["allocation_strategy"] ) + self.assert_writable() member_id = str(uuid4()) directory = self._member_path(member_id) directory.mkdir(parents=True, exist_ok=False) @@ -391,8 +450,8 @@ class MemberRepository: birth_date=birth_date, application_date=application_date, ) - write_json_atomic(directory / "member.json", member.to_dict()) - write_json_atomic(directory / "contributions.json", ContributionData().to_dict()) + self._write_json(directory / "member.json", member.to_dict()) + self._write_json(directory / "contributions.json", ContributionData().to_dict()) self.append_event( member_id, event_type="member_created", @@ -440,7 +499,7 @@ class MemberRepository: self._assert_member_number_available(member.member_number, exclude_member_id=member.member_id) changes = self._summarize_changes(existing, member) member.updated_at = datetime.now().astimezone().isoformat(timespec="seconds") - write_json_atomic(self._member_path(member.member_id) / "member.json", member.to_dict()) + self._write_json(self._member_path(member.member_id) / "member.json", member.to_dict()) if changes: self.append_event( member.member_id, @@ -501,6 +560,7 @@ class MemberRepository: if deposit_amount < 0: raise RepositoryError("Die Kaution darf nicht negativ sein.") self._validate_asset_relationships(owner_type, owner_member_id, owner_name, custody_type, "") + self.assert_writable() asset_id = str(uuid4()) directory = self._asset_path(asset_id) directory.mkdir(parents=True, exist_ok=False) @@ -522,7 +582,7 @@ class MemberRepository: condition=condition.strip(), estimated_value=estimated_value.strip(), ) - write_json_atomic(directory / "asset.json", asset.to_dict()) + self._write_json(directory / "asset.json", asset.to_dict()) self.append_asset_event( asset.asset_id, event_type="asset_created", @@ -585,7 +645,7 @@ class MemberRepository: raise RepositoryError("Status issued benötigt ein zugeordnetes Mitglied.") changes = self._summarize_asset_changes(existing, asset) asset.updated_at = datetime.now().astimezone().isoformat(timespec="seconds") - write_json_atomic(self._asset_path(asset.asset_id) / "asset.json", asset.to_dict()) + self._write_json(self._asset_path(asset.asset_id) / "asset.json", asset.to_dict()) if changes: self.append_asset_event( asset.asset_id, @@ -625,7 +685,7 @@ class MemberRepository: asset.custody_type = "member" asset.status = "issued" asset.updated_at = datetime.now().astimezone().isoformat(timespec="seconds") - write_json_atomic(self._asset_path(asset.asset_id) / "asset.json", asset.to_dict()) + self._write_json(self._asset_path(asset.asset_id) / "asset.json", asset.to_dict()) self.append_asset_event( asset.asset_id, event_type="asset_issued", @@ -653,7 +713,7 @@ class MemberRepository: asset.custody_type = "club" asset.status = "available" asset.updated_at = datetime.now().astimezone().isoformat(timespec="seconds") - write_json_atomic(self._asset_path(asset.asset_id) / "asset.json", asset.to_dict()) + self._write_json(self._asset_path(asset.asset_id) / "asset.json", asset.to_dict()) self.append_asset_event( asset.asset_id, event_type="asset_returned", @@ -812,6 +872,7 @@ class MemberRepository: references=references or {}, data=data or {}, ) + self.assert_writable() path = directory / "events.jsonl" line = json.dumps(event.to_dict(), ensure_ascii=False, separators=(",", ":")) + "\n" with path.open("a", encoding="utf-8", newline="\n") as handle: @@ -867,7 +928,7 @@ class MemberRepository: def save_contributions(self, member_id: str, data: ContributionData) -> None: self.get_member(member_id) - write_json_atomic(self._member_path(member_id) / "contributions.json", data.to_dict()) + self._write_json(self._member_path(member_id) / "contributions.json", data.to_dict()) def get_claim(self, member_id: str, claim_id: str) -> tuple[ContributionData, dict]: data = self.get_contributions(member_id) @@ -2390,6 +2451,7 @@ class MemberRepository: references=references or {}, data=data or {}, ) + self.assert_writable() path = directory / "events.jsonl" line = json.dumps(event.to_dict(), ensure_ascii=False, separators=(",", ":")) + "\n" with path.open("a", encoding="utf-8", newline="\n") as handle: @@ -2496,12 +2558,12 @@ class MemberRepository: def refresh_member_record_hashes(self, member_id: str) -> None: member = self.get_member(member_id) contributions = self.get_contributions(member_id) - write_json_atomic(self._member_path(member_id) / "member.json", member.to_dict()) - write_json_atomic(self._member_path(member_id) / "contributions.json", contributions.to_dict()) + self._write_json(self._member_path(member_id) / "member.json", member.to_dict()) + self._write_json(self._member_path(member_id) / "contributions.json", contributions.to_dict()) def refresh_asset_record_hashes(self, asset_id: str) -> None: asset = self.get_asset(asset_id) - write_json_atomic(self._asset_path(asset_id) / "asset.json", asset.to_dict()) + self._write_json(self._asset_path(asset_id) / "asset.json", asset.to_dict()) def get_member_number_policy(self) -> dict[str, str]: try: @@ -2538,7 +2600,7 @@ class MemberRepository: "allocation_strategy": allocation_strategy, } config.setdefault("member_number_sequences", {}) - write_json_atomic(self.root / "repository.json", config) + self._write_json(self.root / "repository.json", config) def get_reminder_policy(self) -> dict: config = self.get_configuration() @@ -2626,7 +2688,7 @@ class MemberRepository: "levels": normalized_levels, "standard_fee_items": normalized_items, } - write_json_atomic(self.root / "repository.json", config) + self._write_json(self.root / "repository.json", config) def get_email_settings(self) -> dict: config = self.get_configuration() @@ -2720,7 +2782,7 @@ class MemberRepository: "imap_sent_enabled": bool(imap_sent_enabled), "imap_sent_folder": imap_sent_folder.strip() or "INBOX.Sent", } - write_json_atomic(self.root / "repository.json", config) + self._write_json(self.root / "repository.json", config) def save_organization(self, values: dict[str, str]) -> None: organization = {key: str(value).strip() for key, value in values.items()} @@ -2732,7 +2794,7 @@ class MemberRepository: raise RepositoryError("Der Vereinsname ist erforderlich.") config = self.get_configuration() config["organization"] = organization - write_json_atomic(self.root / "repository.json", config) + self._write_json(self.root / "repository.json", config) def preview_member_number( self, pattern: str | None = None, allocation_strategy: str | None = None @@ -2778,7 +2840,7 @@ class MemberRepository: sequences = {} config["member_number_sequences"] = sequences sequences[pattern] = next_value - write_json_atomic(self.root / "repository.json", config) + self._write_json(self.root / "repository.json", config) return member_number def _next_available_member_number( @@ -2804,6 +2866,7 @@ class MemberRepository: @contextmanager def _member_number_lock(self): + self.assert_writable() lock_path = self.root / ".member-number.lock" lock_path.parent.mkdir(parents=True, exist_ok=True) with lock_path.open("a+", encoding="utf-8") as handle: diff --git a/src/ccma/ui/asset_tab.py b/src/ccma/ui/asset_tab.py index 075fc9d..7548e72 100644 --- a/src/ccma/ui/asset_tab.py +++ b/src/ccma/ui/asset_tab.py @@ -404,7 +404,11 @@ class AssetTab(ttk.Frame): self._confirm_integrity_and_then(self._refresh_hashes_only) def _refresh_hashes_only(self) -> None: - self.repository.refresh_asset_record_hashes(self.asset_id) + try: + self.repository.refresh_asset_record_hashes(self.asset_id) + except RepositoryError as exc: + messagebox.showerror("Prüfsummen konnten nicht erneuert werden", str(exc), parent=self) + return self.refresh() self.on_changed() diff --git a/src/ccma/ui/main_window.py b/src/ccma/ui/main_window.py index fc4ab69..7dc3c9a 100644 --- a/src/ccma/ui/main_window.py +++ b/src/ccma/ui/main_window.py @@ -13,6 +13,7 @@ from ccma.ui.claim_tab import ClaimTab from ccma.ui.dialogs import EditAssetDialog, IssueAssetDialog, NewAssetDialog, NewMemberDialog from ccma.ui.icons import IconStore from ccma.ui.member_tab import MemberTab +from ccma.ui.messages import MessageAction, MessageBannerList, TabMessage from ccma.ui.options_dialog import OptionsDialog from ccma.ui.sepa_dialog import SepaExportDialog from ccma.ui.theme import load_theme @@ -98,10 +99,15 @@ class MainWindow(ttk.Frame): def _build_ui(self) -> None: self.columnconfigure(0, weight=1) - self.rowconfigure(1, weight=1) + self.rowconfigure(2, weight=1) self._build_ribbon() + # The read-only notice sits above the tabs rather than inside one, so it stays + # visible no matter which member, asset or housekeeper tab is in front. + self.messages = MessageBannerList(self) + self.messages.grid(row=1, column=0, sticky="ew", pady=(10, 0)) + self._refresh_store_messages() self.notebook = ttk.Notebook(self) - self.notebook.grid(row=1, column=0, sticky="nsew", pady=(10, 0)) + self.notebook.grid(row=2, column=0, sticky="nsew", pady=(10, 0)) self.tabs = TabManager(self.notebook) self.dashboard = DashboardTab( self.notebook, @@ -117,17 +123,63 @@ class MainWindow(ttk.Frame): icon_name="view-dashboard", ) status = ttk.Frame(self, padding=(6, 5)) - status.grid(row=2, column=0, sticky="ew") + status.grid(row=3, column=0, sticky="ew") status.columnconfigure(0, weight=1) ttk.Label(status, textvariable=self.status_var, style="Status.TLabel").grid( row=0, column=0, sticky="w" ) + store_label = f"STORE {self.repository.root}" + if self.repository.read_only: + store_label = f"{store_label} · NUR LESEN" ttk.Label( status, - text=f"STORE {self.repository.root} · VERSION {__version__}", + text=f"{store_label} · VERSION {__version__}", style="Status.TLabel", ).grid(row=0, column=1, sticky="e") + def _refresh_store_messages(self) -> None: + messages = [] + if self.repository.read_only: + messages.append( + TabMessage( + "warning", + "ACHTUNG: Der Mitglieder-Store ist schreibgeschützt eingebunden. " + "Alle Daten sind nur lesbar – Änderungen, Dokumente, E-Mails und der " + "Hausmeister sind deaktiviert. Zum Bearbeiten den Store mit " + "Schreibrechten einbinden und CCMA neu starten.", + MessageAction("Erneut prüfen", self._recheck_store_access), + ) + ) + self.messages.set_messages(messages) + + def _recheck_store_access(self) -> None: + """Lets the board re-mount the volume without restarting first -- if it came + back writable, the notice disappears and the housekeeper works again.""" + if self.repository.refresh_read_only(): + self.status_var.set("Der Store ist weiterhin schreibgeschützt.") + self._refresh_store_messages() + return + self._refresh_store_messages() + self._refresh_ribbon_icons() + self.status_var.set("Der Store ist wieder beschreibbar.") + messagebox.showinfo( + "Store beschreibbar", + "Der Mitglieder-Store ist jetzt mit Schreibrechten eingebunden. " + "Der Hausmeister wurde beim Start übersprungen und kann nun ausgeführt werden.", + parent=self, + ) + + def _refuse_read_only(self, action: str) -> bool: + if not self.repository.read_only: + return False + messagebox.showwarning( + "Store ist schreibgeschützt", + f"{action} ist nicht möglich, solange der Mitglieder-Store nur lesend " + "eingebunden ist.", + parent=self, + ) + return True + def _build_ribbon(self) -> None: ribbon = ttk.Frame(self, style="Ribbon.TFrame") self.ribbon = ribbon @@ -417,9 +469,13 @@ class MainWindow(ttk.Frame): SepaExportDialog(self, self.repository) def new_member(self) -> None: + if self._refuse_read_only("Das Anlegen eines Mitglieds"): + return NewMemberDialog(self, self.repository, self._member_created) def new_asset(self) -> None: + if self._refuse_read_only("Das Anlegen eines Assets"): + return NewAssetDialog(self, self.repository, self._asset_created) def _member_created(self, member: Member) -> None: @@ -499,6 +555,8 @@ class MainWindow(ttk.Frame): ) def run_housekeeper(self) -> list[HousekeeperFinding]: + if self._refuse_read_only("Ein Hausmeisterlauf"): + return self.findings self.findings = Housekeeper(self.repository, self.config.housekeeper_settings()).run() self.refresh_overview(run_housekeeper=False) self.status_var.set(f"Hausmeisterlauf beendet: {len(self.findings)} Vorgänge.") @@ -507,6 +565,8 @@ class MainWindow(ttk.Frame): def run_housekeeper_for_member( self, member_id: str, retroactive_claims: bool = False ) -> list[HousekeeperFinding]: + if self._refuse_read_only("Ein Hausmeisterlauf"): + return self.findings self.findings = Housekeeper(self.repository, self.config.housekeeper_settings()).run( member_id=member_id, retroactive_claims=retroactive_claims or None ) @@ -515,13 +575,15 @@ class MainWindow(ttk.Frame): return self.findings def delete_housekeeper_task(self, key: str) -> list[HousekeeperFinding]: + if self._refuse_read_only("Das Löschen eines Hausmeister-Tasks"): + return self.findings self.findings = Housekeeper(self.repository, self.config.housekeeper_settings()).delete_task(key) self.refresh_overview(run_housekeeper=False) self.status_var.set("Hausmeister-Task gelöscht.") return self.findings def refresh_overview(self, *, run_housekeeper: bool = True) -> None: - if run_housekeeper: + if run_housekeeper and not self.repository.read_only: self.findings = Housekeeper(self.repository, self.config.housekeeper_settings()).run() self.dashboard.update_data(self.repository.member_count(), self.findings) members_tab = self.tabs.tabs.get("members") diff --git a/src/ccma/ui/member_tab.py b/src/ccma/ui/member_tab.py index f599045..7fad302 100644 --- a/src/ccma/ui/member_tab.py +++ b/src/ccma/ui/member_tab.py @@ -1122,7 +1122,11 @@ class MemberTab(ttk.Frame): self._confirm_integrity_and_then(self._refresh_hashes_only) def _refresh_hashes_only(self) -> None: - self.repository.refresh_member_record_hashes(self.member_id) + try: + self.repository.refresh_member_record_hashes(self.member_id) + except RepositoryError as exc: + messagebox.showerror("Prüfsummen konnten nicht erneuert werden", str(exc), parent=self) + return self.refresh() self.on_changed() @@ -1142,13 +1146,17 @@ class MemberTab(ttk.Frame): text = self.comment_var.get().strip() if not text: return - self.repository.append_event( - self.member_id, - event_type="board_comment", - summary=text, - actor_type="user", - actor_name="Vorstand", - ) + try: + self.repository.append_event( + self.member_id, + event_type="board_comment", + summary=text, + actor_type="user", + actor_name="Vorstand", + ) + except RepositoryError as exc: + messagebox.showerror("Kommentar konnte nicht gespeichert werden", str(exc), parent=self) + return self.comment_var.set("") self._refresh_events() diff --git a/src/ccma/ui/options_dialog.py b/src/ccma/ui/options_dialog.py index 2083ad1..cc1fe1b 100644 --- a/src/ccma/ui/options_dialog.py +++ b/src/ccma/ui/options_dialog.py @@ -891,25 +891,38 @@ class OptionsDialog(tk.Toplevel): self.config_obj.optional_member_fields = tuple( field for field, variable in self.optional_member_field_vars.items() if variable.get() ) + # Program settings live in the user's config directory and stay writable even + # when the member store does not -- so they are saved either way, and only the + # store-backed settings are skipped (with a notice) on a read-only store. + store_read_only = self.repository.read_only try: self.config_obj.save() - self.repository.save_member_number_policy( - mode=number_mode, - pattern=number_pattern, - allocation_strategy=self.number_strategy_var.get(), - ) - self.repository.save_organization( - {key: variable.get() for key, variable in self.organization_vars.items()} - ) - self.repository.save_reminder_policy( - grace_days_after_due=grace_days_after_due, - levels=self.levels, - standard_fee_items=self.standard_items, - ) - self.repository.save_email_settings(**email_settings) + if not store_read_only: + self.repository.save_member_number_policy( + mode=number_mode, + pattern=number_pattern, + allocation_strategy=self.number_strategy_var.get(), + ) + self.repository.save_organization( + {key: variable.get() for key, variable in self.organization_vars.items()} + ) + self.repository.save_reminder_policy( + grace_days_after_due=grace_days_after_due, + levels=self.levels, + standard_fee_items=self.standard_items, + ) + self.repository.save_email_settings(**email_settings) except (OSError, RepositoryError) as exc: messagebox.showerror("Optionen konnten nicht gespeichert werden", str(exc), parent=self) return + if store_read_only: + messagebox.showwarning( + "Store ist schreibgeschützt", + "Die Programmeinstellungen wurden gespeichert. Vereinsdaten, " + "Mitgliedsnummern, Mahnungen und E-Mail-Versand konnten nicht gespeichert " + "werden, weil der Mitglieder-Store nur lesend eingebunden ist.", + parent=self, + ) if self.on_saved: self.on_saved(store_changed) self.destroy() diff --git a/src/ccma/ui/splash.py b/src/ccma/ui/splash.py index 60b1164..7bebf60 100644 --- a/src/ccma/ui/splash.py +++ b/src/ccma/ui/splash.py @@ -121,7 +121,14 @@ class SplashScreen(tk.Toplevel): self.repository.list_members() member_count = len(self.repository.list_member_ids()) findings = [] - if self.run_housekeeper: + # A read-only store makes the housekeeper impossible, not just + # unwanted: every one of its passes writes. Skipping it keeps the + # rest of the startup -- and the whole read-only session -- working. + if self.repository.read_only: + self._messages.put( + ("status", "Store ist schreibgeschützt – Hausmeister übersprungen …") + ) + elif self.run_housekeeper: self._messages.put(("status", "Starte Hausmeister …")) findings = Housekeeper(self.repository, self.housekeeper_settings).run( member_delay=_member_delay_for_splash( diff --git a/tests/test_read_only_store.py b/tests/test_read_only_store.py new file mode 100644 index 0000000..b595b27 --- /dev/null +++ b/tests/test_read_only_store.py @@ -0,0 +1,171 @@ +import os +import stat +from datetime import date + +import pytest + +from ccma.domain.models import ContributionData +from ccma.services.documents import DocumentService +from ccma.services.housekeeper import Housekeeper +from ccma.services.reminder_mail import generate_and_send_reminder_mail +from ccma.storage.repository import MemberRepository, ReadOnlyStoreError + +# Root ignores the permission bits this test relies on, so the read-only mount it +# stands in for cannot be simulated there. +pytestmark = pytest.mark.skipif( + hasattr(os, "geteuid") and os.geteuid() == 0, reason="root bypasses file permissions" +) + + +def _set_writable(root, writable: bool) -> None: + mode_add = stat.S_IWUSR + for path in sorted(root.rglob("*"), reverse=True) + [root]: + current = stat.S_IMODE(path.stat().st_mode) + path.chmod(current | mode_add if writable else current & ~mode_add) + + +@pytest.fixture +def read_only_store(tmp_path): + root = tmp_path / "store" + repository = MemberRepository(root) + repository.initialize() + member = repository.create_member( + first_name="Ada", last_name="Lovelace", birth_date="1990-01-01" + ) + repository.save_contributions( + member.member_id, + ContributionData( + claims=[ + { + "claim_id": "claim-1", + "claim_key": "overdue", + "title": "Offene Forderung", + "amount": "100.00", + "due_date": "2026-01-31", + "status": "open", + } + ] + ), + ) + _set_writable(root, False) + yield MemberRepository(root), member + _set_writable(root, True) + + +def test_a_writable_store_is_not_reported_as_read_only(tmp_path): + repository = MemberRepository(tmp_path / "store") + repository.initialize() + + assert repository.read_only is False + + +def test_store_directory_that_does_not_exist_yet_counts_as_writable(tmp_path): + assert MemberRepository(tmp_path / "not" / "created" / "yet").read_only is False + + +def test_read_only_store_opens_and_stays_readable(read_only_store): + repository, member = read_only_store + + assert repository.read_only is True + repository.initialize() + assert repository.validate() == [] + assert [item.member_id for item in repository.list_members()] == [member.member_id] + assert repository.get_member(member.member_id).first_name == "Ada" + assert repository.get_contributions(member.member_id).claims + assert repository.get_events(member.member_id) + assert repository.get_configuration()["organization"]["name"] + + +def test_every_write_is_refused_with_one_clear_message(read_only_store): + repository, member = read_only_store + stored = repository.get_member(member.member_id) + + for action in ( + lambda: repository.save_member(stored), + lambda: repository.create_member(first_name="Grace", last_name="Hopper"), + lambda: repository.create_manual_claim( + member.member_id, title="Beitrag", amount="10.00", due_date="2026-09-30" + ), + lambda: repository.save_contributions(member.member_id, ContributionData()), + lambda: repository.append_event( + member.member_id, event_type="comment", summary="Test", actor_type="user" + ), + lambda: repository.save_organization({"name": "Test", "iban": "", "bic": ""}), + lambda: repository.create_asset(label="Beamer"), + ): + with pytest.raises(ReadOnlyStoreError, match="schreibgeschützt"): + action() + + +def test_housekeeper_refuses_to_run_instead_of_failing_on_its_lock(read_only_store): + repository, _member = read_only_store + + with pytest.raises(ReadOnlyStoreError, match="schreibgeschützt"): + Housekeeper(repository).run(today=date(2026, 2, 10)) + with pytest.raises(ReadOnlyStoreError, match="schreibgeschützt"): + Housekeeper(repository).delete_task("some-key") + assert not (repository.root / ".housekeeper.lock").exists() + + +def test_documents_and_mails_refuse_before_touching_the_member_file(read_only_store, tmp_path): + repository, member = read_only_store + + service = DocumentService(repository) + template = next(item for item in service.list_templates() if item.name == "Mitglied") + with pytest.raises(ReadOnlyStoreError, match="schreibgeschützt"): + service.generate(template, member.member_id, output_name="Test") + with pytest.raises(ReadOnlyStoreError, match="schreibgeschützt"): + generate_and_send_reminder_mail( + repository, + member.member_id, + "claim-1", + "reminder-1", + delivery_mode="local", + output_path=tmp_path / "Mahnung.eml", + sender_name="Verwaltung", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + +def test_uninitialized_read_only_store_reports_why_it_cannot_be_opened(tmp_path): + root = tmp_path / "empty-store" + root.mkdir() + root.chmod(stat.S_IRUSR | stat.S_IXUSR) + try: + with pytest.raises(ReadOnlyStoreError, match="repository.json"): + MemberRepository(root).initialize() + finally: + root.chmod(stat.S_IRWXU) + + +def test_remounting_writable_is_picked_up_without_a_restart(read_only_store): + repository, _member = read_only_store + assert repository.read_only is True + + _set_writable(repository.root, True) + + assert repository.read_only is True, "the cached answer must stay put until refreshed" + assert repository.refresh_read_only() is False + repository.append_event( + _member.member_id, + event_type="comment", + summary="Wieder beschreibbar", + actor_type="user", + ) + + +def test_startup_sequence_completes_without_the_housekeeper(read_only_store): + """Mirrors what the splash screen's worker thread does -- this is the sequence + that used to abort the whole start with a PermissionError.""" + repository, member = read_only_store + + repository.initialize() + errors = repository.validate() + members = repository.list_members() + findings = [] if repository.read_only else Housekeeper(repository).run() + + assert errors == [] + assert [item.member_id for item in members] == [member.member_id] + assert findings == [] + assert repository.member_count() == 1 From 528c24ad429eceb4d050accf9a5478a38d5c0acd Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Sat, 29 Aug 2026 00:01:37 +0200 Subject: [PATCH 2/3] Clear every read-only marker when the store becomes writable again MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit "Erneut prüfen" refreshed the banner but left the window title and the status bar reading "NUR LESEN" -- both were built once and never updated -- while the banner itself asked for a restart that the recheck exists to avoid. Title, status bar and banner are refreshed from one place now, so none of them can be left behind, and the main window owns the title instead of app.py setting it once at startup. The banner points at "Erneut prüfen" and says outright that no restart is needed. Since the startup pass was skipped, its task list is empty rather than current, so a successful recheck offers the housekeeper run that fills it. UI tests cover both directions of the recheck; their Tk root moved into a shared conftest fixture, because a second root in another module invalidates the icon images bound to the first. Co-Authored-By: Claude Opus 5 --- README.md | 6 +- src/ccma/app.py | 2 - src/ccma/ui/main_window.py | 56 ++++++++++------- tests/conftest.py | 20 ++++++ tests/test_main_window_ui.py | 105 ++++++++++++++++++++++++++++++++ tests/test_options_dialog_ui.py | 18 +----- 6 files changed, 164 insertions(+), 43 deletions(-) create mode 100644 tests/conftest.py create mode 100644 tests/test_main_window_ui.py diff --git a/README.md b/README.md index c2fb346..a417d0a 100644 --- a/README.md +++ b/README.md @@ -219,8 +219,10 @@ error. Program settings still save normally -- they live in the user's config directory, not in the store. Settings that belong to the store (club data, member numbers, -reminders, e-mail) are skipped with a notice. When the volume is remounted with -write access, "Erneut prüfen" in the banner picks that up without a restart. +reminders, e-mail, mail templates) are skipped with a notice. When the volume is +remounted with write access, "Erneut prüfen" in the banner picks that up without a +restart: title, status bar and banner drop their markers, and the housekeeper pass +that was skipped at startup is offered right away. A store that was never initialized cannot be opened read-only: creating it needs write access, and CCMA says so instead of failing obscurely. diff --git a/src/ccma/app.py b/src/ccma/app.py index 89c7ef2..b5f690d 100644 --- a/src/ccma/app.py +++ b/src/ccma/app.py @@ -78,8 +78,6 @@ class CCMAApp(tk.Tk): def _startup_complete(self, result: StartupResult) -> None: self.deiconify() - if result.repository.read_only: - self.title(f"CCMA · v{__version__} · NUR LESEN (Store schreibgeschützt)") main = MainWindow( self, result.repository, diff --git a/src/ccma/ui/main_window.py b/src/ccma/ui/main_window.py index 7dc3c9a..c3f4128 100644 --- a/src/ccma/ui/main_window.py +++ b/src/ccma/ui/main_window.py @@ -105,7 +105,6 @@ class MainWindow(ttk.Frame): # visible no matter which member, asset or housekeeper tab is in front. self.messages = MessageBannerList(self) self.messages.grid(row=1, column=0, sticky="ew", pady=(10, 0)) - self._refresh_store_messages() self.notebook = ttk.Notebook(self) self.notebook.grid(row=2, column=0, sticky="nsew", pady=(10, 0)) self.tabs = TabManager(self.notebook) @@ -128,46 +127,59 @@ class MainWindow(ttk.Frame): ttk.Label(status, textvariable=self.status_var, style="Status.TLabel").grid( row=0, column=0, sticky="w" ) - store_label = f"STORE {self.repository.root}" - if self.repository.read_only: - store_label = f"{store_label} · NUR LESEN" - ttk.Label( - status, - text=f"{store_label} · VERSION {__version__}", - style="Status.TLabel", - ).grid(row=0, column=1, sticky="e") + self.store_var = tk.StringVar() + ttk.Label(status, textvariable=self.store_var, style="Status.TLabel").grid( + row=0, column=1, sticky="e" + ) + # Everything that reflects write access is refreshed from one place, so a + # store that becomes writable mid-session updates all of it together. + self._refresh_store_state() - def _refresh_store_messages(self) -> None: + def _refresh_store_state(self) -> None: + """Window title, status bar and banner all say whether the store can be + written to -- they are rebuilt together so none of them can be left behind.""" + read_only = self.repository.read_only + title = f"CCMA · v{__version__}" + self.master.title(f"{title} · NUR LESEN (Store schreibgeschützt)" if read_only else title) + store_label = f"STORE {self.repository.root}" + if read_only: + store_label = f"{store_label} · NUR LESEN" + self.store_var.set(f"{store_label} · VERSION {__version__}") messages = [] - if self.repository.read_only: + if read_only: messages.append( TabMessage( "warning", "ACHTUNG: Der Mitglieder-Store ist schreibgeschützt eingebunden. " "Alle Daten sind nur lesbar – Änderungen, Dokumente, E-Mails und der " - "Hausmeister sind deaktiviert. Zum Bearbeiten den Store mit " - "Schreibrechten einbinden und CCMA neu starten.", + "Hausmeister sind deaktiviert. Den Store mit Schreibrechten einbinden " + "und hier auf „Erneut prüfen“ klicken – ein Neustart ist dafür nicht " + "nötig.", MessageAction("Erneut prüfen", self._recheck_store_access), ) ) self.messages.set_messages(messages) def _recheck_store_access(self) -> None: - """Lets the board re-mount the volume without restarting first -- if it came - back writable, the notice disappears and the housekeeper works again.""" + """Lets the board remount the volume without restarting first -- if it came + back writable, every read-only marker goes away and the housekeeper, skipped + at startup, can run right away.""" if self.repository.refresh_read_only(): + self._refresh_store_state() self.status_var.set("Der Store ist weiterhin schreibgeschützt.") - self._refresh_store_messages() return - self._refresh_store_messages() - self._refresh_ribbon_icons() + self._refresh_store_state() self.status_var.set("Der Store ist wieder beschreibbar.") - messagebox.showinfo( + # The startup pass was skipped, so the task list is empty rather than current: + # offer the run that fills it instead of leaving a misleading dashboard. + if messagebox.askyesno( "Store beschreibbar", - "Der Mitglieder-Store ist jetzt mit Schreibrechten eingebunden. " - "Der Hausmeister wurde beim Start übersprungen und kann nun ausgeführt werden.", + "Der Mitglieder-Store ist jetzt mit Schreibrechten eingebunden. Der " + "Hausmeister wurde beim Start übersprungen, seine Vorgangsliste ist daher " + "noch leer.\n\nJetzt einen Hausmeisterlauf starten?", parent=self, - ) + ): + self.run_housekeeper() def _refuse_read_only(self, action: str) -> bool: if not self.repository.read_only: diff --git a/tests/conftest.py b/tests/conftest.py new file mode 100644 index 0000000..8a23002 --- /dev/null +++ b/tests/conftest.py @@ -0,0 +1,20 @@ +import pytest + + +@pytest.fixture(scope="session") +def tk_root(): + """One Tk root for the whole test session, shared by every UI test module: the + icon library binds its images to the first root, so a second one (or a root torn + down between tests) invalidates them with 'image "pyimage1" doesn\'t exist'. + Skips where there is no display, which keeps the suite running on headless CI.""" + tk = pytest.importorskip("tkinter") + from ccma.ui.theme import load_theme + + try: + root = tk.Tk() + except tk.TclError as exc: + pytest.skip(f"kein Display verfügbar: {exc}") + root.withdraw() + load_theme(root, "dark") + yield root + root.destroy() diff --git a/tests/test_main_window_ui.py b/tests/test_main_window_ui.py new file mode 100644 index 0000000..7fbd555 --- /dev/null +++ b/tests/test_main_window_ui.py @@ -0,0 +1,105 @@ +import os +import stat + +import pytest + +pytest.importorskip("tkinter") + +from ccma.config import AppConfig # noqa: E402 +from ccma.storage.repository import MemberRepository # noqa: E402 + +# Root ignores the permission bits that stand in for a read-only mount here. +pytestmark = pytest.mark.skipif( + hasattr(os, "geteuid") and os.geteuid() == 0, reason="root bypasses file permissions" +) + + +def _set_writable(root, writable: bool) -> None: + for path in sorted(root.rglob("*"), reverse=True) + [root]: + mode = stat.S_IMODE(path.stat().st_mode) + path.chmod(mode | stat.S_IWUSR if writable else mode & ~stat.S_IWUSR) + + +@pytest.fixture +def read_only_window(tk_root, tmp_path): + from ccma.ui.main_window import MainWindow + + root_path = tmp_path / "store" + repository = MemberRepository(root_path) + repository.initialize() + repository.create_member(first_name="Ada", last_name="Lovelace", birth_date="1990-01-01") + _set_writable(root_path, False) + + window = MainWindow(tk_root, MemberRepository(root_path), AppConfig(store_path=str(root_path)), [], []) + window.pack(fill="both", expand=True) + tk_root.update() + yield window + window.destroy() + tk_root.title("") + tk_root.update() + _set_writable(root_path, True) + + +def test_a_read_only_store_is_marked_everywhere(read_only_window, tk_root): + assert "NUR LESEN" in tk_root.title() + assert "NUR LESEN" in read_only_window.store_var.get() + assert read_only_window.messages.winfo_manager(), "der Warnbanner fehlt" + + +def test_the_banner_does_not_ask_for_a_restart(read_only_window): + banner = read_only_window.messages.winfo_children()[0] + text = " ".join( + str(child.cget("text")) + for child in banner.winfo_children()[0].winfo_children() + if "text" in child.keys() + ) + # The banner used to end with "und CCMA neu starten", which is wrong -- the + # session recovers in place. + assert "Erneut prüfen" in text + assert "neu starten" not in text.casefold() + assert "Neustart ist dafür nicht nötig" in text + + +def test_a_remounted_store_clears_every_read_only_marker(read_only_window, tk_root, monkeypatch): + from tkinter import messagebox + + _set_writable(read_only_window.repository.root, True) + asked = [] + monkeypatch.setattr( + messagebox, "askyesno", lambda *args, **kwargs: asked.append(args) or False + ) + + read_only_window._recheck_store_access() + tk_root.update() + + assert read_only_window.repository.read_only is False + assert "NUR LESEN" not in tk_root.title() + assert "NUR LESEN" not in read_only_window.store_var.get() + assert not read_only_window.messages.winfo_manager(), "der Warnbanner steht noch" + assert read_only_window.status_var.get() == "Der Store ist wieder beschreibbar." + assert asked, "der übersprungene Hausmeisterlauf wird nicht angeboten" + + +def test_the_skipped_housekeeper_run_can_be_started_right_away( + read_only_window, tk_root, monkeypatch +): + from tkinter import messagebox + + _set_writable(read_only_window.repository.root, True) + monkeypatch.setattr(messagebox, "askyesno", lambda *args, **kwargs: True) + + read_only_window._recheck_store_access() + tk_root.update() + + assert (read_only_window.repository.root / "housekeeper.json").is_file() + assert read_only_window.status_var.get().startswith("Hausmeisterlauf beendet") + + +def test_a_store_that_is_still_read_only_keeps_its_markers(read_only_window, tk_root): + read_only_window._recheck_store_access() + tk_root.update() + + assert "NUR LESEN" in tk_root.title() + assert "NUR LESEN" in read_only_window.store_var.get() + assert read_only_window.messages.winfo_manager() + assert read_only_window.status_var.get() == "Der Store ist weiterhin schreibgeschützt." diff --git a/tests/test_options_dialog_ui.py b/tests/test_options_dialog_ui.py index c7bea97..860ae94 100644 --- a/tests/test_options_dialog_ui.py +++ b/tests/test_options_dialog_ui.py @@ -3,7 +3,7 @@ from email.policy import default import pytest -tk = pytest.importorskip("tkinter") +pytest.importorskip("tkinter") from ccma.config import AppConfig # noqa: E402 from ccma.domain.mail_templates import placeholder_entries # noqa: E402 @@ -11,22 +11,6 @@ from ccma.services.welcome_mail import generate_and_send_welcome_mail # noqa: E from ccma.storage.repository import MemberRepository, RepositoryError # noqa: E402 -# One root for the whole session: the icon library binds its images to the first -# Tk instance, so tearing a root down between tests would invalidate them. -@pytest.fixture(scope="session") -def tk_root(): - from ccma.ui.theme import load_theme - - try: - root = tk.Tk() - except tk.TclError as exc: # headless CI has no display to build widgets on - pytest.skip(f"kein Display verfügbar: {exc}") - root.withdraw() - load_theme(root, "dark") - yield root - root.destroy() - - @pytest.fixture def repository(tmp_path): repository = MemberRepository(tmp_path / "store") From e87f8591011d1db9aa6cc0254614eea231624085 Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Sat, 29 Aug 2026 01:09:05 +0200 Subject: [PATCH 3/3] Refuse the welcome mail on a read-only store before it renders anything generate_and_send_welcome_mail() arrived with the mail templates, after the read-only guards were added to the other services, and never got one. On a read-only store it therefore rendered the mail, could hand it to the mail server, and only failed when it tried to create the archive directory in the member file -- surfacing a PermissionError instead of the ReadOnlyStoreError every other write path reports. The guard now sits at the top, next to the delivery-mode check, so nothing is rendered, sent or written. The read-only test covers this path (and the SEPA batch alongside it) and asserts that nothing at all was left behind: no export file, no archive directory, no "sent" event. Its member carries an e-mail address now -- without one the mail services bail out for that reason, and the write the test exists for is never reached. Note that the SEPA CSV/XML export keeps writing without a guard on purpose: it writes to a path the board picks outside the store, which a read-only store has no say over. Co-Authored-By: Claude Opus 5 --- src/ccma/services/welcome_mail.py | 3 ++ tests/test_read_only_store.py | 54 +++++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+) diff --git a/src/ccma/services/welcome_mail.py b/src/ccma/services/welcome_mail.py index 6ef0d41..08c95f3 100644 --- a/src/ccma/services/welcome_mail.py +++ b/src/ccma/services/welcome_mail.py @@ -141,6 +141,9 @@ def generate_and_send_welcome_mail( ) -> GeneratedWelcomeMail: if delivery_mode not in {"local", "send", "drafts"}: raise RepositoryError(f"Unbekannter Versandmodus: {delivery_mode}") + # The mail is archived in the member file, so a read-only store rules the whole + # path out -- said here, before anything is rendered or handed to a mail server. + repository.assert_writable() member = repository.get_member(member_id) if not member.email.strip(): raise RepositoryError("Für das Mitglied ist keine E-Mail-Adresse hinterlegt.") diff --git a/tests/test_read_only_store.py b/tests/test_read_only_store.py index 49b0335..534f092 100644 --- a/tests/test_read_only_store.py +++ b/tests/test_read_only_store.py @@ -8,6 +8,8 @@ from ccma.domain.models import ContributionData from ccma.services.documents import DocumentService from ccma.services.housekeeper import Housekeeper from ccma.services.reminder_mail import generate_and_send_reminder_mail +from ccma.services.sepa_mail import generate_debit_mails +from ccma.services.welcome_mail import generate_and_send_welcome_mail from ccma.storage.repository import MemberRepository, ReadOnlyStoreError # Root ignores the permission bits this test relies on, so the read-only mount it @@ -32,6 +34,11 @@ def read_only_store(tmp_path): member = repository.create_member( first_name="Ada", last_name="Lovelace", birth_date="1990-01-01" ) + # A complete member: without an address the mail services would bail out early + # for that reason, and the write they are actually being tested for is never + # reached. + member.email = "ada@example.org" + repository.save_member(member) repository.save_contributions( member.member_id, ContributionData( @@ -126,6 +133,53 @@ def test_documents_and_mails_refuse_before_touching_the_member_file(read_only_st sender_email="verwaltung@example.org", signature="Der Vorstand", ) + with pytest.raises(ReadOnlyStoreError, match="schreibgeschützt"): + generate_and_send_welcome_mail( + repository, + member.member_id, + delivery_mode="local", + output_path=tmp_path / "Willkommen.eml", + sender_name="Verwaltung", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + with pytest.raises(ReadOnlyStoreError, match="schreibgeschützt"): + generate_debit_mails( + repository, + [], + collection_date=date(2026, 9, 1), + delivery_mode="local", + output_directory=tmp_path / "sepa", + sender_name="Verwaltung", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + +def test_the_welcome_mail_writes_nothing_at_all_when_refused(read_only_store, tmp_path): + """The refusal has to come before rendering, sending and archiving -- not out of + the failing write at the end, by which point the mail would already be out.""" + repository, member = read_only_store + export_path = tmp_path / "Willkommen.eml" + + with pytest.raises(ReadOnlyStoreError, match="schreibgeschützt"): + generate_and_send_welcome_mail( + repository, + member.member_id, + delivery_mode="local", + output_path=export_path, + sender_name="Verwaltung", + sender_email="verwaltung@example.org", + signature="Der Vorstand", + ) + + assert not export_path.exists() + files = repository.members_root / member.member_id / "files" + assert not (files / "documents" / "Willkommen").exists() + assert all( + event.event_type != "welcome_email_sent" + for event in repository.get_events(member.member_id) + ) def test_uninitialized_read_only_store_reports_why_it_cannot_be_opened(tmp_path):