Add OCR and sync logging overviews to settings - #67
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds end-to-end visibility for OCR and OneDrive sync (upload) job activity in the Settings UI, bringing those services to parity with the existing File Naming logs by persisting job lifecycle data, exposing new paginated API endpoints, and rendering new log tables in the frontend.
Changes:
- Added persistence for OCR and Sync job lifecycles (
ocr_jobspopulation + newsync_jobstable) fromocr_serviceandupload_service. - Added new paginated/filterable API endpoints (
/api/ocr-logs,/api/sync-logs) using a shared_fetch_job_logshelper. - Extended the Settings UI with OCR and Sync log accordions and refactored frontend log rendering into a reusable
createLogsTablefactory.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| web_service/src/templates/settings-tab/settings-tab-onedrive.html | Adds a Sync Logs accordion/table to the OneDrive settings tab. |
| web_service/src/templates/settings-tab/settings-tab-ocr.html | Replaces placeholder OCR text with an OCR Logs accordion/table. |
| web_service/src/static/js/settings.js | Introduces createLogsTable/initLogTables and shared rendering helpers for all log tables. |
| web_service/src/routes/api.py | Adds _fetch_job_logs plus new /api/ocr-logs and /api/sync-logs endpoints. |
| upload_service/main.py | Inserts/updates sync_jobs records across the upload lifecycle. |
| tests/test_logs_api.py | Adds API tests for OCR/sync logs pagination and filtering. |
| scansynclib/scansynclib/ProcessItem.py | Adds sync_db_id field to ProcessItem. |
| scansynclib/scansynclib/db/schema.sql | Adds the new sync_jobs table schema. |
| ocr_service/main.py | Inserts ocr_jobs rows at start and finalizes status/error on completion/failure. |
| page = int(request.args.get('page', 1)) | ||
| per_page = int(request.args.get('per_page', 20)) | ||
| filter = request.args.get('filter', 'all').lower() | ||
| offset = (page - 1) * per_page | ||
There was a problem hiding this comment.
Fixed in the latest commit. Both params are now parsed inside try/except (ValueError, TypeError) blocks with safe defaults. page is clamped to max(1, ...) with no upper limit, and per_page is clamped to max(1, min(100, ...)) to prevent zero/negative values and unbounded queries.
| logger.exception(f"Failed processing {body}.") | ||
| item.status = ProcessStatus.SYNC_FAILED | ||
| update_scanneddata_database(item, {"file_status": item.status.value}) | ||
| finalize_sync_job(item, "Unexpected error during upload") |
There was a problem hiding this comment.
Fixed in the latest commit. item is now initialized to None before the try block. The except clause checks item is not None and isinstance(item, ProcessItem) before updating status/DB, and additionally checks item.sync_db_id is not None before calling finalize_sync_job.
| # Mock Redis before any scansynclib imports, since settings.py connects at module level | ||
| import redis as _real_redis | ||
| _orig_from_url = _real_redis.Redis.from_url | ||
| def _mock_from_url(*args, **kwargs): | ||
| mock_client = MagicMock() | ||
| mock_client.get.return_value = None # No existing settings in Redis | ||
| mock_client.set.return_value = True | ||
| mock_client.publish.return_value = 0 | ||
| mock_pubsub = MagicMock() | ||
| mock_pubsub.subscribe.return_value = None | ||
| mock_pubsub.listen.return_value = iter([]) # Empty iterator | ||
| mock_client.pubsub.return_value = mock_pubsub | ||
| return mock_client | ||
| _real_redis.Redis.from_url = _mock_from_url | ||
There was a problem hiding this comment.
Fixed in the latest commit. The module-level assignment and the unused _orig_from_url have been removed. The patch is now applied and torn down in a @pytest.fixture(scope="session", autouse=True) fixture that saves the original, yields, and restores it—preventing any leak into other test modules.
…lback, use autouse fixture for Redis mock
maxi07
commented
Jul 2, 2026
@copilot I dont like your addition of the our status text within a JS file and the except blocks within ocr_service/main.py. Please, similar to the ProcessItem definition, define all the error descriptions for the OCR failures once and make them reusable, so we can display the message also to the user in the web instead of creating new error messages in the js file with |
…s in ocr_service and expose via API
Done. Here's what changed:
|
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
maxi07
commented
Jul 2, 2026
@copilot resolve the merge conflicts in this pull request |
…sItem and ocr_service
Done. Merged
|
Only file naming jobs were surfaced in the web UI; OCR and sync (upload) jobs had no visibility. This adds OCR and Sync log tables mirroring the existing File Naming logs.
Persistence
sync_jobstable added toschema.sql; the pre-existing but unusedocr_jobstable is now populated.ocr_servicewrites aPROCESSINGrow on start and finalizes with status + a human-readable error on every completion/failure branch.upload_servicerecords the sync lifecycle (including missing-OCR-file and upload-failure paths) with asuccessflag and error description;ProcessItemgainssync_db_id.API
GET /api/ocr-logsandGET /api/sync-logs, paginated and filterable (all/success/failed), via a shared_fetch_job_logshelper. Table/filter fragments are hardcoded constants; only pagination values are bound parameters.Frontend
createLogsTablefactory (own pagination/filter state, lazy-load on expand) driving all three tables:data-fulltextattribute +addEventListenerrather than inlineonclickstring interpolation, avoiding injection from unescaped backslashes.Tests
tests/test_logs_api.pycovers pagination, filters, null-count handling, and error responses for both endpoints.Note: the click-to-expand uses
alert()for parity with the existing File Naming table; a more accessible modal is left as a potential follow-up.