From 528c24ad429eceb4d050accf9a5478a38d5c0acd Mon Sep 17 00:00:00 2001 From: Marcel Peterkau Date: Sat, 29 Aug 2026 00:01:37 +0200 Subject: [PATCH] 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")