diff --git a/tools/developer_tools/bely-cli/README.md b/tools/developer_tools/bely-cli/README.md index a404c0291..a04a1c03a 100644 --- a/tools/developer_tools/bely-cli/README.md +++ b/tools/developer_tools/bely-cli/README.md @@ -257,10 +257,14 @@ All three levels render as full-width, aligned tables (rows stay in API order, n | Level | Columns | |-------|---------| -| Logbook | Name, Display, Description | -| Document | Name, Description, Systems, Owner, Modified | +| Logbook | Display, Description | +| Document | Name, Systems, Owner, Modified | | Entry | Date, Author, Entry (a snippet of the first line) — replies render as indented rows beneath their parent, expanded by default | +Logbook types render as an indented hierarchy in API order. Grouping parents are +informational; select a leaf to browse or create documents. Filtering includes hierarchy +labels and type metadata. + Press `i` at the logbook/document levels to open a side info panel with a few extra fields for the highlighted row (it splits the table's width; `i` again closes it). Entries always show a preview pane alongside the table — the entry body rendered as markdown (headings, @@ -297,15 +301,15 @@ on — `i` disappears once you drill into entries, and `s` / `y` / `e` / `f` / ` | `s` | Entries level only: save the highlighted entry's markdown to a file in the current directory. | | `y` | Entries level only: copy a `bely-cli entry get` reference for the highlighted entry to the clipboard. | | `e` | Entries level only: open the highlighted entry in `$EDITOR`; if you change it, offers to save the result back to the server (a mutation, so this is where the app authenticates if it hasn't already). | +| `p` | Entries level only: reply to the highlighted entry's top-level thread. Attachments are supported. | | `t` | Entries level only: collapse/expand the reply thread under the highlighted entry (or its parent, if the highlight is on a reply). Replies start expanded. | | `i` | Logbook/document levels only: toggle the side info panel. | | `f` | Entries level only: toggle the table to widen the preview pane. | -| `r` | Refresh the current level, bypassing the in-session cache. Collapsed threads stay collapsed. | +| `r` | Refresh the current level, bypassing the in-session cache. Entry selection, preview scroll, filters, and collapsed threads are preserved when possible. | | `q` | Quit without selecting. | -Replies only ever nest one level deep — the server doesn't return replies-to-replies — and -`n` on a highlighted reply adds a new top-level entry, not a reply to that reply (there's no -API for that yet). +Replies only nest one level deep. Pressing `p` on either a top-level entry or one of its +replies targets the top-level thread; `n` still adds a separate top-level entry. On selecting an entry the TUI exits and prints its `doc-id` / `log-id`, plus a ready-to-run `bely-cli entry get` command so you can fetch it: diff --git a/tools/developer_tools/bely-cli/src/bely_cli/auth.py b/tools/developer_tools/bely-cli/src/bely_cli/auth.py index 14a83cebd..2cce4ceca 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/auth.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/auth.py @@ -21,10 +21,13 @@ def get_token_file(): def get_host(): - """Return the BELY server URL from env var or settings.""" + """Return the BELY server URL from env var or settings without trailing slashes.""" host = os.environ.get("BELY_HOST") or get_setting("host") if not host: raise ValueError("no host configured. Set BELY_HOST or add 'host' to settings.yaml.") + host = host.rstrip("/") + if not host: + raise ValueError("BELY host must not contain only slashes.") return host @@ -130,7 +133,9 @@ def login(username, password): except belyApi.exceptions.UnauthorizedException: raise ValueError(f"Authentication failed: invalid credentials for user '{username}'") except Exception as e: - raise RuntimeError(f"Authentication failed: {e}") from e + from .common import format_error_message + + raise RuntimeError(f"Authentication failed: {format_error_message(e, factory)}") from e save_token(factory.get_authenticate_token()) return factory diff --git a/tools/developer_tools/bely-cli/src/bely_cli/cli.py b/tools/developer_tools/bely-cli/src/bely_cli/cli.py index 4f8249d92..85741ff15 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/cli.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/cli.py @@ -2,7 +2,7 @@ import click -from .common import FORMATS, set_no_prompt +from .common import FORMATS, format_error_message, set_no_prompt from .config import VALID_FIELDS from .commands import ( cmd_new_doc, @@ -212,7 +212,7 @@ def main(): try: cli() except Exception as e: - print(f"Error: {e}", file=sys.stderr) + print(f"Error: {format_error_message(e)}", file=sys.stderr) sys.exit(1) diff --git a/tools/developer_tools/bely-cli/src/bely_cli/common.py b/tools/developer_tools/bely-cli/src/bely_cli/common.py index ea743e359..6b0512c68 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/common.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/common.py @@ -25,6 +25,27 @@ def is_no_prompt(): return _no_prompt +def format_error_message(error, factory=None): + """Return a concise server message for API errors, or the normal exception text.""" + try: + import belyApi + + if not isinstance(error, belyApi.exceptions.ApiException): + return str(error) + except Exception: + return str(error) + + try: + if factory is None: + from . import auth + + factory = auth.get_factory() + parsed = factory.parse_api_exception(error) + return parsed.message or str(error) + except Exception: + return str(error) + + def print_items(items, columns, fmt="text"): """Print a list of dicts as a table, JSON array, or YAML sequence. diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/data.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/data.py index daeb78af2..38316e710 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/data.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/data.py @@ -34,7 +34,7 @@ def __init__(self, logbook_api, download_api=None): def logbook_types(self): if self._types is None: - self._types = self._logbook_api.get_logbook_types() + self._types = self._logbook_api.get_logbook_type_hierarchy() return self._types def logbook_systems(self): diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/format.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/format.py index 1947c3cdb..ad78a9ba9 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/format.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/format.py @@ -11,11 +11,46 @@ # -- list-row formatting (used by both the old curses UI and the new # Textual OptionList rows) -- +class TypeNode(NamedTuple): + """One flattened hierarchy row wrapping its original EntityType.""" + + entity: object + depth: int + branch: str + selectable: bool + hierarchy_text: str + + +def flatten_types(types): + """Flatten EntityType children depth-first while retaining hierarchy guides.""" + rows = [] + + def walk(items, depth, prefix_lasts, ancestors): + for index, entity in enumerate(items or []): + is_last = index == len(items) - 1 + branch = _branch_prefix(prefix_lasts, is_last) if depth else "" + children = getattr(entity, "entity_type_children", None) or [] + name = getattr(entity, "name", None) or "" + hierarchy_text = " / ".join(ancestors + [name]) + rows.append(TypeNode(entity, depth, branch, not children, hierarchy_text)) + if children: + next_prefix = prefix_lasts + [is_last] if depth else [] + walk(children, depth + 1, next_prefix, ancestors + [name]) + + walk(types, 0, [], []) + return rows + + +def type_entity(t): + return t.entity if isinstance(t, TypeNode) else t + + def format_type(t): - """Display string for a logbook type (EntityType).""" - display = getattr(t, "display_name", None) or "" - name = getattr(t, "name", None) or "" - return f"{name} ({display})" if display else name + """Display name for a logbook type, with hierarchy guides when present.""" + node = t if isinstance(t, TypeNode) else None + entity = type_entity(t) + label = getattr(entity, "display_name", None) or getattr(entity, "name", None) or "" + return f"{node.branch}{label}" if node else label def format_doc(d): @@ -65,27 +100,29 @@ def _doc_owner(d): return getattr(more_info, "owner_username", None) or "" -TYPE_COLUMNS = [("Name", 24), ("Display", 24), ("Description", None)] +TYPE_COLUMNS = [("Display", 32), ("Description", None)] def type_row(t): - """DataTable row cells for a logbook type (EntityType).""" - name = getattr(t, "name", None) or "" - display = getattr(t, "display_name", None) or "" - description = getattr(t, "description", None) or "" - return (name, display, description) + """DataTable row cells for a logbook type (EntityType or TypeNode).""" + node = t if isinstance(t, TypeNode) else None + entity = type_entity(t) + display = getattr(entity, "display_name", None) or getattr(entity, "name", None) or "" + if node: + display = node.branch + display + description = getattr(entity, "description", None) or "" + return (display, description) DOC_COLUMNS = [ - ("Name", 32), ("Description", None), ("Systems", 20), ("Owner", 14), ("Modified", 16), + ("Name", None), ("Systems", 20), ("Owner", 14), ("Modified", 16), ] def doc_row(d): """DataTable row cells for a log document (ItemDomainLogbook).""" name = getattr(d, "name", None) or "(unnamed)" - description = getattr(d, "description", None) or "" - return (name, description, _doc_systems(d), _doc_owner(d), _doc_modified(d)) + return (name, _doc_systems(d), _doc_owner(d), _doc_modified(d)) ENTRY_COLUMNS = [("Date", 16), ("Author", 16), ("Entry", None)] @@ -220,8 +257,13 @@ def summarize_reactions(reactions): def entry_metadata_rows(entry, doc, parent=None): """[(label, value)] metadata rows for the entry preview header.""" rows = [("log_id", str(getattr(entry, "log_id", "") or ""))] - if parent is not None: - rows.append(("reply to", str(getattr(parent, "log_id", "") or ""))) + doc_id = getattr(entry, "item_id", None) or getattr(doc, "id", None) or "" + rows.append(("log_doc_id", str(doc_id))) + parent_id = getattr(entry, "parent_log_id", None) + if parent_id is None and parent is not None: + parent_id = getattr(parent, "log_id", None) + if parent_id is not None: + rows.append(("parent_log_id", str(parent_id))) rows.append(("doc", getattr(doc, "name", None) or "")) entered_by = getattr(entry, "entered_by_username", None) or "" @@ -284,6 +326,7 @@ def doc_metadata_rows(doc): def type_metadata_rows(t): """[(label, value)] metadata rows for the logbook-type preview header.""" + t = type_entity(t) rows = [("name", getattr(t, "name", None) or "")] display_name = getattr(t, "display_name", None) diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/browse.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/browse.py index 559ceeade..f04e27f69 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/browse.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/browse.py @@ -31,6 +31,7 @@ from textual.widgets import DataTable, Footer, Header, Input, Markdown, Static from ... import config +from ...common import format_error_message from . import rows_table from ..format import ( DOC_COLUMNS, @@ -43,10 +44,12 @@ entry_row, filter_items, flatten_entries, + flatten_types, format_attachment, format_doc, format_type, reference_command, + type_entity, type_metadata_rows, type_row, ) @@ -74,7 +77,10 @@ class BrowseScreen(Screen): LEVEL_COLUMNS = {LEVEL_TYPES: TYPE_COLUMNS, LEVEL_DOCS: DOC_COLUMNS, LEVEL_ENTRIES: ENTRY_COLUMNS} LEVEL_ROW_FN = {LEVEL_TYPES: type_row, LEVEL_DOCS: doc_row, LEVEL_ENTRIES: entry_node_row} # Entries render via entry_node_row (tree glyphs) but filter on the plain entry cells. - LEVEL_SEARCH_FN = {LEVEL_ENTRIES: lambda node: entry_row(node.entry)} + LEVEL_SEARCH_FN = { + LEVEL_TYPES: lambda node: type_row(node) + (node.hierarchy_text,), + LEVEL_ENTRIES: lambda node: entry_row(node.entry), + } # Per-level nav pane width (%), used whenever a preview/info panel is visible. LEVEL_WIDTH = {LEVEL_TYPES: 42, LEVEL_DOCS: 60, LEVEL_ENTRIES: 42} @@ -89,6 +95,7 @@ class BrowseScreen(Screen): "copy_reference": (LEVEL_ENTRIES,), "open_editor": (LEVEL_ENTRIES,), "update_entry": (LEVEL_ENTRIES,), + "reply": (LEVEL_ENTRIES,), "new_entry": (LEVEL_DOCS, LEVEL_ENTRIES), "new_doc": (LEVEL_TYPES, LEVEL_DOCS), "toggle_info": (LEVEL_TYPES, LEVEL_DOCS), @@ -106,6 +113,7 @@ class BrowseScreen(Screen): Binding("e", "open_editor", "Edit in editor"), Binding("n", "new_entry", "New entry"), Binding("u", "update_entry", "Edit in TUI"), + Binding("p", "reply", "Reply"), Binding("d", "new_doc", "New doc"), Binding("r", "refresh_level", "Refresh"), Binding("i", "toggle_info", "Info"), @@ -131,6 +139,7 @@ def __init__(self, session, limit, *, select_mode=True, source="types", root=Fal self._nav_hidden = False self._info_open = False self._table_columns_for = None + self._pending_entry_restore = None def compose(self) -> ComposeResult: yield Header() @@ -188,7 +197,13 @@ def _ensure_columns(self): # -- level loading -- - def show_level(self, level, *, preserve_filter=False): + def show_level(self, level, *, preserve_filter=False, preserve_entry_position=False): + if preserve_entry_position and self.level == self.LEVEL_ENTRIES: + node = self._current_node() + row = self._nav().cursor_row or 0 + scroll_y = self.query_one("#preview", VerticalScroll).scroll_y + self._pending_entry_restore = ( + getattr(node.entry, "log_id", None) if node else None, row, scroll_y) self.level = level # "f" full-screen and the reply tree only apply at the entries level; reset when leaving. if level != self.LEVEL_ENTRIES: @@ -225,9 +240,10 @@ def show_level(self, level, *, preserve_filter=False): @work(thread=True, exclusive=True, group="fetch") def _load_types(self): try: - items = self.data.logbook_types() + items = flatten_types(self.data.logbook_types()) except Exception as e: - self.app.call_from_thread(self._fetch_failed, str(e)) + self.app.call_from_thread( + self._fetch_failed, format_error_message(e, self.session.factory)) return self.app.call_from_thread(self._populate, items) @@ -236,7 +252,8 @@ def _load_docs(self, type_id): try: items = self.data.documents(type_id, self.limit) except Exception as e: - self.app.call_from_thread(self._fetch_failed, str(e)) + self.app.call_from_thread( + self._fetch_failed, format_error_message(e, self.session.factory)) return self.app.call_from_thread(self._populate, items) @@ -248,7 +265,8 @@ def _load_recent_docs(self): raise RuntimeError("cannot determine username. Set BELY_USER or 'user' in settings.") items = self.data.recent_documents(self.session.factory, username, self.limit) except Exception as e: - self.app.call_from_thread(self._fetch_failed, str(e)) + self.app.call_from_thread( + self._fetch_failed, format_error_message(e, self.session.factory)) return self.app.call_from_thread(self._populate, items) @@ -257,7 +275,8 @@ def _load_entries(self, doc_id): try: items = self.data.entries(doc_id) except Exception as e: - self.app.call_from_thread(self._fetch_failed, str(e)) + self.app.call_from_thread( + self._fetch_failed, format_error_message(e, self.session.factory)) return self.app.call_from_thread(self._populate, items) @@ -279,10 +298,33 @@ def _populate(self, items): self.entry_tree = items items = flatten_entries(items, self._collapsed) self.all_items = items - self._apply_filter("") + query = self.query_one("#filter", Input).value + self._apply_filter(query) + if self.level == self.LEVEL_ENTRIES and self._pending_entry_restore is not None: + self._restore_entry_position() self._update_header() + self.refresh_bindings() nav.focus() + def _restore_entry_position(self): + log_id, prior_row, scroll_y = self._pending_entry_restore + self._pending_entry_restore = None + row = next( + (index for index, node in enumerate(self.shown_items) + if node.entry.log_id == log_id), + min(prior_row, len(self.shown_items) - 1) if self.shown_items else None, + ) + if row is None: + return + self._nav().move_cursor(row=row) + self.run_worker( + partial(self._show_preview, self.shown_items[row]), exclusive=True, group="preview") + self.call_after_refresh(self._restore_preview_scroll, scroll_y) + + def _restore_preview_scroll(self, scroll_y): + preview = self.query_one("#preview", VerticalScroll) + preview.scroll_to(y=min(scroll_y, preview.max_scroll_y), animate=False) + def _apply_filter(self, query): row_fn = self.LEVEL_ROW_FN[self.level] search_fn = self.LEVEL_SEARCH_FN.get(self.level, row_fn) @@ -490,7 +532,10 @@ def on_data_table_row_selected(self, event): return item = self.shown_items[event.cursor_row] if self.level == self.LEVEL_TYPES: - self.sel_type = item + if not item.selectable: + self.notify("Select a leaf logbook type.") + return + self.sel_type = type_entity(item) self.show_level(self.LEVEL_DOCS) elif self.level == self.LEVEL_DOCS: self.sel_doc = item @@ -539,7 +584,17 @@ def action_toggle_info(self): def check_action(self, action, parameters): levels = self.ACTION_LEVELS.get(action) - return True if levels is None else (self.level in levels or None) + if levels is not None and self.level not in levels: + return None + if action == "new_entry" and self._current_doc() is None: + return None + if action == "reply" and self._current_node() is None: + return None + if action == "new_doc" and self.level == self.LEVEL_TYPES: + item = self._current_item() + if item is not None and not item.selectable: + return None + return True def action_refresh_level(self): if self.level == self.LEVEL_TYPES: @@ -551,10 +606,20 @@ def action_refresh_level(self): self.data.invalidate("docs", type_id=self.sel_type.id) else: self.data.invalidate("entries", doc_id=self.sel_doc.id) - self.show_level(self.level, preserve_filter=True) + self.show_level( + self.level, + preserve_filter=True, + preserve_entry_position=self.level == self.LEVEL_ENTRIES, + ) # -- current-selection helpers -- + def _current_item(self): + table = self._nav() + if table.cursor_row is None or table.cursor_row >= len(self.shown_items): + return None + return self.shown_items[table.cursor_row] + def _current_node(self): if self.level != self.LEVEL_ENTRIES: return None @@ -589,7 +654,8 @@ def _current_type(self): table = self._nav() if table.cursor_row is None or table.cursor_row >= len(self.shown_items): return None - return self.shown_items[table.cursor_row] + node = self.shown_items[table.cursor_row] + return type_entity(node) if node.selectable else None return None # -- entry actions -- @@ -687,11 +753,15 @@ async def _edit_entry_externally(self, entry): try: await asyncio.to_thread(core.save_entry, api, entry, edited) except Exception as e: - self.notify(f"Save failed: {e}", severity="error") + self.notify( + f"Save failed: {format_error_message(e, self.session.factory)}", + severity="error", + ) return self.data.invalidate("entries", doc_id=self.sel_doc.id) - self.show_level(self.LEVEL_ENTRIES, preserve_filter=True) + self.show_level( + self.LEVEL_ENTRIES, preserve_filter=True, preserve_entry_position=True) self.notify("Entry saved.") # -- add / update entry (mutating: goes through the auth gate) -- @@ -703,6 +773,14 @@ def action_new_entry(self): return self._run_compose(doc, None) + def action_reply(self): + node = self._current_node() + if node is None: + self.notify("Select an entry first.", severity="warning") + return + target = node.parent if node.depth > 0 else node.entry + self._run_compose(self.sel_doc, None, reply_to=target) + def action_update_entry(self): entry = self._current_entry() if entry is None: @@ -711,21 +789,31 @@ def action_update_entry(self): self._run_compose(self.sel_doc, entry) @work - async def _run_compose(self, doc, entry): - """Authenticate, then push ComposeScreen for a new or existing entry.""" + async def _run_compose(self, doc, entry, reply_to=None): + """Authenticate, then push ComposeScreen for a new, reply, or existing entry.""" from .compose import open_composer api = await self.app.ensure_auth() if api is None: return - saved = await open_composer(self.app, doc, api, entry=entry) + saved = await open_composer( + self.app, doc, api, entry=entry, factory=self.session.factory, reply_to=reply_to) if not saved: return self.sel_doc = doc self.data.invalidate("entries", doc_id=doc.id) + if reply_to is not None: + self._collapsed.discard(reply_to.log_id) + saved_id = getattr(saved, "log_id", None) or reply_to.log_id + preview = self.query_one("#preview", VerticalScroll) + self._pending_entry_restore = (saved_id, self._nav().cursor_row or 0, preview.scroll_y) if self.level == self.LEVEL_ENTRIES: - self.show_level(self.LEVEL_ENTRIES, preserve_filter=True) + self.show_level( + self.LEVEL_ENTRIES, + preserve_filter=True, + preserve_entry_position=reply_to is None, + ) else: self.show_level(self.LEVEL_ENTRIES) diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/compose.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/compose.py index 05e386249..55897dac2 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/compose.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/compose.py @@ -19,7 +19,7 @@ from textual.widgets import Button, Input, Static, TextArea from ... import core -from ...common import editor_changed +from ...common import editor_changed, format_error_message from .dialog import CANCEL_HINT, SAVE_HINT, DialogButtons, DialogScreen, hinted_label @@ -42,17 +42,23 @@ class ComposeScreen(DialogScreen): # Save first: right after the attachment field in tab order, since it's used most. BUTTON_ROWS = [["compose-save", "compose-editor", "compose-cancel"]] - def __init__(self, doc, entry, api, *, is_new): + def __init__(self, doc, entry, api, *, is_new, factory=None, reply_to=None): super().__init__() self.doc = doc self.entry = entry self.api = api + self.factory = factory self.is_new = is_new + self.reply_to = reply_to self._initial_text = entry.log_entry or "" def compose(self) -> ComposeResult: - title = (f'New entry in "{self.doc.name}"' if self.is_new - else f'Update entry #{self.entry.log_id} in "{self.doc.name}"') + if self.reply_to is not None: + title = f'Reply to entry #{self.reply_to.log_id} in "{self.doc.name}"' + elif self.is_new: + title = f'New entry in "{self.doc.name}"' + else: + title = f'Update entry #{self.entry.log_id} in "{self.doc.name}"' with Vertical(id="compose-dialog", classes="dialog"): yield Static(title, id="compose-title") yield TextArea(self._initial_text, language="markdown", id="compose-area") @@ -150,13 +156,14 @@ async def _save(self): await asyncio.to_thread( core.upload_attachment, self.api, self.doc.id, saved_entry.log_id, attach_path) except Exception as e: - self.notify(f"Save failed: {e}", severity="error") + self.notify( + f"Save failed: {format_error_message(e, self.factory)}", severity="error") return self.dismiss(saved_entry) -async def open_composer(app, doc, api, *, entry=None): +async def open_composer(app, doc, api, *, entry=None, factory=None, reply_to=None): """Push ComposeScreen for a new or existing entry. When `entry` is None, fetches a fresh template first (needs an @@ -171,9 +178,16 @@ async def open_composer(app, doc, api, *, entry=None): try: entry = await asyncio.to_thread(core.new_entry_template, api, doc.id) except Exception as e: - app.notify(f"Could not load entry template: {e}", severity="error") + app.notify( + f"Could not load entry template: {format_error_message(e, factory)}", + severity="error", + ) return None + if reply_to is not None: + entry.parent_log_id = reply_to.log_id is_new = True else: is_new = False - return await app.push_screen_wait(ComposeScreen(doc, entry, api, is_new=is_new)) + return await app.push_screen_wait( + ComposeScreen( + doc, entry, api, is_new=is_new, factory=factory, reply_to=reply_to)) diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/configscreen.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/configscreen.py index 38868833a..52e16548e 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/configscreen.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/configscreen.py @@ -12,7 +12,7 @@ from textual import work from textual.app import ComposeResult from textual.binding import Binding -from textual.containers import Vertical +from textual.containers import Horizontal, Vertical from textual.widgets import Button, Input, Select, Static from ... import config, core @@ -25,6 +25,20 @@ class ConfigScreen(DialogScreen): #config-dialog { width: 70; } + + .config-field { + height: auto; + align: left middle; + margin-top: 1; + } + + .config-label { + width: 24; + } + + .config-field Input, .config-field Select { + width: 1fr; + } """ BINDINGS = [Binding("ctrl+s", "submit", "Save", show=False)] @@ -37,21 +51,35 @@ class ConfigScreen(DialogScreen): FIELD_CHOICES = {"images": IMAGE_MODES} # enum fields get a Select instead of an Input FIELD_DEFAULTS = {"images": "auto"} + FIELD_LABELS = { + "host": "Host", + "user": "User", + "editor": "Editor", + "token_path": "Token path", + "theme": "Theme", + "images": "Images", + } def compose(self) -> ComposeResult: with Vertical(id="config-dialog", classes="dialog"): yield Static(id="config-breadcrumb") yield Static(id="config-summary") for field in config.VALID_FIELDS: - choices = self.FIELD_CHOICES.get(field) - if choices: - yield Select( - [(f"{choice} — {IMAGE_MODE_HELP[choice]}", choice) - for choice in choices], - allow_blank=False, id=f"config-{field}", + with Horizontal(classes="config-field"): + yield Static( + f"{self.FIELD_LABELS[field]}:", + id=f"config-{field}-label", + classes="config-label", ) - else: - yield Input(placeholder=field, id=f"config-{field}") + choices = self.FIELD_CHOICES.get(field) + if choices: + yield Select( + [(f"{choice} — {IMAGE_MODE_HELP[choice]}", choice) + for choice in choices], + allow_blank=False, id=f"config-{field}", + ) + else: + yield Input(id=f"config-{field}") with DialogButtons(): yield Button(hinted_label("Save", SAVE_HINT), variant="primary", id="config-save") yield Button(hinted_label("Edit file"), id="config-edit") @@ -101,9 +129,10 @@ def _load(self): box = self.query_one(f"#config-{field}", Input) box.value = str(settings.get(field, "") or "") env_var = self.ENV_FOR_FIELD.get(field) - box.placeholder = ( - f"{field} (overridden by {env_var})" if env_var and env_var in env else field - ) + label = f"{self.FIELD_LABELS[field]}:" + if env_var and env_var in env: + label += f" [dim](overridden by {env_var})[/dim]" + self.query_one(f"#config-{field}-label", Static).update(label) def action_submit(self): self._save() diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/newdoc.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/newdoc.py index 1e9e93329..91e8aa5a9 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/newdoc.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/newdoc.py @@ -21,7 +21,8 @@ from textual.widgets import Button, Input, Static from ... import core -from ...common import find_logdoc +from ...common import find_logdoc, format_error_message +from ..format import flatten_types, format_type from .dialog import CANCEL_HINT, SAVE_HINT, DialogButtons, DialogScreen, hinted_label # Sentinel for "explicitly skip the default template" -- distinct from @@ -104,15 +105,19 @@ async def _pick_type(self): from .picker import PickerScreen try: - types = await asyncio.to_thread(self.session.data.logbook_types) + types = flatten_types(await asyncio.to_thread(self.session.data.logbook_types)) except Exception as e: - self.notify(f"Could not load types: {e}", severity="error") + self.notify( + f"Could not load types: {format_error_message(e, self.session.factory)}", + severity="error", + ) return choice = await self.app.push_screen_wait( - PickerScreen("Logbook type", types, lambda t: t.name or "") + PickerScreen( + "Logbook type", types, format_type, selectable_fn=lambda node: node.selectable) ) if choice is not None: - self.logbook_type = choice + self.logbook_type = choice.entity self._refresh_labels() @work @@ -122,7 +127,10 @@ async def _pick_systems(self): try: systems = await asyncio.to_thread(self.session.data.logbook_systems) except Exception as e: - self.notify(f"Could not load systems: {e}", severity="error") + self.notify( + f"Could not load systems: {format_error_message(e, self.session.factory)}", + severity="error", + ) return choice = await self.app.push_screen_wait( PickerScreen("Systems (space to toggle)", systems, lambda s: s.name or "", multi=True) @@ -138,7 +146,10 @@ async def _pick_template(self): try: templates = await asyncio.to_thread(self.session.data.logbook_templates) except Exception as e: - self.notify(f"Could not load templates: {e}", severity="error") + self.notify( + f"Could not load templates: {format_error_message(e, self.session.factory)}", + severity="error", + ) return items = [_NO_TEMPLATE] + list(templates) choice = await self.app.push_screen_wait( @@ -191,7 +202,10 @@ async def _create(self): skip_default_template=self.skip_template, ) except Exception as e: - self.notify(f"Create failed: {e}", severity="error") + self.notify( + f"Create failed: {format_error_message(e, self.session.factory)}", + severity="error", + ) return self.notify(f'Document "{doc.name}" created, id={doc.id}') @@ -215,12 +229,13 @@ async def _post_create(self, api, doc): ) ) if edit_now: - await open_composer(self.app, doc, api, entry=entry) + await open_composer( + self.app, doc, api, entry=entry, factory=self.session.factory) else: create_entry = await self.app.push_screen_wait( ConfirmScreen("Create a log entry now?", confirm_label="Create entry", cancel_label="Skip") ) if create_entry: - await open_composer(self.app, doc, api) + await open_composer(self.app, doc, api, factory=self.session.factory) self.dismiss(doc) diff --git a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/picker.py b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/picker.py index e6fa28530..d73d7793d 100644 --- a/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/picker.py +++ b/tools/developer_tools/bely-cli/src/bely_cli/tui/screens/picker.py @@ -32,12 +32,13 @@ class PickerScreen(DialogScreen): BUTTON_ROWS = [["picker-select", "picker-cancel"]] - def __init__(self, title, items, label_fn, *, multi=False): + def __init__(self, title, items, label_fn, *, multi=False, selectable_fn=None): super().__init__() self.title_text = title self.items = list(items) self.label_fn = label_fn self.multi = multi + self.selectable_fn = selectable_fn or (lambda item: True) self.selected = set() self.shown = list(self.items) @@ -60,7 +61,7 @@ def _option_text(self, item): if not self.multi: return label idx = self.items.index(item) - mark = "[x]" if idx in self.selected else "[ ]" + mark = "☒" if idx in self.selected else "☐" return f"{mark} {label}" def _populate(self, query): @@ -99,7 +100,11 @@ def _select_highlighted(self): lst = self.query_one("#picker-list", OptionList) if lst.highlighted is None or not self.shown: return - self.dismiss(self.shown[lst.highlighted]) + item = self.shown[lst.highlighted] + if not self.selectable_fn(item): + self.notify("Select a leaf logbook type.") + return + self.dismiss(item) def action_toggle(self): if not self.multi: diff --git a/tools/developer_tools/bely-cli/test/test_auth.py b/tools/developer_tools/bely-cli/test/test_auth.py index af2d7a290..0cc1b0cc6 100644 --- a/tools/developer_tools/bely-cli/test/test_auth.py +++ b/tools/developer_tools/bely-cli/test/test_auth.py @@ -47,6 +47,22 @@ def get_authenticate_token(self): return FakeFactory +class GetHostTests(unittest.TestCase): + @patch.dict(os.environ, {"BELY_HOST": "https://example.test/bely///"}, clear=True) + def test_environment_host_drops_trailing_slashes(self): + self.assertEqual(auth.get_host(), "https://example.test/bely") + + @patch.dict(os.environ, {}, clear=True) + @patch.object(auth, "get_setting", return_value="https://example.test/bely/") + def test_settings_host_drops_trailing_slash(self, _get_setting): + self.assertEqual(auth.get_host(), "https://example.test/bely") + + @patch.dict(os.environ, {"BELY_HOST": "///"}, clear=True) + def test_slashes_only_host_is_rejected(self): + with self.assertRaisesRegex(ValueError, "only slashes"): + auth.get_host() + + class AuthTestCase(unittest.TestCase): """Points auth.py's token file at a scratch path and stubs get_host().""" diff --git a/tools/developer_tools/bely-cli/test/test_common.py b/tools/developer_tools/bely-cli/test/test_common.py index 517646bf0..2261c0506 100644 --- a/tools/developer_tools/bely-cli/test/test_common.py +++ b/tools/developer_tools/bely-cli/test/test_common.py @@ -4,6 +4,28 @@ from bely_cli import common +class FormatErrorMessageTests(unittest.TestCase): + def test_non_api_error_uses_exception_text(self): + self.assertEqual(common.format_error_message(ValueError("concise")), "concise") + + def test_api_error_uses_factory_parser_message(self): + import belyApi + + error = belyApi.exceptions.ApiException(status=400, body="long response") + factory = unittest.mock.Mock() + factory.parse_api_exception.return_value.message = "short message" + self.assertEqual(common.format_error_message(error, factory), "short message") + factory.parse_api_exception.assert_called_once_with(error) + + def test_parser_failure_falls_back_to_exception_text(self): + import belyApi + + error = belyApi.exceptions.ApiException(status=400, body="response") + factory = unittest.mock.Mock() + factory.parse_api_exception.side_effect = ValueError("bad response") + self.assertEqual(common.format_error_message(error, factory), str(error)) + + class OpenInEditorTests(unittest.TestCase): def test_shell_style_editor_command_is_split(self): with patch.object(common.config, "get_editor", return_value="code -w"), \ diff --git a/tools/developer_tools/bely-cli/test/test_core.py b/tools/developer_tools/bely-cli/test/test_core.py index 400a6ee4c..709864e51 100644 --- a/tools/developer_tools/bely-cli/test/test_core.py +++ b/tools/developer_tools/bely-cli/test/test_core.py @@ -146,6 +146,13 @@ def test_save_entry_sets_content_and_saves(self): self.assertEqual(saved.log_id, 99) self.assertIs(api.saved, entry) + def test_save_entry_preserves_reply_parent(self): + api = FakeLogbookApi() + entry = SimpleNamespace(log_id=None, log_entry="", parent_log_id=7) + saved = core.save_entry(api, entry, "reply") + self.assertEqual(saved.parent_log_id, 7) + self.assertIs(api.saved, entry) + def test_find_entry_found_and_missing(self): entries = [SimpleNamespace(log_id=1), SimpleNamespace(log_id=2)] self.assertIs(core.find_entry(entries, 2), entries[1]) diff --git a/tools/developer_tools/bely-cli/test/test_tui.py b/tools/developer_tools/bely-cli/test/test_tui.py index 48ed30a5a..ffd5a1894 100644 --- a/tools/developer_tools/bely-cli/test/test_tui.py +++ b/tools/developer_tools/bely-cli/test/test_tui.py @@ -22,17 +22,54 @@ def test_substring_anywhere(self): class TypeRowTests(unittest.TestCase): + def test_picker_format_uses_display_name_and_hierarchy(self): + entity = SimpleNamespace(name="ops", display_name="Operations", entity_type_children=[]) + row = fmt.flatten_types([entity])[0] + self.assertEqual(fmt.format_type(row), "Operations") + + def test_picker_format_falls_back_to_name(self): + entity = SimpleNamespace(name="ops", display_name=None, entity_type_children=[]) + self.assertEqual(fmt.format_type(fmt.flatten_types([entity])[0]), "ops") + + def test_flattens_hierarchy_depth_first_with_guides(self): + leaf_a = SimpleNamespace(name="a", entity_type_children=[]) + leaf_b = SimpleNamespace(name="b", entity_type_children=[]) + group = SimpleNamespace(name="group", entity_type_children=[leaf_a, leaf_b]) + leaf_c = SimpleNamespace(name="c", entity_type_children=[]) + + rows = fmt.flatten_types([group, leaf_c]) + + self.assertEqual([row.entity.name for row in rows], ["group", "a", "b", "c"]) + self.assertEqual([row.selectable for row in rows], [False, True, True, True]) + self.assertEqual(fmt.type_row(rows[1])[0], " ├─ a") + self.assertEqual(fmt.type_row(rows[2])[0], " └─ b") + + def test_hierarchy_text_is_filterable(self): + leaf = SimpleNamespace( + name="leaf", display_name="Leaf", description="Operations", + entity_type_children=[]) + row = fmt.flatten_types([leaf])[0] + result = fmt.filter_items([row], "operations", lambda item: " ".join(fmt.type_row(item))) + self.assertEqual(result, [row]) + + child = SimpleNamespace( + name="child", display_name="Child", description="", entity_type_children=[]) + parent = SimpleNamespace( + name="parent", display_name="Parent", description="", entity_type_children=[child]) + child_row = fmt.flatten_types([parent])[1] + self.assertIn("parent", child_row.hierarchy_text) + def test_returns_name_display_description(self): t = SimpleNamespace(name="ops", display_name="Ops", description="Operations log") - self.assertEqual(fmt.type_row(t), ("ops", "Ops", "Operations log")) + self.assertEqual(fmt.type_row(t), ("Ops", "Operations log")) def test_missing_fields_become_empty_strings(self): t = SimpleNamespace(name="ops", display_name=None, description=None) - self.assertEqual(fmt.type_row(t), ("ops", "", "")) + self.assertEqual(fmt.type_row(t), ("ops", "")) class DocRowTests(unittest.TestCase): - def test_returns_name_description_systems_owner_modified(self): + def test_returns_name_systems_owner_modified(self): more_info = SimpleNamespace( last_modified_on_date_time=datetime.datetime(2026, 6, 19, 14, 30), owner_username="alice", @@ -44,12 +81,12 @@ def test_returns_name_description_systems_owner_modified(self): ) self.assertEqual( fmt.doc_row(d), - ("Shift Report", "daily notes", "SR, software", "alice", "2026-06-19 14:30"), + ("Shift Report", "SR, software", "alice", "2026-06-19 14:30"), ) def test_none_more_info_and_item_type_list_do_not_raise(self): d = SimpleNamespace(name=None, description=None, item_type_list=None, more_info=None) - self.assertEqual(fmt.doc_row(d), ("(unnamed)", "", "", "", "")) + self.assertEqual(fmt.doc_row(d), ("(unnamed)", "", "", "")) class RowColumnArityTests(unittest.TestCase): @@ -253,11 +290,13 @@ def _entry(self, **overrides): base.update(overrides) return SimpleNamespace(**base) - def test_minimal_entry_has_log_id_and_doc(self): + def test_minimal_entry_has_log_and_document_ids(self): doc = SimpleNamespace(id=1, name="Ops") rows = fmt.entry_metadata_rows(self._entry(entered_by_username=None), doc) + values = dict(rows) labels = [label for label, _ in rows] - self.assertIn("log_id", labels) + self.assertEqual(values["log_id"], "4821") + self.assertEqual(values["log_doc_id"], "1") self.assertIn("doc", labels) self.assertNotIn("by", labels) self.assertNotIn("replies", labels) @@ -273,19 +312,30 @@ def test_replies_and_reactions_shown_when_present(self): self.assertEqual(rows["replies"], "2") self.assertEqual(rows["reactions"], "👍 1") - def test_parent_adds_reply_to_row_right_after_log_id(self): + def test_entry_item_id_precedes_selected_document_id(self): + doc = SimpleNamespace(id=1, name="Ops") + rows = dict(fmt.entry_metadata_rows(self._entry(item_id=42), doc)) + self.assertEqual(rows["log_doc_id"], "42") + + def test_parent_adds_parent_log_id(self): doc = SimpleNamespace(id=1, name="Ops") parent = SimpleNamespace(log_id=100) rows = fmt.entry_metadata_rows(self._entry(), doc, parent=parent) labels = [label for label, _ in rows] - self.assertEqual(labels[0], "log_id") - self.assertEqual(labels[1], "reply to") - self.assertEqual(dict(rows)["reply to"], "100") + self.assertEqual(labels[:3], ["log_id", "log_doc_id", "parent_log_id"]) + self.assertEqual(dict(rows)["parent_log_id"], "100") + + def test_entry_parent_id_precedes_tree_parent(self): + doc = SimpleNamespace(id=1, name="Ops") + parent = SimpleNamespace(log_id=100) + rows = dict(fmt.entry_metadata_rows( + self._entry(parent_log_id=99), doc, parent=parent)) + self.assertEqual(rows["parent_log_id"], "99") - def test_no_parent_omits_reply_to_row(self): + def test_no_parent_omits_parent_log_id(self): doc = SimpleNamespace(id=1, name="Ops") rows = fmt.entry_metadata_rows(self._entry(), doc) - self.assertNotIn("reply to", dict(rows)) + self.assertNotIn("parent_log_id", dict(rows)) class DocMetadataRowsTests(unittest.TestCase): diff --git a/tools/developer_tools/bely-cli/test/test_tui_app.py b/tools/developer_tools/bely-cli/test/test_tui_app.py index f049b4997..79c8be690 100644 --- a/tools/developer_tools/bely-cli/test/test_tui_app.py +++ b/tools/developer_tools/bely-cli/test/test_tui_app.py @@ -37,7 +37,7 @@ def try_token(self): class FakeLogbookApi: - def get_logbook_types(self): + def get_logbook_type_hierarchy(self): return [SimpleNamespace(id=1, name="ops", display_name="Ops")] def get_log_documents(self, logbook_type_id, limit): @@ -160,6 +160,14 @@ def get_search_api(self): class TuiAppSmokeTests(unittest.IsolatedAsyncioTestCase): + async def _open_entries(self, pilot): + await pilot.pause() + await pilot.press("enter") # type -> docs + await pilot.pause() + await pilot.press("enter") # docs -> entries + await pilot.pause() + await pilot.pause() + async def test_browse_populates_list_and_drives_preview(self): data = LogbookData(FakeLogbookApi()) app = BelyTuiApp(FakeSession(data), limit=10, mode="lookup") @@ -168,7 +176,7 @@ async def test_browse_populates_list_and_drives_preview(self): screen = app.screen table = screen.query_one("#nav-table", DataTable) self.assertEqual(table.row_count, 1) - self.assertEqual(screen.shown_items[0].name, "ops") + self.assertEqual(screen.shown_items[0].entity.name, "ops") # No info panel open yet: the table gets the full width, no preview. self.assertFalse(screen.query_one("#preview").display) self.assertEqual(table.styles.width.value, 100) @@ -439,6 +447,23 @@ async def test_new_entry_action_visible_at_docs_and_entries_not_types(self): await pilot.pause() self.assertTrue(screen.check_action("new_entry", ())) + async def test_new_entry_is_unavailable_with_empty_document_list(self): + api = FakeLogbookApi() + api.get_log_documents = lambda logbook_type_id, limit: [] + data = LogbookData(api) + app = BelyTuiApp(FakeSession(data), limit=10, mode="lookup") + async with app.run_test() as pilot: + await pilot.pause() + screen = app.screen + await pilot.press("enter") + await pilot.pause() + await pilot.pause() + self.assertEqual(screen.level, screen.LEVEL_DOCS) + self.assertIsNone(screen.check_action("new_entry", ())) + await pilot.press("n") + await pilot.pause() + self.assertIs(app.screen, screen) + async def test_new_entry_key_creates_entry_and_refreshes(self): api = FakeLogbookApi() data = LogbookData(api) @@ -469,6 +494,46 @@ async def test_new_entry_key_creates_entry_and_refreshes(self): self.assertEqual(screen.level, screen.LEVEL_ENTRIES) self.assertEqual(len(screen.shown_items), 1) + async def test_reply_key_targets_top_level_parent_and_focuses_saved_reply(self): + api = FakeLogbookApiWithReplies() + data = LogbookData(api) + session = FakeSession(data, factory=FakeFactory(api=api)) + app = BelyTuiApp(session, limit=10, mode="lookup") + async with app.run_test() as pilot: + await self._open_entries(pilot) + screen = app.screen + screen._nav().move_cursor(row=1) # select a reply + await pilot.pause() + + await pilot.press("p") + await pilot.pause() + await pilot.pause() + self.assertEqual(type(app.screen).__name__, "ComposeScreen") + self.assertEqual(app.screen.reply_to.log_id, 100) + self.assertEqual(app.screen.entry.parent_log_id, 100) + + from textual.widgets import Button, TextArea + app.screen.query_one("#compose-area", TextArea).text = "new reply" + app.screen.query_one("#compose-save", Button).press() + await pilot.pause() + await pilot.pause() + await pilot.pause() + + self.assertIs(app.screen, screen) + self.assertNotIn(100, screen._collapsed) + self.assertEqual(screen._current_entry().log_id, 101) + + async def test_reply_binding_only_available_for_entries_with_selection(self): + api = FakeLogbookApi() + app = BelyTuiApp(FakeSession(LogbookData(api)), limit=10, mode="lookup") + async with app.run_test() as pilot: + await pilot.pause() + screen = app.screen + self.assertIsNone(screen.check_action("reply", ())) + await self._open_entries(pilot) + self.assertTrue(screen.check_action("reply", ())) + self.assertTrue(screen.check_action("refresh_level", ())) + async def test_update_entry_key_opens_compose_prefilled_and_cancel_returns(self): api = FakeLogbookApi() data = LogbookData(api) diff --git a/tools/developer_tools/bely-cli/test/test_tui_data.py b/tools/developer_tools/bely-cli/test/test_tui_data.py index eda1ff27a..9ad69d716 100644 --- a/tools/developer_tools/bely-cli/test/test_tui_data.py +++ b/tools/developer_tools/bely-cli/test/test_tui_data.py @@ -9,7 +9,7 @@ def __init__(self): self.calls = [] self.fail_next = False - def get_logbook_types(self): + def get_logbook_type_hierarchy(self): self.calls.append(("types",)) if self.fail_next: self.fail_next = False diff --git a/tools/developer_tools/bely-cli/test/test_tui_screens.py b/tools/developer_tools/bely-cli/test/test_tui_screens.py index ab1fcb54a..3c96872b5 100644 --- a/tools/developer_tools/bely-cli/test/test_tui_screens.py +++ b/tools/developer_tools/bely-cli/test/test_tui_screens.py @@ -12,12 +12,12 @@ from unittest.mock import patch from textual.app import App -from textual.widgets import Button, Input, Select, Static, TextArea +from textual.widgets import Button, Input, OptionList, Select, Static, TextArea from bely_cli.tui.app import BelyTuiApp from bely_cli.tui.data import LogbookData from bely_cli.tui.screens import configscreen -from bely_cli.tui.screens.compose import ComposeScreen +from bely_cli.tui.screens.compose import ComposeScreen, open_composer from bely_cli.tui.screens.confirm import ConfirmScreen from bely_cli.tui.screens.configscreen import ConfigScreen from bely_cli.tui.screens.login import LoginScreen @@ -34,7 +34,7 @@ def __init__(self, existing_doc=None): self.created = None self._existing_doc = existing_doc - def get_logbook_types(self): + def get_logbook_type_hierarchy(self): return [SimpleNamespace(id=1, name="ops", display_name="Ops")] def get_logbook_systems(self): @@ -207,6 +207,9 @@ async def test_multi_select_space_toggles_then_enter_confirms(self): await pilot.press("enter") # filter -> list await pilot.pause() await pilot.press("space") # toggle Alpha + await pilot.pause() + picker = app.screen + self.assertTrue(str(picker.query_one("#picker-list", OptionList).get_option_at_index(0).prompt).startswith("☒")) await pilot.press("down") await pilot.press("space") # toggle Beta await pilot.pause() @@ -283,6 +286,58 @@ async def test_confirm_button_dismisses_multi_selection(self): class ComposeScreenTests(unittest.IsolatedAsyncioTestCase): + async def test_reply_loads_blank_template_sets_parent_and_titles_composer(self): + api = FakeLogbookApi() + doc = SimpleNamespace(id=1, name="Doc") + parent = SimpleNamespace(log_id=7) + app = App() + async with app.run_test() as pilot: + task = app.run_worker(open_composer(app, doc, api, reply_to=parent)) + await pilot.pause() + screen = app.screen + self.assertIn("Reply to entry #7", str(screen.query_one("#compose-title", Static).render())) + self.assertEqual(screen.entry.parent_log_id, 7) + screen.query_one("#compose-area", TextArea).text = "reply text" + screen.query_one("#compose-save", Button).press() + await pilot.pause() + await pilot.pause() + saved = await task.wait() + self.assertEqual(saved.parent_log_id, 7) + self.assertEqual(saved.log_entry, "reply text") + + async def test_reply_uses_existing_attachment_flow(self): + api = FakeLogbookApi() + api.upload_attachment = lambda **kwargs: None + doc = SimpleNamespace(id=1, name="Doc") + parent = SimpleNamespace(log_id=7) + app = App() + async with app.run_test() as pilot: + task = app.run_worker(open_composer(app, doc, api, reply_to=parent)) + await pilot.pause() + screen = app.screen + screen.query_one("#compose-area", TextArea).text = "reply text" + screen.query_one("#compose-attach", Input).value = "/tmp/reply.txt" + with patch("bely_cli.core.validate_attachment_path", return_value="/tmp/reply.txt"), \ + patch("bely_cli.core.upload_attachment") as upload: + screen.query_one("#compose-save", Button).press() + await pilot.pause() + await pilot.pause() + saved = await task.wait() + upload.assert_called_once_with(api, 1, saved.log_id, "/tmp/reply.txt") + + async def test_reply_cancel_returns_none(self): + api = FakeLogbookApi() + doc = SimpleNamespace(id=1, name="Doc") + parent = SimpleNamespace(log_id=7) + app = App() + async with app.run_test() as pilot: + task = app.run_worker(open_composer(app, doc, api, reply_to=parent)) + await pilot.pause() + app.screen.query_one("#compose-cancel", Button).press() + await pilot.pause() + result = await task.wait() + self.assertIsNone(result) + async def test_save_button_saves_and_dismisses_with_entry(self): api = FakeLogbookApi() doc = SimpleNamespace(id=1, name="Doc") @@ -698,8 +753,18 @@ async def test_load_prefills_inputs_and_flags_env_overrides(self): screen = app.screen self.assertEqual(screen.query_one("#config-host", Input).value, "https://example") self.assertEqual(screen.query_one("#config-editor", Input).value, "nano") - self.assertIn( - "overridden by BELY_USER", screen.query_one("#config-user", Input).placeholder) + labels = { + field: str(screen.query_one(f"#config-{field}-label", Static).render()) + for field in configscreen.config.VALID_FIELDS + } + self.assertEqual(labels["host"], "Host:") + self.assertEqual(labels["editor"], "Editor:") + self.assertEqual(labels["token_path"], "Token path:") + self.assertEqual(labels["theme"], "Theme:") + self.assertEqual(labels["images"], "Images:") + self.assertIn("User:", labels["user"]) + self.assertIn("overridden by BELY_USER", labels["user"]) + self.assertEqual(screen.query_one("#config-user", Input).placeholder, "") async def test_save_writes_changed_fields_and_warns_on_env_override(self): state = { diff --git a/tools/developer_tools/python-client/packages/api/BelyApiFactory.py b/tools/developer_tools/python-client/packages/api/BelyApiFactory.py index fc76289ff..8ed12e517 100755 --- a/tools/developer_tools/python-client/packages/api/BelyApiFactory.py +++ b/tools/developer_tools/python-client/packages/api/BelyApiFactory.py @@ -8,6 +8,7 @@ from __future__ import annotations import base64 +import json import os import warnings import typing @@ -155,13 +156,30 @@ def logout_user(self): self.auth_api.log_out() def parse_api_exception(self, open_api_exception): + """Parse an API exception response into an ApiExceptionMessage.""" from belyApi import ApiExceptionMessage - response_type = ApiExceptionMessage.__name__ - open_api_exception.data = open_api_exception.body - ex_obj = self.api_client.deserialize(open_api_exception, response_type) - ex_obj.status = open_api_exception.status - return ex_obj + payload = open_api_exception.body + if payload is None: + payload = open_api_exception.data + if isinstance(payload, bytes): + payload = payload.decode("utf-8") + if isinstance(payload, str): + payload = json.loads(payload) + if not isinstance(payload, dict): + raise ValueError("API exception does not contain a JSON object") + + parsed = ApiExceptionMessage.from_dict(payload) + if not parsed.message: + exception = parsed.exception + parsed.message = ( + getattr(exception, "message", None) + or getattr(exception, "localized_message", None) + or parsed.simple_name + or getattr(open_api_exception, "reason", None) + or "API request failed" + ) + return parsed # -- Deprecated camelCase wrappers -- diff --git a/tools/developer_tools/python-client/test/api_factory_test.py b/tools/developer_tools/python-client/test/api_factory_test.py new file mode 100644 index 000000000..2b58df1aa --- /dev/null +++ b/tools/developer_tools/python-client/test/api_factory_test.py @@ -0,0 +1,83 @@ +import json +import unittest +from types import SimpleNamespace + +from BelyApiFactory import BelyApiFactory + + +class ParseApiExceptionTests(unittest.TestCase): + def _factory_without_client(self): + return object.__new__(BelyApiFactory) + + def test_parses_real_server_response_body(self): + payload = { + "simpleName": "ObjectNotFound", + "message": "Could not find item with id: 90099", + "exception": { + "cause": None, + "stackTrace": [{ + "classLoaderName": None, + "moduleName": None, + "moduleVersion": None, + "methodName": "getItemByIdBase", + "fileName": "ItemBaseRoute.java", + "lineNumber": 30, + "className": "gov.anl.aps.logr.rest.routes.ItemBaseRoute", + "nativeMethod": False, + }], + "message": "Could not find item with id: 90099", + "suppressed": [], + "localizedMessage": "Could not find item with id: 90099", + }, + } + error = SimpleNamespace(body=json.dumps(payload), data=None) + + parsed = self._factory_without_client().parse_api_exception(error) + + self.assertEqual(parsed.simple_name, "ObjectNotFound") + self.assertEqual(parsed.message, "Could not find item with id: 90099") + + def test_null_message_falls_back_to_exception_type(self): + error = SimpleNamespace( + body=json.dumps({ + "simpleName": "NullPointerException", + "message": None, + "exception": {"message": None, "localizedMessage": None}, + }), + data=None, + reason="Internal Server Error", + ) + parsed = self._factory_without_client().parse_api_exception(error) + self.assertEqual(parsed.message, "NullPointerException") + + def test_nested_exception_message_precedes_type(self): + error = SimpleNamespace( + body=json.dumps({ + "simpleName": "Failure", + "message": None, + "exception": {"message": "Specific failure"}, + }), + data=None, + reason="Internal Server Error", + ) + parsed = self._factory_without_client().parse_api_exception(error) + self.assertEqual(parsed.message, "Specific failure") + + def test_accepts_bytes_body(self): + error = SimpleNamespace(body=b'{"message":"Denied"}', data=None) + parsed = self._factory_without_client().parse_api_exception(error) + self.assertEqual(parsed.message, "Denied") + + def test_uses_data_when_body_is_missing(self): + error = SimpleNamespace(body=None, data={"message": "Unavailable"}) + parsed = self._factory_without_client().parse_api_exception(error) + self.assertEqual(parsed.message, "Unavailable") + + def test_rejects_non_object_payload(self): + error = SimpleNamespace(body='["invalid"]', data=None) + with self.assertRaisesRegex(ValueError, "JSON object"): + self._factory_without_client().parse_api_exception(error) + + +if __name__ == "__main__": + unittest.main()