UI dialogs: operation context & feedback system

.github/skills/propose-fixes/SKILL.md

@@ -0,0 +1,11 @@
+---
+name: propose-fixes
+description: Guide for proposing fixes for review findings.
+---
+
+When asked to propose fixes for review findings:
+
+- Don't overengineer the fixes
+- Consider the broader impact of the fixes
+- Prefer simple and effective solutions
+- Prefer low-risk and low-effort fixes

.github/skills/review-code/SKILL.md

@@ -0,0 +1,37 @@
+---
+name: review-code
+description: Guide for reviewing code changes, new implementations, or planning documents. Use this when asked to review.
+---
+
+When asked to review code, changes, new implementations, or planning documents:
+
+How the code should be, ideally:
+
+- Robust
+- Maintainable
+- Secure
+- Performant
+- Readable
+- Well-structured
+- Testable
+
+Multiple agents:
+
+- Use one agent to perform the actual review and another agent to challenge the findings of the first agent.
+
+Things to look for:
+
+- Code correctness and logic
+- Code readability and maintainability
+- Adherence to coding standards and best practices
+- Potential performance issues
+- Security vulnerabilities
+- Proper error handling and logging
+- Test coverage and quality
+- Documentation and comments
+
+Things to avoid:
+
+- Focusing on minor issues at the expense of more significant problems
+- Ignoring the broader context of the code changes
+- Overengineering

Dockerfile

@@ -139,6 +139,7 @@ COPY --chown=vscode:vscode archive-contract/ ./archive-contract/
 COPY --chown=vscode:vscode companion/ ./companion/
 COPY --chown=vscode:vscode backend/ ./backend/
 COPY --chown=vscode:vscode archive_testdata/ ./archive_testdata/
+COPY --chown=vscode:vscode shared/ ./shared/
 COPY --chown=vscode:vscode VERSION ./VERSION
 COPY --chown=vscode:vscode .github/ ./.github/
 COPY --chown=vscode:vscode scripts/ ./scripts/
@@ -165,6 +166,7 @@ RUN --mount=type=cache,target=/root/.cache/pip \
 # The production image needs only the application package and startup guards.
 # Tests, development requirements, and backend-local configuration stay out.
 COPY --link backend/app ./app
+COPY --link shared ./shared
 COPY --link scripts/preflight-archive-v2-cutover scripts/reset-archive-v2-cutover-state ./scripts/
 RUN install -d -o sambee -g sambee /app/data
 

TODO.md

@@ -2,18 +2,6 @@
 
 ## Bugs
 
-- Image viewer: while swiping through images in a directory (not an archive), rendering the last image (in either direction) times out.
-
-## Settings
-
-### Styling
-
-- Apply the new dialog styling to dialogs we missed previously:
-   - "Choose viewer"
-- Apply the new dialog styling to the settings pages, too, e.g.:
-   - 2-column design on desktop, 1-column on smaller devices
-   - background colors, etc.
-
 ## Misc. commands
 
 - Add a command to open the current file list location in a new browser tab (which keyboard shortcut to assign? Ctrl+(Shift)+Tab is needed by the browser)

VERSION

@@ -1 +1 @@
-1.0.21
+1.0.22

backend/app/api/auth.py

@@ -25,7 +25,7 @@
     is_user_expired,
     verify_password,
 )
-from app.db.database import get_session
+from app.db.database import get_immediate_session, get_session
 from app.models.oidc import OidcBrowserSession, OidcBrowserSessionStatus, OidcFlowPurpose, OidcIdentity, OidcProviderConfiguration
 from app.models.oidc_api import OidcBrowserSessionListRead, OidcBrowserSessionRead, OidcBrowserSessionRevokeRead, OidcGrantExchangeRequest
 from app.models.user import (
@@ -39,7 +39,7 @@
     build_current_user_read,
     normalize_utc_datetime,
 )
