diff --git a/.gitignore b/.gitignore index 58a3364..19996c9 100644 --- a/.gitignore +++ b/.gitignore @@ -50,3 +50,6 @@ tmp/ # Transient pre-modification backups backups/*.bak + +# Local SonarCloud API token (never commit) +.sonar_cloud_token diff --git a/CHANGELOG.md b/CHANGELOG.md index 9347302..370e9dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,7 @@ This project follows the structure from Keep a Changelog and intends to use Sema ### Changed +- **Refactored repeated string literals** (GUI selectors, HTML fragments, the `reports.` TOML prefix, the Git LFS label, and the workflows pathspec) into module-level constants to clear SonarCloud high-severity S1192/S7688 issues. - **GitHub Actions workflow now defaults to `--email-format html`:** The committed `email-report.yml` and its template were updated from `--email-format text` to `--email-format html`. Existing users who re-run setup (option 5 / wizard) will have their workflow re-rendered with this default. To keep plain-text output, set `email_format = "text"` in `.github-usage/config.toml` before re-running setup, or select **Text** in the wizard. - **Terminology:** the interactive CLI/TUI usage report is now called the **local full report** in user-facing copy (README, `start.sh`, CLI help, TUI). Internal `legacy_*` module names are unchanged for now (tracked in `TO_DO.md`). - **GitHub Actions `setup-python` v7:** Bump `actions/setup-python` from v6 to v7 in CI, security, email-report, and the email-report template (folds in Dependabot #7; no workflow input changes — this repo does not use the removed `pip-install` input). diff --git a/src/github_usage/cli_runs_diff.py b/src/github_usage/cli_runs_diff.py index 9cc47fa..47fe8c6 100644 --- a/src/github_usage/cli_runs_diff.py +++ b/src/github_usage/cli_runs_diff.py @@ -22,6 +22,9 @@ # stable sort order in porcelain output regardless of the user's locale. GIT_ENV: dict[str, str] = {**os.environ, "LC_ALL": "C"} +# Directory holding the email-report workflow files; used as a git pathspec. +_WORKFLOWS_DIR = ".github/workflows/" + # Valid drift categories. Asserted by tests. DRIFT_CATEGORIES: frozenset[str] = frozenset( { @@ -191,7 +194,7 @@ def _normalize_path(p: str | Path, repo_root: Path) -> str: return pp.as_posix() -def _git_status_porcelain(repo_root: Path, pathspec: str = ".github/workflows/") -> dict[str, str]: +def _git_status_porcelain(repo_root: Path, pathspec: str = _WORKFLOWS_DIR) -> dict[str, str]: """Run ``git status --porcelain=v1 -- `` and return ``{path: line}``. Skipped (returns ``{}``) if the subprocess fails for any reason; @@ -302,7 +305,7 @@ def _list_local_paths(repo_root: Path) -> list[str]: paths: set[str] = set() try: proc = _run_git( - ["ls-tree", "-r", "HEAD", "--", ".github/workflows/"], + ["ls-tree", "-r", "HEAD", "--", _WORKFLOWS_DIR], cwd=repo_root, ) if proc.returncode == 0: @@ -351,7 +354,7 @@ def _list_remote_paths(repo_root: Path, remote: str, default_branch: str | None) "-r", f"{remote}/{default_branch}", "--", - ".github/workflows/", + _WORKFLOWS_DIR, ], cwd=repo_root, ) diff --git a/src/github_usage/email_report_html.py b/src/github_usage/email_report_html.py index b274f60..a002132 100644 --- a/src/github_usage/email_report_html.py +++ b/src/github_usage/email_report_html.py @@ -22,6 +22,11 @@ visibility_group_header, ) +# Repeated HTML fragments, hoisted to satisfy S1192 and keep tag spelling in one place. +_UL_CLOSE = "" +_TABLE_OPEN = "" +_TABLE_CLOSE = "
" + def _html_cost_row(label: str, cost: dict[str, float]) -> str: return ( @@ -42,7 +47,7 @@ def _format_html_billing_context_section(data: dict) -> list[str]: "
  • Actions minutes and storage are free for public repositories.
    • ", "
  • Private and internal repositories consume your plan's monthly quota.
    • ", f'
  • See GitHub Actions billing for details.
    • ', - "", + _UL_CLOSE, ] @@ -53,7 +58,7 @@ def _format_html_actions_section(data: dict) -> list[str]: net = (data.get("monthly_costs") or {}).get("actions", {}).get("net", 0.0) return [ "

    Actions

    ", - "", + _TABLE_OPEN, "", ( f"" ), f"", - "
    MetricValue
    Minutes{actions.get('minutes', 0.0):,.1f} / " @@ -66,7 +71,7 @@ def _format_html_actions_section(data: dict) -> list[str]: f"({actions.get('storage_percent', 0.0):.1f}%)
    Net cost{fmt_price(net)}
    ", + _TABLE_CLOSE, ] @@ -82,13 +87,13 @@ def _format_html_copilot_section(data: dict) -> list[str]: ] by_model = copilot.get("by_model") or {} if by_model: - parts.append("") + parts.append(_UL_CLOSE) parts.append("

    By model

    ") parts.append("") + parts.append(_UL_CLOSE) return parts @@ -116,10 +121,10 @@ def _format_html_monthly_costs_section(data: dict) -> list[str]: rows.append(_html_cost_row(label, monthly.get(key, {}))) return [ "

    Monthly Cost Estimate

    ", - "", + _TABLE_OPEN, "", *rows, - "
    CategoryGrossDiscountNet
    ", + _TABLE_CLOSE, ] @@ -225,7 +230,7 @@ def _format_html_artifact_storage_section(data: dict) -> list[str]: f"
  • {html_repo_cell(row)}: " f"{_bytes_to_mb(row['artifact_bytes']):,.1f} MB artifacts
    • " ) - parts.append("") + parts.append(_UL_CLOSE) else: for vis, group_rows in groups.items(): parts.append(f"

    {html.escape(visibility_group_header(vis))}

    ") @@ -235,7 +240,7 @@ def _format_html_artifact_storage_section(data: dict) -> list[str]: f"
  • {html_repo_cell(row)}: " f"{_bytes_to_mb(row['artifact_bytes']):,.1f} MB artifacts
    • " ) - parts.append("") + parts.append(_UL_CLOSE) if artifact_storage.get("truncated"): parts.append( f"

    Artifact scan truncated at " @@ -258,7 +263,7 @@ def _format_html_release_assets_section(data: dict) -> list[str]: f"

  • {html_repo_cell(row)}: " f"{_bytes_to_mb(row['release_asset_bytes']):,.1f} MB release assets
    • " ) - parts.append("") + parts.append(_UL_CLOSE) else: for vis, group_rows in groups.items(): parts.append(f"

    {html.escape(visibility_group_header(vis))}

    ") @@ -268,7 +273,7 @@ def _format_html_release_assets_section(data: dict) -> list[str]: f"
  • {html_repo_cell(row)}: " f"{_bytes_to_mb(row['release_asset_bytes']):,.1f} MB release assets
    • " ) - parts.append("") + parts.append(_UL_CLOSE) if release_assets.get("truncated"): parts.append( f"

    Release asset scan truncated at " @@ -285,7 +290,7 @@ def _format_html_insights_section(data: dict) -> list[str]: "

    Key Insights

    ", "
      ", *[f"
    • {html.escape(insight)}
      • " for insight in insights], - "
    ", + _UL_CLOSE, ] @@ -299,7 +304,7 @@ def _format_html_errors_section(data: dict) -> list[str]: f"
  • {html.escape(section.replace('_', ' ').title())} " f"data unavailable - {html.escape(message)}
    • " ) - parts.append("") + parts.append(_UL_CLOSE) return parts @@ -351,7 +356,7 @@ def _run_out(value: int | None) -> str: parts = [ f"

    Monthly Forecast{scope_note}

    ", f"

    Day {forecast['day_of_month']} of {forecast['days_in_month']}

    ", - "", + _TABLE_OPEN, "", ] for label, metric in rows: @@ -364,7 +369,7 @@ def _run_out(value: int | None) -> str: f"" "" ) - parts.append("
    MetricCurrentProjectedLimitRun-out
    {_run_out(metric['run_out_day'])}
    ") + parts.append(_TABLE_CLOSE) if has_split: note = _public_repos_html_note(forecast) if note: @@ -431,7 +436,7 @@ def _html_api_notes_block(estimate: dict) -> list[str]: parts = ["

    REST API Quota Notes

    ", "
      "] for note in notes: parts.append(f"
    • {html.escape(note)}
      • ") - parts.append("
    ") + parts.append(_UL_CLOSE) return parts @@ -451,7 +456,7 @@ def _html_sources_block(sources: dict) -> list[str]: f"
  • {html.escape(label)}: " f'{html.escape(str(url))}
    • ' ) - parts.append("") + parts.append(_UL_CLOSE) return parts diff --git a/src/github_usage/gui/views/email_report_view.py b/src/github_usage/gui/views/email_report_view.py index 5c7a333..419758e 100644 --- a/src/github_usage/gui/views/email_report_view.py +++ b/src/github_usage/gui/views/email_report_view.py @@ -13,6 +13,9 @@ from ..layout import FormGrid, ViewActions, ViewOutput, ViewSection from ..log_utils import write_log +# Repeated selector/label literals, hoisted to satisfy S1192. +_EMAIL_PREVIEW = "#email-preview" + class EmailReportView(VerticalScroll, AsyncViewMixin): """Dry-run preview and send for email reports.""" @@ -55,7 +58,7 @@ def _on_state_changed(self) -> None: self._reload_profiles() def _show_error(self, message: str) -> None: - preview = self.query_one("#email-preview", RichLog) + preview = self.query_one(_EMAIL_PREVIEW, RichLog) write_log(preview, format_simple(message), level="error") def _finish_reload(self) -> None: @@ -102,7 +105,7 @@ def action_preview(self) -> None: @work(thread=True) def _run_preview(self) -> None: button = self.query_one("#preview-btn", Button) - preview = self.query_one("#email-preview", RichLog) + preview = self.query_one(_EMAIL_PREVIEW, RichLog) self._call_ui( self._begin_async, button, @@ -133,7 +136,7 @@ def _run_preview(self) -> None: self._call_ui(self._end_async, button, "Preview (dry-run)") def _show_preview(self, code: int, body: str) -> None: - preview = self.query_one("#email-preview", RichLog) + preview = self.query_one(_EMAIL_PREVIEW, RichLog) preview.clear() if body.strip(): preview.write(body.rstrip()) @@ -156,7 +159,7 @@ def _send(self) -> None: @work(thread=True) def _run_send(self) -> None: button = self.query_one("#send-btn", Button) - preview = self.query_one("#email-preview", RichLog) + preview = self.query_one(_EMAIL_PREVIEW, RichLog) self._call_ui( self._begin_async, button, @@ -186,7 +189,7 @@ def _run_send(self) -> None: self._call_ui(self._end_async, button, "Send Email") def _show_send_result(self, code: int, message: str) -> None: - preview = self.query_one("#email-preview", RichLog) + preview = self.query_one(_EMAIL_PREVIEW, RichLog) if code == 0: write_log(preview, message, level="success") else: diff --git a/src/github_usage/gui/views/report_view.py b/src/github_usage/gui/views/report_view.py index d63689e..c995997 100644 --- a/src/github_usage/gui/views/report_view.py +++ b/src/github_usage/gui/views/report_view.py @@ -28,6 +28,9 @@ from ..layout import FormGrid, ViewActions, ViewOutput, ViewSection from ..log_utils import write_log +# Repeated selector/label literals, hoisted to satisfy S1192. +_REPORT_LOG = "#report-log" + @dataclass(frozen=True) class _ReportRunParams: @@ -105,7 +108,7 @@ def _run_report(self) -> None: if params is None: return button = self.query_one("#run-report", Button) - log = self.query_one("#report-log", RichLog) + log = self.query_one(_REPORT_LOG, RichLog) self._fetch_report(params, button, log) def action_run_report(self) -> None: @@ -113,12 +116,12 @@ def action_run_report(self) -> None: params = self._read_run_params() if params is not None: button = self.query_one("#run-report", Button) - log = self.query_one("#report-log", RichLog) + log = self.query_one(_REPORT_LOG, RichLog) self._fetch_report(params, button, log) def _read_run_params(self) -> _ReportRunParams | None: """Validate form fields on the UI thread.""" - log = self.query_one("#report-log", RichLog) + log = self.query_one(_REPORT_LOG, RichLog) timeout_str = self.query_one("#timeout", Input).value.strip() or "30" max_retries_str = self.query_one("#max-retries", Input).value.strip() or "3" @@ -245,7 +248,7 @@ def _show_report_result( username_or_err: str | None, cache_hit, ) -> None: - log = self.query_one("#report-log", RichLog) + log = self.query_one(_REPORT_LOG, RichLog) table = self.query_one("#summary-table", DataTable) table.clear() diff --git a/src/github_usage/gui/views/runs_view.py b/src/github_usage/gui/views/runs_view.py index 86763a2..d7f2ff9 100644 --- a/src/github_usage/gui/views/runs_view.py +++ b/src/github_usage/gui/views/runs_view.py @@ -13,6 +13,9 @@ from ..layout import ViewActions, ViewOutput, ViewSection from ..log_utils import write_log +# Repeated selector/label literals, hoisted to satisfy S1192. +_RUNS_LOG = "#runs-log" + class RunsView(VerticalScroll, AsyncViewMixin): """Read-only runs table and drift checker.""" @@ -59,17 +62,17 @@ def _load_runs(self) -> None: row["schedule"], ) except Exception as exc: - log = self.query_one("#runs-log", RichLog) + log = self.query_one(_RUNS_LOG, RichLog) write_log(log, format_error(exc, context="Failed to load runs"), level="error") @on(Button.Pressed, "#refresh-runs") def _refresh(self) -> None: self._load_runs() - write_log(self.query_one("#runs-log", RichLog), "Runs refreshed", level="success") + write_log(self.query_one(_RUNS_LOG, RichLog), "Runs refreshed", level="success") def action_refresh_runs(self) -> None: self._load_runs() - write_log(self.query_one("#runs-log", RichLog), "Runs refreshed", level="success") + write_log(self.query_one(_RUNS_LOG, RichLog), "Runs refreshed", level="success") @on(Button.Pressed, "#check-drift") def _check_drift(self) -> None: @@ -80,7 +83,7 @@ def _check_drift(self) -> None: @work(thread=True) def _run_drift(self) -> None: button = self.query_one("#check-drift", Button) - log = self.query_one("#runs-log", RichLog) + log = self.query_one(_RUNS_LOG, RichLog) self._call_ui( self._begin_async, button, @@ -104,7 +107,7 @@ def _run_drift(self) -> None: self._call_ui(self._end_async, button, "Check drift") def _show_drift(self, result) -> None: - log = self.query_one("#runs-log", RichLog) + log = self.query_one(_RUNS_LOG, RichLog) for message in result.messages: log.write(message) table = self.query_one("#drift-table", DataTable) diff --git a/src/github_usage/gui/views/schedules_view.py b/src/github_usage/gui/views/schedules_view.py index ef4c6d7..302892a 100644 --- a/src/github_usage/gui/views/schedules_view.py +++ b/src/github_usage/gui/views/schedules_view.py @@ -25,6 +25,12 @@ from ..modals import ConfirmScreen from ..widgets.schedule_picker import SchedulePicker +# Repeated selector/label literals, hoisted to satisfy S1192. +_SCHED_LOG = "#sched-log" +_PROFILE_SELECT = "#profile-select" +_SCHEDULE_PICKER = "#schedule-picker" +_INSTALL_LA_LABEL = "Install LaunchAgent" + class SchedulesView(VerticalScroll, AsyncViewMixin): """Local launchd and GitHub Actions schedule forms.""" @@ -85,7 +91,7 @@ def compose(self) -> ComposeResult: yield Button("Regenerate plist", id="regen-plist") yield Button("Regenerate workflow", id="regen-workflow") if sys.platform == "darwin": - yield Button("Install LaunchAgent", id="install-la", variant="success") + yield Button(_INSTALL_LA_LABEL, id="install-la", variant="success") if sys.platform != "darwin": yield Static( "Local launchd scheduling is macOS-only. " @@ -109,19 +115,19 @@ def on_mount(self) -> None: self._on_state_changed() except FileNotFoundError: write_log( - self.query_one("#sched-log", RichLog), + self.query_one(_SCHED_LOG, RichLog), "Config file not found. Run setup first.", level="error", ) except PermissionError: write_log( - self.query_one("#sched-log", RichLog), + self.query_one(_SCHED_LOG, RichLog), "Permission denied reading config.", level="error", ) except Exception as exc: write_log( - self.query_one("#sched-log", RichLog), + self.query_one(_SCHED_LOG, RichLog), format_error(exc, context="Failed to load schedules"), level="error", ) @@ -144,7 +150,7 @@ def _clear_dirty(self) -> None: self.query_one("#dirty-indicator", Static).update("") def _profile_name(self) -> str: - value = self.query_one("#profile-select", Select).value + value = self.query_one(_PROFILE_SELECT, Select).value if value == Select.BLANK or str(value) == "Select.NULL" or value is None: return self.app.app_state.current_profile # type: ignore[attr-defined] return str(value) @@ -155,7 +161,7 @@ def _reload_form(self) -> None: state = self.app.app_state # type: ignore[attr-defined] paths = state.paths names = list(state.profile_names) - select = self.query_one("#profile-select", Select) + select = self.query_one(_PROFILE_SELECT, Select) label = self.query_one("#profile-active-label", Static) if not names: select.set_options([]) @@ -173,7 +179,7 @@ def _reload_form(self) -> None: profile = next(p for p in config["profiles"] if p["name"] == target) sched = profile["schedule"] ga = profile["github_actions"] - picker = self.query_one("#schedule-picker", SchedulePicker) + picker = self.query_one(_SCHEDULE_PICKER, SchedulePicker) picker.set_loading(True) try: picker.set_local_schedule( @@ -211,7 +217,7 @@ def _apply_selection() -> None: self._loading = False raise - @on(Select.Changed, "#profile-select") + @on(Select.Changed, _PROFILE_SELECT) def _profile_changed(self) -> None: if self._loading or self._applying_selection: return @@ -231,7 +237,7 @@ async def _confirm_profile_switch(self) -> None: ) ) if not confirmed: - select = self.query_one("#profile-select", Select) + select = self.query_one(_PROFILE_SELECT, Select) if self._previous_profile: self._applying_selection = True try: @@ -246,7 +252,7 @@ async def _confirm_profile_switch(self) -> None: def _on_schedule_picker_changed(self, event: SchedulePicker.Changed) -> None: if self._loading: return - picker = self.query_one("#schedule-picker", SchedulePicker) + picker = self.query_one(_SCHEDULE_PICKER, SchedulePicker) if getattr(event, "picker", None) is not picker: return self._mark_dirty() @@ -259,7 +265,7 @@ def _on_field_changed(self) -> None: self._mark_dirty() def _validate_schedule_fields(self, log: RichLog) -> bool: - picker = self.query_one("#schedule-picker", SchedulePicker) + picker = self.query_one(_SCHEDULE_PICKER, SchedulePicker) error = picker.validate() if error: write_log(log, error, level="error") @@ -271,11 +277,11 @@ def _save_schedules(self) -> None: self.action_save_schedules() def action_save_schedules(self) -> None: - log = self.query_one("#sched-log", RichLog) + log = self.query_one(_SCHED_LOG, RichLog) if not self._validate_schedule_fields(log): return paths = self.app.app_state.paths # type: ignore[attr-defined] - picker = self.query_one("#schedule-picker", SchedulePicker) + picker = self.query_one(_SCHEDULE_PICKER, SchedulePicker) local = picker.get_local_schedule() cron = picker.get_ga_cron() if local is None or cron is None: @@ -307,7 +313,7 @@ def action_save_schedules(self) -> None: def _regen_plist(self) -> None: if sys.platform != "darwin": return - log = self.query_one("#sched-log", RichLog) + log = self.query_one(_SCHED_LOG, RichLog) try: path = regenerate_launchd_plist(self.app.app_state.paths, self._profile_name()) # type: ignore[attr-defined] write_log(log, f"Generated {path}", level="success") @@ -320,7 +326,7 @@ def _regen_workflow_pressed(self) -> None: @work async def _confirm_regen_workflow(self) -> None: - log = self.query_one("#sched-log", RichLog) + log = self.query_one(_SCHED_LOG, RichLog) confirmed = await self.app.push_screen_wait( ConfirmScreen( "Regenerate workflow file? This overwrites the existing workflow on disk.", @@ -346,7 +352,7 @@ async def _confirm_install_la(self) -> None: confirmed = await self.app.push_screen_wait( ConfirmScreen( "Install or update the LaunchAgent plist for this profile?", - title="Install LaunchAgent", + title=_INSTALL_LA_LABEL, ) ) if confirmed: @@ -355,7 +361,7 @@ async def _confirm_install_la(self) -> None: @work(thread=True) def _install_la(self) -> None: button = self.query_one("#install-la", Button) - log = self.query_one("#sched-log", RichLog) + log = self.query_one(_SCHED_LOG, RichLog) self._call_ui( self._begin_async, button, @@ -378,4 +384,4 @@ def _install_la(self) -> None: except Exception as exc: self._call_ui(write_log, log, format_error(exc), level="error") finally: - self._call_ui(self._end_async, button, "Install LaunchAgent") + self._call_ui(self._end_async, button, _INSTALL_LA_LABEL) diff --git a/src/github_usage/gui/views/setup_profiles_panel.py b/src/github_usage/gui/views/setup_profiles_panel.py index 4c12810..5619a94 100644 --- a/src/github_usage/gui/views/setup_profiles_panel.py +++ b/src/github_usage/gui/views/setup_profiles_panel.py @@ -19,6 +19,11 @@ from .setup_view import SetupView +# Repeated selector/label literals, hoisted to satisfy S1192. +_ONLY_PUBLIC = "#only-public" +_ONLY_PRIVATE = "#only-private" + + class SetupProfilesPanel(VerticalScroll): """Named profiles, report options, and save.""" @@ -135,8 +140,8 @@ def reload_profile_options(self, profile: dict[str, Any]) -> None: self.query_one("#include-release", Checkbox).value = bool( email.get("include_release_assets") ) - self.query_one("#only-public", Checkbox).value = bool(email.get("only_public")) - self.query_one("#only-private", Checkbox).value = bool(email.get("only_private")) + self.query_one(_ONLY_PUBLIC, Checkbox).value = bool(email.get("only_public")) + self.query_one(_ONLY_PRIVATE, Checkbox).value = bool(email.get("only_private")) self.query_one("#max-repos", Input).value = str(email.get("max_repos", 100)) self.query_one("#target-email", Input).value = profile.get("target_email", "") @@ -148,8 +153,8 @@ def read_profile_options(self) -> dict[str, Any]: "include_consumers": self.query_one("#include-consumers", Checkbox).value, "include_artifact_storage": self.query_one("#include-artifact", Checkbox).value, "include_release_assets": self.query_one("#include-release", Checkbox).value, - "only_public": self.query_one("#only-public", Checkbox).value, - "only_private": self.query_one("#only-private", Checkbox).value, + "only_public": self.query_one(_ONLY_PUBLIC, Checkbox).value, + "only_private": self.query_one(_ONLY_PRIVATE, Checkbox).value, "max_repos_str": max_repos_str, "target_email": self.query_one("#target-email", Input).value.strip(), } @@ -187,9 +192,9 @@ def _on_visibility_filter_changed(self, event: Checkbox.Changed) -> None: if self._coordinator.is_form_loading(): return if event.checkbox.id == "only-public" and event.value: - self.query_one("#only-private", Checkbox).value = False + self.query_one(_ONLY_PRIVATE, Checkbox).value = False elif event.checkbox.id == "only-private" and event.value: - self.query_one("#only-public", Checkbox).value = False + self.query_one(_ONLY_PUBLIC, Checkbox).value = False self._coordinator.mark_dirty() @on(Input.Changed) diff --git a/src/github_usage/gui/views/setup_verify_panel.py b/src/github_usage/gui/views/setup_verify_panel.py index f7515f4..0f1b842 100644 --- a/src/github_usage/gui/views/setup_verify_panel.py +++ b/src/github_usage/gui/views/setup_verify_panel.py @@ -17,6 +17,10 @@ from .setup_view import SetupView +# Repeated selector/label literals, hoisted to satisfy S1192. +_VERIFY_LOG = "#verify-log" + + class SetupVerifyPanel(VerticalScroll): """Dry-run verification and configuration status.""" @@ -50,12 +54,12 @@ def show_error(self, message: str) -> None: status = self.query_one("#status-panel", Static) status.update(f"[red]Error: {message}[/red]") - log = self.query_one("#verify-log", RichLog) + log = self.query_one(_VERIFY_LOG, RichLog) write_log(log, format_simple(message), level="error") def show_verify_result(self, result: VerifyResult) -> None: """Render dry-run verification output.""" - log = self.query_one("#verify-log", RichLog) + log = self.query_one(_VERIFY_LOG, RichLog) if result.output.strip(): log.write(result.output.rstrip()) if result.exit_code == 0: @@ -70,7 +74,7 @@ def show_verify_result(self, result: VerifyResult) -> None: @property def log(self) -> RichLog: # type: ignore[override] - return self.query_one("#verify-log", RichLog) + return self.query_one(_VERIFY_LOG, RichLog) @on(Button.Pressed, "#verify-btn") def _verify_pressed(self) -> None: diff --git a/src/github_usage/gui/wizard/setup_wizard_screen.py b/src/github_usage/gui/wizard/setup_wizard_screen.py index 2c9e712..6eae58e 100644 --- a/src/github_usage/gui/wizard/setup_wizard_screen.py +++ b/src/github_usage/gui/wizard/setup_wizard_screen.py @@ -30,6 +30,13 @@ validate_secrets, ) +# Repeated selector/label literals, hoisted to satisfy S1192. +_WIZ_LOCAL_PICKER = "#wizard-local-picker" +_WIZ_GA_PICKER = "#wizard-ga-picker" +_WIZ_ONLY_PUBLIC = "#wizard-only-public" +_WIZ_ONLY_PRIVATE = "#wizard-only-private" +_WIZ_VERIFY_LOG = "#wizard-verify-log" + class SetupWizardScreen(ModalScreen[bool]): """Multi-step first-run and guided setup. Dismisses True when completed.""" @@ -172,13 +179,13 @@ def on_mount(self) -> None: self._data = load_initial_data(paths) self._populate_secrets() self._populate_options() - local_picker = self.query_one("#wizard-local-picker", SchedulePicker) + local_picker = self.query_one(_WIZ_LOCAL_PICKER, SchedulePicker) local_picker.set_local_schedule( self._data.local_weekday, self._data.local_hour, self._data.local_minute, ) - ga_picker = self.query_one("#wizard-ga-picker", SchedulePicker) + ga_picker = self.query_one(_WIZ_GA_PICKER, SchedulePicker) ga_picker.set_ga_cron(self._data.ga_cron) self._update_progress() @@ -191,8 +198,8 @@ def _populate_options(self) -> None: self.query_one("#wizard-consumers", Checkbox).value = self._data.include_consumers self.query_one("#wizard-artifact", Checkbox).value = self._data.include_artifact_storage self.query_one("#wizard-release", Checkbox).value = self._data.include_release_assets - self.query_one("#wizard-only-public", Checkbox).value = self._data.only_public - self.query_one("#wizard-only-private", Checkbox).value = self._data.only_private + self.query_one(_WIZ_ONLY_PUBLIC, Checkbox).value = self._data.only_public + self.query_one(_WIZ_ONLY_PRIVATE, Checkbox).value = self._data.only_private self.query_one("#wizard-max-repos", Input).value = str(self._data.max_repos) self.query_one("#wizard-email-format", Select).value = self._data.email_format self.query_one("#wizard-target-email", Input).value = self._data.target_email @@ -213,14 +220,14 @@ def _read_options_from_form(self) -> None: self._data.include_consumers = self.query_one("#wizard-consumers", Checkbox).value self._data.include_artifact_storage = self.query_one("#wizard-artifact", Checkbox).value self._data.include_release_assets = self.query_one("#wizard-release", Checkbox).value - self._data.only_public = self.query_one("#wizard-only-public", Checkbox).value - self._data.only_private = self.query_one("#wizard-only-private", Checkbox).value + self._data.only_public = self.query_one(_WIZ_ONLY_PUBLIC, Checkbox).value + self._data.only_private = self.query_one(_WIZ_ONLY_PRIVATE, Checkbox).value fmt = self.query_one("#wizard-email-format", Select).value self._data.email_format = str(fmt) if fmt in ("text", "html") else "text" self._data.target_email = self.query_one("#wizard-target-email", Input).value.strip() def _read_local_from_form(self) -> bool: - local = self.query_one("#wizard-local-picker", SchedulePicker).get_local_schedule() + local = self.query_one(_WIZ_LOCAL_PICKER, SchedulePicker).get_local_schedule() if local is None: return False self._data.local_weekday = local.weekday @@ -229,7 +236,7 @@ def _read_local_from_form(self) -> bool: return True def _read_ga_from_form(self) -> bool: - cron = self.query_one("#wizard-ga-picker", SchedulePicker).get_ga_cron() + cron = self.query_one(_WIZ_GA_PICKER, SchedulePicker).get_ga_cron() if cron is None: return False self._data.ga_cron = cron @@ -275,9 +282,9 @@ def _toggle_wizard_secrets(self, event: Checkbox.Changed) -> None: @on(Checkbox.Changed, "#wizard-only-public, #wizard-only-private") def _toggle_wizard_visibility_filters(self, event: Checkbox.Changed) -> None: if event.checkbox.id == "wizard-only-public" and event.value: - self.query_one("#wizard-only-private", Checkbox).value = False + self.query_one(_WIZ_ONLY_PRIVATE, Checkbox).value = False elif event.checkbox.id == "wizard-only-private" and event.value: - self.query_one("#wizard-only-public", Checkbox).value = False + self.query_one(_WIZ_ONLY_PUBLIC, Checkbox).value = False @on(Button.Pressed, "#wizard-cancel") def _cancel_wizard(self) -> None: @@ -316,7 +323,7 @@ def _next(self) -> None: def _validate_and_save_current_step(self) -> str | None: paths = self.app.app_state.paths # type: ignore[attr-defined] - log = self.query_one("#wizard-verify-log", RichLog) + log = self.query_one(_WIZ_VERIFY_LOG, RichLog) if self._step == 1: self._read_secrets_from_form() @@ -341,7 +348,7 @@ def _validate_and_save_current_step(self) -> str | None: write_log(log, format_error(exc), level="error") return str(exc) elif self._step == 3: - picker = self.query_one("#wizard-local-picker", SchedulePicker) + picker = self.query_one(_WIZ_LOCAL_PICKER, SchedulePicker) error = picker.validate() if error: write_log(log, error, level="error") @@ -354,7 +361,7 @@ def _validate_and_save_current_step(self) -> str | None: write_log(log, format_error(exc), level="error") return str(exc) elif self._step == 4: - picker = self.query_one("#wizard-ga-picker", SchedulePicker) + picker = self.query_one(_WIZ_GA_PICKER, SchedulePicker) error = picker.validate() if error: write_log(log, error, level="error") @@ -372,7 +379,7 @@ def _start_verify(self) -> None: if self._verifying: return self._verifying = True - log = self.query_one("#wizard-verify-log", RichLog) + log = self.query_one(_WIZ_VERIFY_LOG, RichLog) log.clear() write_log(log, "Running email-report dry-run...", level="progress") self._run_verify_worker() @@ -385,7 +392,7 @@ def _run_verify_worker(self) -> None: self._data.verify_result = result def _report() -> None: - log = self.query_one("#wizard-verify-log", RichLog) + log = self.query_one(_WIZ_VERIFY_LOG, RichLog) if result.exit_code == 0: write_log(log, "Verify passed", level="success") else: @@ -397,7 +404,7 @@ def _report() -> None: def _report_error() -> None: write_log( - self.query_one("#wizard-verify-log", RichLog), + self.query_one(_WIZ_VERIFY_LOG, RichLog), format_error(err, context="Verify failed"), level="error", ) @@ -408,7 +415,7 @@ def _report_error() -> None: def _finish(self) -> None: paths = self.app.app_state.paths # type: ignore[attr-defined] - log = self.query_one("#wizard-verify-log", RichLog) + log = self.query_one(_WIZ_VERIFY_LOG, RichLog) if sys.platform == "darwin": install_box = self.query_one("#wizard-install-la", Checkbox) if install_box.value: diff --git a/src/github_usage/legacy_report_summary.py b/src/github_usage/legacy_report_summary.py index cafba36..2accaa6 100644 --- a/src/github_usage/legacy_report_summary.py +++ b/src/github_usage/legacy_report_summary.py @@ -9,6 +9,9 @@ from .report_helpers import fmt_price from .visibility import repo_visibility, visibility_label +# Display label for the Git LFS product, repeated across summary rows. +_GIT_LFS_LABEL = "Git LFS" + def _section(title: str) -> tuple[str, str]: """Return a section header row for two-column tables.""" @@ -174,13 +177,13 @@ def _git_lfs_rows(data: dict[str, Any]) -> list[tuple[str, str]]: errors = data.get("errors") or {} billing = data.get("lfs_billing") if billing is None and errors.get("lfs_billing"): - return [("Git LFS", f"n/a ({errors['lfs_billing']})")] + return [(_GIT_LFS_LABEL, f"n/a ({errors['lfs_billing']})")] if not billing or not billing.get("items"): git_lfs = data.get("git_lfs") if git_lfs is None and errors.get("git_lfs"): - return [("Git LFS", f"n/a ({errors['git_lfs']})")] + return [(_GIT_LFS_LABEL, f"n/a ({errors['git_lfs']})")] if not git_lfs: - return [("Git LFS", "No usage")] + return [(_GIT_LFS_LABEL, "No usage")] return [("Git LFS net", fmt_price(float(git_lfs.get("total_net", 0.0))))] rows: list[tuple[str, str]] = [ ("Git LFS net", fmt_price(float(billing.get("total_net", 0.0)))), @@ -259,7 +262,7 @@ def _monthly_cost_rows(data: dict[str, Any]) -> list[tuple[str, str]]: else: rows.append(("Actions", _format_cost_block(monthly, "actions"))) rows.append(("Copilot", _format_cost_block(monthly, "copilot"))) - rows.append(("Git LFS", _format_cost_block(monthly, "git_lfs"))) + rows.append((_GIT_LFS_LABEL, _format_cost_block(monthly, "git_lfs"))) rows.append(("Total", _format_cost_block(monthly, "total"))) return rows @@ -433,7 +436,7 @@ def _tail_rows(data: dict[str, Any]) -> list[tuple[str, str]]: lfs_rows = _git_lfs_rows(data) if lfs_rows: - rows.append(_section("Git LFS")) + rows.append(_section(_GIT_LFS_LABEL)) rows.extend(lfs_rows) insights = data.get("insights") or [] diff --git a/src/github_usage/setup_config.py b/src/github_usage/setup_config.py index 7c85ac2..688c1c7 100644 --- a/src/github_usage/setup_config.py +++ b/src/github_usage/setup_config.py @@ -11,6 +11,7 @@ from .setup_prompts import _prompt_int from .setup_workflow import DEFAULT_PROFILE_NAME, DEFAULT_WORKFLOW_CONFIG, workflow_path +_REPORTS_TOML_PREFIX = "reports." DEFAULT_ENV_FILE = ".env.email-report" DEFAULT_CONFIG_DIR = ".github-usage" DEFAULT_CONFIG_FILE = "config.toml" @@ -327,9 +328,13 @@ def write_config(path: Path, config: dict) -> None: if target_subject: parts.append(f'target_subject = "{target_subject}"') parts.append("") - parts.append(_emit_email_report_block(profile["email_report"], prefix="reports.")) - parts.append(_emit_schedule_block(profile["schedule"], prefix="reports.")) - parts.append(_emit_github_actions_block(profile["github_actions"], prefix="reports.")) + parts.append( + _emit_email_report_block(profile["email_report"], prefix=_REPORTS_TOML_PREFIX) + ) + parts.append(_emit_schedule_block(profile["schedule"], prefix=_REPORTS_TOML_PREFIX)) + parts.append( + _emit_github_actions_block(profile["github_actions"], prefix=_REPORTS_TOML_PREFIX) + ) text = "\n".join(parts) else: profile = profiles[0] diff --git a/start.sh b/start.sh index 19eafe6..b45fc79 100755 --- a/start.sh +++ b/start.sh @@ -79,7 +79,7 @@ run_github_usage() { CLI_MODE=0 FILTERED=() for arg in "$@"; do - if [ "$arg" = "--cli" ]; then + if [[ "$arg" == "--cli" ]]; then CLI_MODE=1 else FILTERED+=("$arg") @@ -97,14 +97,14 @@ case "$COMMAND" in exit 0 ;; "") - if [ "$CLI_MODE" = "1" ]; then - if [ -t 0 ] || [ "${FORCE_INTERACTIVE:-}" = "1" ]; then + if [[ "$CLI_MODE" == "1" ]]; then + if [[ -t 0 || "${FORCE_INTERACTIVE:-}" == "1" ]]; then show_menu else show_help exit 0 fi - elif [ -t 0 ]; then + elif [[ -t 0 ]]; then run_github_usage else show_help @@ -182,7 +182,7 @@ case "$COMMAND" in run_github_usage runs --diff "$@" ;; *) - if [ "$CLI_MODE" = "1" ]; then + if [[ "$CLI_MODE" == "1" ]]; then run_github_usage --cli "$COMMAND" "${@:2}" else echo "Error: Unknown command '$COMMAND'" >&2 diff --git a/tests/test_gui_selectors.py b/tests/test_gui_selectors.py new file mode 100644 index 0000000..823cd26 --- /dev/null +++ b/tests/test_gui_selectors.py @@ -0,0 +1,46 @@ +"""Tests that GUI selector constants match their expected widget IDs. + +This ensures that any refactoring of widget IDs will fail this test if the +corresponding constants are not also updated, mitigating the risk of the +indirection introduced for SonarCloud rule S1192. +""" + +from github_usage.gui.views.email_report_view import _EMAIL_PREVIEW +from github_usage.gui.views.report_view import _REPORT_LOG +from github_usage.gui.views.runs_view import _RUNS_LOG +from github_usage.gui.views.schedules_view import ( + _INSTALL_LA_LABEL, + _PROFILE_SELECT, + _SCHED_LOG, + _SCHEDULE_PICKER, +) +from github_usage.gui.views.setup_profiles_panel import _ONLY_PRIVATE, _ONLY_PUBLIC +from github_usage.gui.views.setup_verify_panel import _VERIFY_LOG +from github_usage.gui.wizard.setup_wizard_screen import ( + _WIZ_GA_PICKER, + _WIZ_LOCAL_PICKER, + _WIZ_ONLY_PRIVATE, + _WIZ_ONLY_PUBLIC, + _WIZ_VERIFY_LOG, +) + + +def test_gui_selectors_match_widget_ids(): + assert _EMAIL_PREVIEW == "#email-preview" + assert _REPORT_LOG == "#report-log" + assert _RUNS_LOG == "#runs-log" + + assert _SCHED_LOG == "#sched-log" + assert _PROFILE_SELECT == "#profile-select" + assert _SCHEDULE_PICKER == "#schedule-picker" + assert _INSTALL_LA_LABEL == "Install LaunchAgent" + + assert _ONLY_PUBLIC == "#only-public" + assert _ONLY_PRIVATE == "#only-private" + assert _VERIFY_LOG == "#verify-log" + + assert _WIZ_LOCAL_PICKER == "#wizard-local-picker" + assert _WIZ_GA_PICKER == "#wizard-ga-picker" + assert _WIZ_ONLY_PUBLIC == "#wizard-only-public" + assert _WIZ_ONLY_PRIVATE == "#wizard-only-private" + assert _WIZ_VERIFY_LOG == "#wizard-verify-log"