-from app.models.user_settings import CurrentUserSettingsRead, CurrentUserSettingsUpdate
+from app.models.user_settings import CurrentUserSettingsRead, CurrentUserSettingsUpdate, CurrentUserSettingsUpdateResult
 from app.services.audit import AuditDetails, AuditEventName, AuditResult, write_audit_event
 from app.services.authentication_config import (
     build_public_auth_configuration,
@@ -1102,20 +1102,18 @@ async def get_current_user_settings(
     return build_current_user_settings_read(user_id=current_user.id, session=session)
 
 
-@router.put("/me/settings", response_model=CurrentUserSettingsRead)
+@router.put("/me/settings", response_model=CurrentUserSettingsUpdateResult)
 async def put_current_user_settings(
     payload: CurrentUserSettingsUpdate,
+    session: Session = Depends(get_immediate_session),
     current_user: User = Depends(get_current_user_with_auth_check),
-    session: Session = Depends(get_session),
-) -> CurrentUserSettingsRead:
+) -> CurrentUserSettingsUpdateResult:
     set_user(current_user.username)
     try:
-        update_current_user_settings(user_id=current_user.id, payload=payload, session=session)
+        return update_current_user_settings(user_id=current_user.id, payload=payload, session=session)
     except ValueError as exc:
         raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc
 
-    return build_current_user_settings_read(user_id=current_user.id, session=session)
-
 
 #
 # change_password

backend/app/api/system_settings.py

@@ -6,18 +6,21 @@
 from app.core.authorization import Capability
 from app.core.logging import set_user
 from app.core.security import require_capability
-from app.db.database import get_session
+from app.db.database import get_immediate_session, get_session
 from app.models.system_settings import (
     AboutSettingsRead,
     AdvancedSystemSettingsRead,
-    AdvancedSystemSettingsUpdate,
+    AdvancedSystemSettingUpdate,
     FileSearchSettingsRead,
     FileSearchSettingsUpdate,
+    FileSearchSettingsUpdateResult,
     NetworkSettingsRead,
     NetworkSettingsUpdate,
+    NetworkSettingsUpdateResult,
     PublicSupportReportRead,
     SmbSettingsRead,
     SmbSettingsUpdate,
+    SmbSettingsUpdateResult,
 )
 from app.models.user import User
 from app.services.system_settings import (
@@ -48,12 +51,12 @@ async def get_file_search_settings(
     return build_file_search_settings_read(session)
 
 
-@router.put("/settings/file-search", response_model=FileSearchSettingsRead)
+@router.put("/settings/file-search", response_model=FileSearchSettingsUpdateResult)
 async def put_file_search_settings(
     payload: FileSearchSettingsUpdate,
+    session: Session = Depends(get_immediate_session),
     current_user: User = Depends(require_capability(Capability.ACCESS_ADMIN_SETTINGS)),
-    session: Session = Depends(get_session),
-) -> FileSearchSettingsRead:
+) -> FileSearchSettingsUpdateResult:
     set_user(current_user.username)
     try:
         return update_file_search_settings(payload, updated_by_user_id=current_user.id, session=session)
@@ -86,18 +89,17 @@ async def get_advanced_system_settings(
     return build_advanced_system_settings_read()
 
 
-@router.put("/settings/advanced", response_model=AdvancedSystemSettingsRead)
+@router.put("/settings/advanced", response_model=AdvancedSystemSettingUpdate)
 async def put_advanced_system_settings(
-    payload: AdvancedSystemSettingsUpdate,
+    payload: AdvancedSystemSettingUpdate,
     current_user: User = Depends(require_capability(Capability.ACCESS_ADMIN_SETTINGS)),
     session: Session = Depends(get_session),
-) -> AdvancedSystemSettingsRead:
+) -> AdvancedSystemSettingUpdate:
     set_user(current_user.username)
     try:
-        update_advanced_system_settings(payload, updated_by_user_id=current_user.id, session=session)
+        return update_advanced_system_settings(payload, updated_by_user_id=current_user.id, session=session)
     except ValueError as exc:
         raise HTTPException(status_code=status.HTTP_400_BAD_REQUEST, detail=str(exc)) from exc
-    return build_advanced_system_settings_read()
 
 
 @router.get("/settings/smb", response_model=SmbSettingsRead)
@@ -108,12 +110,12 @@ async def get_smb_settings(
     return build_smb_settings_read()
 
 
-@router.put("/settings/smb", response_model=SmbSettingsRead)
+@router.put("/settings/smb", response_model=SmbSettingsUpdateResult)
 async def put_smb_settings(
     payload: SmbSettingsUpdate,
     current_user: User = Depends(require_capability(Capability.ACCESS_ADMIN_SETTINGS)),
     session: Session = Depends(get_session),
-) -> SmbSettingsRead:
+) -> SmbSettingsUpdateResult:
     set_user(current_user.username)
     try:
         policy_changed = smb_policy_will_change(payload, session)
@@ -135,12 +137,12 @@ async def get_network_settings(
     return build_network_settings_read(session)
 
 
-@router.put("/settings/network", response_model=NetworkSettingsRead)
+@router.put("/settings/network", response_model=NetworkSettingsUpdateResult)
 async def put_network_settings(
     payload: NetworkSettingsUpdate,
     current_user: User = Depends(require_capability(Capability.ACCESS_ADMIN_SETTINGS)),
     session: Session = Depends(get_session),
-) -> NetworkSettingsRead:
+) -> NetworkSettingsUpdateResult:
     set_user(current_user.username)
     try:
         return update_network_settings(payload, updated_by_user_id=current_user.id, session=session)

backend/app/core/system_setting_definitions.py

@@ -6,7 +6,9 @@
 
 class SystemSettingKey(StrEnum):
     SMB_READ_CHUNK_SIZE_BYTES = "smb.read_chunk_size_bytes"
-    SMB_POLICY = "smb.policy"
+    SMB_AUTHENTICATION_MODE = "smb.authentication_mode"
+    SMB_ENCRYPTION_MODE = "smb.encryption_mode"
+    SMB_CONNECTION_TIMEOUT_SECONDS = "smb.connection_timeout_seconds"
     PREPROCESSOR_IMAGEMAGICK_MAX_FILE_SIZE_BYTES = "preprocessors.imagemagick.max_file_size_bytes"
     PREPROCESSOR_IMAGEMAGICK_TIMEOUT_SECONDS = "preprocessors.imagemagick.timeout_seconds"
     PDF_VIEWER_CACHE_QUOTA_BYTES = "pdf.viewer.cache_quota_bytes"
@@ -21,7 +23,10 @@ class SystemSettingKey(StrEnum):
     PDF_NORMALIZER_QUEUE_WAIT_SECONDS = "pdf.normalizer.queue_wait_seconds"
     PDF_SCREEN_DERIVATIVE_ENABLED = "pdf.screen_derivative.enabled"
     PDF_SCREEN_MAX_DECODED_PIXELS = "pdf.screen_derivative.max_decoded_pixels"
-    FILE_SEARCH_POLICY = "file_search.policy"
+    FILE_SEARCH_RETENTION_LIMIT = "file_search.retention_limit"
+    FILE_SEARCH_RESULT_LIMIT = "file_search.result_limit"
+    FILE_SEARCH_EXCLUDED_CATEGORIES = "file_search.excluded_categories"
+    FILE_SEARCH_EXCLUDED_EXTENSIONS = "file_search.excluded_extensions"
 
 
 class SystemSettingSource(StrEnum):

backend/app/db/database.py

@@ -1,6 +1,7 @@
 import os
 from typing import Any, Generator
 
+from fastapi import Depends
 from sqlalchemy import event
 from sqlalchemy.engine import Engine
 from sqlalchemy.pool import StaticPool
@@ -128,3 +129,11 @@ def get_session() -> Generator[Session, None, None]:
 
     with Session(engine) as session:
         yield session
+
+
+def get_immediate_session(session: Session = Depends(get_session)) -> Session:
+    bind = session.get_bind()
+    bind_engine = bind if isinstance(bind, Engine) else bind.engine
+    if bind.dialect.name == "sqlite" and not isinstance(bind_engine.pool, StaticPool):
+        session.connection().exec_driver_sql("BEGIN IMMEDIATE")
+    return session

backend/app/db/migrations.py

@@ -1,5 +1,6 @@
 from __future__ import annotations
 
+import json
 from collections.abc import Callable
 from dataclasses import dataclass
 
@@ -897,6 +898,74 @@ def _apply_transfer_operations_migration(connection: Connection) -> None:
         connection.execute(text(f"CREATE INDEX IF NOT EXISTS ix_transfer_operations_{column} ON transfer_operations ({column})"))
 
 
+def _apply_per_field_system_settings_migration(connection: Connection) -> None:
+    """Replace legacy policy blobs with independently persisted setting rows."""
+
+    if not inspect(connection).has_table("systemsetting"):
+        return
+
+    from app.models.system_settings import FileSearchSettings, SmbPolicySettings
+
+    legacy_settings = (
+        (
+            "file_search.policy",
+            FileSearchSettings,
+            {
+                "retention_limit": "file_search.retention_limit",
+                "result_limit": "file_search.result_limit",
+                "excluded_categories": "file_search.excluded_categories",
+                "excluded_extensions": "file_search.excluded_extensions",
+            },
+        ),
+        (
+            "smb.policy",
+            SmbPolicySettings,
+            {
+                "authentication_mode": "smb.authentication_mode",
+                "encryption_mode": "smb.encryption_mode",
+                "connection_timeout_seconds": "smb.connection_timeout_seconds",
+            },
+        ),
+    )
+
+    for legacy_key, model, target_keys in legacy_settings:
+        legacy_row = (
+            connection.execute(
+                text("SELECT key, value, updated_at, updated_by_user_id FROM systemsetting WHERE key = :key"), {"key": legacy_key}
+            )
+            .mappings()
+            .one_or_none()
+        )
+        if legacy_row is None:
+            continue
+
+        try:
+            parsed = model.model_validate(json.loads(str(legacy_row["value"])))
+        except (ValueError, json.JSONDecodeError) as exc:
+            raise RuntimeError(f"Cannot migrate invalid legacy system setting {legacy_key}: {exc}") from exc
+
+        for field, target_key in target_keys.items():
+            value = getattr(parsed, field)
+            serialized = json.dumps(sorted(value), separators=(",", ":")) if isinstance(value, set) else str(value)
+            connection.execute(
+                text(
+                    """
+                    INSERT INTO systemsetting (key, value, updated_at, updated_by_user_id)
+                    SELECT :key, :value, :updated_at, :updated_by_user_id
+                    WHERE NOT EXISTS (SELECT 1 FROM systemsetting WHERE key = :key)
+                    """
+                ),
+                {
+                    "key": target_key,
+                    "value": serialized,
+                    "updated_at": legacy_row["updated_at"],
+                    "updated_by_user_id": legacy_row["updated_by_user_id"],
+                },
+            )
+
+        connection.execute(text("DELETE FROM systemsetting WHERE key = :key"), {"key": legacy_key})
+
+
 MIGRATIONS: tuple[Migration, ...] = (
     Migration(version=1, name="ensure_connection_slugs", apply=_apply_connection_slug_migration),
     Migration(version=2, name="add_user_role_and_session_fields", apply=_apply_user_role_migration),
@@ -934,6 +1003,7 @@ def _apply_transfer_operations_migration(connection: Connection) -> None:
     ),
     Migration(version=33, name="add_archive_operation_member_selection", apply=_apply_archive_operation_member_selection_migration),
     Migration(version=34, name="add_transfer_operations", apply=_apply_transfer_operations_migration),
+    Migration(version=35, name="split_system_setting_policy_blobs", apply=_apply_per_field_system_settings_migration),
 )