fix: robust NC activity filter, title extraction, scan result detail
Watcher: - Accept both NC 22+ (type="file_created") and older NC (subject="created_self") so the upload filter works across all Nextcloud versions - Add .opus to audio_extensions - Fix tests: set nc.username on mocks, use realistic activity dicts with type field - Add tests for old NC style, non-band path filter, normalize_nc_path, cursor advance API: - Fix internal.py title extraction: always use filename stem (was using parts[-2] for >3-part paths, which gave folder name instead of song title) - nc-scan now returns NcScanResult with folder, files_found, imported, skipped counts instead of bare song list — gives the UI actionable feedback Web: - Show rich scan result message: folder scanned, count imported, count already registered Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -53,12 +53,17 @@ async def nc_upload(
|
|||||||
log.warning("nc-upload: band slug '%s' not found in DB", band_slug)
|
log.warning("nc-upload: band slug '%s' not found in DB", band_slug)
|
||||||
return {"status": "skipped", "reason": "band not found"}
|
return {"status": "skipped", "reason": "band not found"}
|
||||||
|
|
||||||
# Determine song title and folder from remaining path segments
|
# Determine song title and folder from path.
|
||||||
# e.g. bands/my-band/songs/session1/rec.mp3 → folder=bands/my-band/songs/session1/, title=session1
|
# The title is always the filename stem (e.g. "take1" from "take1.wav").
|
||||||
# e.g. bands/my-band/rec.mp3 → folder=bands/my-band/, title=rec
|
# The nc_folder groups all versions of the same recording (the parent directory).
|
||||||
|
#
|
||||||
|
# Examples:
|
||||||
|
# bands/my-band/take1.wav → folder=bands/my-band/, title=take1
|
||||||
|
# bands/my-band/231015/take1.wav → folder=bands/my-band/231015/, title=take1
|
||||||
|
# bands/my-band/songs/groove/take1.wav → folder=bands/my-band/songs/groove/, title=take1
|
||||||
parent = str(Path(path).parent)
|
parent = str(Path(path).parent)
|
||||||
nc_folder = parent.rstrip("/") + "/"
|
nc_folder = parent.rstrip("/") + "/"
|
||||||
title = Path(path).stem if len(parts) == 3 else parts[-2]
|
title = Path(path).stem
|
||||||
|
|
||||||
version_repo = AudioVersionRepository(session)
|
version_repo = AudioVersionRepository(session)
|
||||||
if event.nc_file_etag and await version_repo.get_by_etag(event.nc_file_etag):
|
if event.nc_file_etag and await version_repo.get_by_etag(event.nc_file_etag):
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ import uuid
|
|||||||
from pathlib import Path
|
from pathlib import Path
|
||||||
|
|
||||||
from fastapi import APIRouter, Depends, HTTPException, status
|
from fastapi import APIRouter, Depends, HTTPException, status
|
||||||
|
from pydantic import BaseModel
|
||||||
from sqlalchemy.ext.asyncio import AsyncSession
|
from sqlalchemy.ext.asyncio import AsyncSession
|
||||||
|
|
||||||
from rehearsalhub.db.engine import get_session
|
from rehearsalhub.db.engine import get_session
|
||||||
@@ -25,6 +26,14 @@ router = APIRouter(tags=["songs"])
|
|||||||
AUDIO_EXTENSIONS = {".mp3", ".wav", ".flac", ".ogg", ".m4a", ".aac", ".opus"}
|
AUDIO_EXTENSIONS = {".mp3", ".wav", ".flac", ".ogg", ".m4a", ".aac", ".opus"}
|
||||||
|
|
||||||
|
|
||||||
|
class NcScanResult(BaseModel):
|
||||||
|
folder: str
|
||||||
|
files_found: int
|
||||||
|
imported: int
|
||||||
|
skipped: int
|
||||||
|
songs: list[SongRead]
|
||||||
|
|
||||||
|
|
||||||
@router.get("/bands/{band_id}/songs", response_model=list[SongRead])
|
@router.get("/bands/{band_id}/songs", response_model=list[SongRead])
|
||||||
async def list_songs(
|
async def list_songs(
|
||||||
band_id: uuid.UUID,
|
band_id: uuid.UUID,
|
||||||
@@ -65,7 +74,7 @@ async def create_song(
|
|||||||
return read
|
return read
|
||||||
|
|
||||||
|
|
||||||
@router.post("/bands/{band_id}/nc-scan", response_model=list[SongRead])
|
@router.post("/bands/{band_id}/nc-scan", response_model=NcScanResult)
|
||||||
async def scan_nextcloud(
|
async def scan_nextcloud(
|
||||||
band_id: uuid.UUID,
|
band_id: uuid.UUID,
|
||||||
session: AsyncSession = Depends(get_session),
|
session: AsyncSession = Depends(get_session),
|
||||||
@@ -98,7 +107,8 @@ async def scan_nextcloud(
|
|||||||
return href[len(dav_prefix):]
|
return href[len(dav_prefix):]
|
||||||
return href.lstrip("/")
|
return href.lstrip("/")
|
||||||
|
|
||||||
imported: list[SongRead] = []
|
imported_songs: list[SongRead] = []
|
||||||
|
skipped_count = 0
|
||||||
band_folder = band.nc_folder_path or f"bands/{band.slug}/"
|
band_folder = band.nc_folder_path or f"bands/{band.slug}/"
|
||||||
|
|
||||||
log.info("Starting NC scan for band '%s' in folder '%s'", band.slug, band_folder)
|
log.info("Starting NC scan for band '%s' in folder '%s'", band.slug, band_folder)
|
||||||
@@ -159,6 +169,7 @@ async def scan_nextcloud(
|
|||||||
|
|
||||||
if etag and await version_repo.get_by_etag(etag):
|
if etag and await version_repo.get_by_etag(etag):
|
||||||
log.debug("Skipping '%s' — etag already registered", nc_file_path)
|
log.debug("Skipping '%s' — etag already registered", nc_file_path)
|
||||||
|
skipped_count += 1
|
||||||
continue
|
continue
|
||||||
|
|
||||||
# Find or create song record
|
# Find or create song record
|
||||||
@@ -191,12 +202,21 @@ async def scan_nextcloud(
|
|||||||
|
|
||||||
read = SongRead.model_validate(song)
|
read = SongRead.model_validate(song)
|
||||||
read.version_count = 1
|
read.version_count = 1
|
||||||
imported.append(read)
|
imported_songs.append(read)
|
||||||
label_info = f" [rehearsal: {rehearsal_label}]" if rehearsal_label else ""
|
label_info = f" [rehearsal: {rehearsal_label}]" if rehearsal_label else ""
|
||||||
log.info("Imported '%s' as song '%s'%s", nc_file_path, song_title, label_info)
|
log.info("Imported '%s' as song '%s'%s", nc_file_path, song_title, label_info)
|
||||||
|
|
||||||
log.info("NC scan complete: %d new versions imported", len(imported))
|
log.info(
|
||||||
return imported
|
"NC scan complete for '%s': %d imported, %d skipped (already registered)",
|
||||||
|
band_folder, len(imported_songs), skipped_count,
|
||||||
|
)
|
||||||
|
return NcScanResult(
|
||||||
|
folder=band_folder,
|
||||||
|
files_found=len(to_import),
|
||||||
|
imported=len(imported_songs),
|
||||||
|
skipped=skipped_count,
|
||||||
|
songs=imported_songs,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
# ── Comments ──────────────────────────────────────────────────────────────────
|
# ── Comments ──────────────────────────────────────────────────────────────────
|
||||||
|
|||||||
@@ -16,7 +16,7 @@ class WatcherSettings(BaseSettings):
|
|||||||
poll_interval: int = 30 # seconds
|
poll_interval: int = 30 # seconds
|
||||||
|
|
||||||
# File extensions to watch
|
# File extensions to watch
|
||||||
audio_extensions: list[str] = [".wav", ".mp3", ".flac", ".aac", ".ogg", ".m4a"]
|
audio_extensions: list[str] = [".wav", ".mp3", ".flac", ".aac", ".ogg", ".m4a", ".opus"]
|
||||||
|
|
||||||
|
|
||||||
@lru_cache
|
@lru_cache
|
||||||
|
|||||||
@@ -16,11 +16,15 @@ log = logging.getLogger("watcher.event_loop")
|
|||||||
# Persist last seen activity ID in-process (good enough for a POC)
|
# Persist last seen activity ID in-process (good enough for a POC)
|
||||||
_last_activity_id: int = 0
|
_last_activity_id: int = 0
|
||||||
|
|
||||||
# Nextcloud activity app subject names for file create/modify events.
|
# Nextcloud Activity API v2 filter sets.
|
||||||
# These differ across NC versions; we accept all known variants.
|
#
|
||||||
|
# NC 22+ returns: type="file_created"|"file_changed" (subject is human-readable)
|
||||||
|
# NC <22 returns: type="files" (subject is a machine key like "created_self")
|
||||||
|
#
|
||||||
|
# We accept either style so the watcher works across NC versions.
|
||||||
|
_UPLOAD_TYPES = {"file_created", "file_changed"}
|
||||||
|
|
||||||
_UPLOAD_SUBJECTS = {
|
_UPLOAD_SUBJECTS = {
|
||||||
"file_created",
|
|
||||||
"file_changed",
|
|
||||||
"created_by",
|
"created_by",
|
||||||
"changed_by",
|
"changed_by",
|
||||||
"created_public",
|
"created_public",
|
||||||
@@ -110,11 +114,11 @@ async def poll_once(nc_client: NextcloudWatcherClient, settings: WatcherSettings
|
|||||||
for activity in activities:
|
for activity in activities:
|
||||||
activity_id = int(activity.get("activity_id", 0))
|
activity_id = int(activity.get("activity_id", 0))
|
||||||
activity_type = activity.get("type", "")
|
activity_type = activity.get("type", "")
|
||||||
subject = activity.get("subject", "") # human-readable, for logging only
|
subject = activity.get("subject", "")
|
||||||
raw_path = extract_nc_file_path(activity)
|
raw_path = extract_nc_file_path(activity)
|
||||||
|
|
||||||
log.debug(
|
log.debug(
|
||||||
"Activity id=%d type=%r subject=%r path=%r",
|
"Activity id=%d type=%r subject=%r raw_path=%r",
|
||||||
activity_id, activity_type, subject, raw_path,
|
activity_id, activity_type, subject, raw_path,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -142,8 +146,11 @@ async def poll_once(nc_client: NextcloudWatcherClient, settings: WatcherSettings
|
|||||||
)
|
)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
if activity_type not in _UPLOAD_SUBJECTS:
|
if activity_type not in _UPLOAD_TYPES and subject not in _UPLOAD_SUBJECTS:
|
||||||
log.debug("Skipping activity %d: type %r not a file upload event", activity_id, activity_type)
|
log.debug(
|
||||||
|
"Skipping activity %d: type=%r subject=%r — not a file upload event",
|
||||||
|
activity_id, activity_type, subject,
|
||||||
|
)
|
||||||
continue
|
continue
|
||||||
|
|
||||||
log.info("Detected audio upload: %s (activity %d)", nc_path, activity_id)
|
log.info("Detected audio upload: %s (activity %d)", nc_path, activity_id)
|
||||||
|
|||||||
@@ -8,6 +8,7 @@ from watcher.event_loop import (
|
|||||||
extract_nc_file_path,
|
extract_nc_file_path,
|
||||||
is_audio_file,
|
is_audio_file,
|
||||||
is_band_audio_path,
|
is_band_audio_path,
|
||||||
|
normalize_nc_path,
|
||||||
poll_once,
|
poll_once,
|
||||||
)
|
)
|
||||||
|
|
||||||
@@ -27,6 +28,22 @@ def test_is_band_audio_path():
|
|||||||
assert not is_band_audio_path("/")
|
assert not is_band_audio_path("/")
|
||||||
|
|
||||||
|
|
||||||
|
def test_normalize_nc_path_strips_dav_prefix():
|
||||||
|
assert normalize_nc_path("remote.php/dav/files/ncadmin/bands/myband/take.wav", "ncadmin") == "bands/myband/take.wav"
|
||||||
|
|
||||||
|
|
||||||
|
def test_normalize_nc_path_strips_user_files_prefix():
|
||||||
|
assert normalize_nc_path("ncadmin/files/bands/myband/take.wav", "ncadmin") == "bands/myband/take.wav"
|
||||||
|
|
||||||
|
|
||||||
|
def test_normalize_nc_path_strips_files_prefix():
|
||||||
|
assert normalize_nc_path("files/bands/myband/take.wav", "ncadmin") == "bands/myband/take.wav"
|
||||||
|
|
||||||
|
|
||||||
|
def test_normalize_nc_path_already_relative():
|
||||||
|
assert normalize_nc_path("bands/myband/take.wav", "ncadmin") == "bands/myband/take.wav"
|
||||||
|
|
||||||
|
|
||||||
def test_extract_nc_file_path_from_objects():
|
def test_extract_nc_file_path_from_objects():
|
||||||
activity = {"objects": {"42": "/bands/foo/songs/bar/take.wav"}}
|
activity = {"objects": {"42": "/bands/foo/songs/bar/take.wav"}}
|
||||||
path = extract_nc_file_path(activity)
|
path = extract_nc_file_path(activity)
|
||||||
@@ -45,16 +62,25 @@ def test_extract_nc_file_path_returns_none_when_missing():
|
|||||||
assert path is None
|
assert path is None
|
||||||
|
|
||||||
|
|
||||||
|
def _make_nc_mock(username: str = "admin") -> AsyncMock:
|
||||||
|
"""Return a properly configured NextcloudWatcherClient mock."""
|
||||||
|
from watcher.nc_client import NextcloudWatcherClient
|
||||||
|
nc = AsyncMock(spec=NextcloudWatcherClient)
|
||||||
|
nc.username = username
|
||||||
|
return nc
|
||||||
|
|
||||||
|
|
||||||
|
# ── poll_once: NC 22+ style (type field) ──────────────────────────────────────
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_poll_once_ignores_non_audio_files(settings):
|
async def test_poll_once_ignores_non_audio_files(settings):
|
||||||
from watcher.nc_client import NextcloudWatcherClient
|
nc = _make_nc_mock()
|
||||||
|
|
||||||
nc = AsyncMock(spec=NextcloudWatcherClient)
|
|
||||||
nc.get_activities.return_value = [
|
nc.get_activities.return_value = [
|
||||||
{
|
{
|
||||||
"activity_id": 1,
|
"activity_id": 1,
|
||||||
"subject": "file_created",
|
"type": "file_created",
|
||||||
"objects": {"1": "/bands/foo/songs/bar/image.jpg"},
|
"subject": "created_self",
|
||||||
|
"objects": {"1": "bands/foo/songs/bar/image.jpg"},
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
|
||||||
@@ -64,15 +90,14 @@ async def test_poll_once_ignores_non_audio_files(settings):
|
|||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_poll_once_registers_audio_upload(settings):
|
async def test_poll_once_registers_audio_upload_new_nc_style(settings):
|
||||||
from watcher.nc_client import NextcloudWatcherClient
|
nc = _make_nc_mock()
|
||||||
|
|
||||||
nc = AsyncMock(spec=NextcloudWatcherClient)
|
|
||||||
nc.get_activities.return_value = [
|
nc.get_activities.return_value = [
|
||||||
{
|
{
|
||||||
"activity_id": 5,
|
"activity_id": 5,
|
||||||
"subject": "file_created",
|
"type": "file_created",
|
||||||
"objects": {"10": "/bands/myband/songs/mysong/take1.wav"},
|
"subject": "created_self",
|
||||||
|
"objects": {"10": "bands/myband/songs/mysong/take1.wav"},
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
nc.get_file_etag.return_value = "abc123"
|
nc.get_file_etag.return_value = "abc123"
|
||||||
@@ -82,22 +107,63 @@ async def test_poll_once_registers_audio_upload(settings):
|
|||||||
) as mock_register:
|
) as mock_register:
|
||||||
await poll_once(nc, settings)
|
await poll_once(nc, settings)
|
||||||
mock_register.assert_called_once_with(
|
mock_register.assert_called_once_with(
|
||||||
"/bands/myband/songs/mysong/take1.wav",
|
"bands/myband/songs/mysong/take1.wav",
|
||||||
"abc123",
|
"abc123",
|
||||||
settings.api_url,
|
settings.api_url,
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_poll_once_ignores_non_file_events(settings):
|
async def test_poll_once_registers_audio_upload_old_nc_style(settings):
|
||||||
from watcher.nc_client import NextcloudWatcherClient
|
"""Old NC: type='files', subject='created_self' (machine key)."""
|
||||||
|
nc = _make_nc_mock()
|
||||||
|
nc.get_activities.return_value = [
|
||||||
|
{
|
||||||
|
"activity_id": 7,
|
||||||
|
"type": "files",
|
||||||
|
"subject": "created_self",
|
||||||
|
"objects": {"11": "bands/myband/231015/take1.wav"},
|
||||||
|
}
|
||||||
|
]
|
||||||
|
nc.get_file_etag.return_value = "etag999"
|
||||||
|
|
||||||
nc = AsyncMock(spec=NextcloudWatcherClient)
|
with patch(
|
||||||
|
"watcher.event_loop.register_version_with_api", new_callable=AsyncMock, return_value=True
|
||||||
|
) as mock_register:
|
||||||
|
await poll_once(nc, settings)
|
||||||
|
mock_register.assert_called_once_with(
|
||||||
|
"bands/myband/231015/take1.wav",
|
||||||
|
"etag999",
|
||||||
|
settings.api_url,
|
||||||
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_poll_once_ignores_non_file_events(settings):
|
||||||
|
nc = _make_nc_mock()
|
||||||
nc.get_activities.return_value = [
|
nc.get_activities.return_value = [
|
||||||
{
|
{
|
||||||
"activity_id": 2,
|
"activity_id": 2,
|
||||||
"subject": "shared", # not file_created or file_changed
|
"type": "shared_with_user",
|
||||||
"objects": {"5": "/bands/foo/songs/bar/take.wav"},
|
"subject": "shared_with_you",
|
||||||
|
"objects": {"5": "bands/foo/songs/bar/take.wav"},
|
||||||
|
}
|
||||||
|
]
|
||||||
|
|
||||||
|
with patch("watcher.event_loop.register_version_with_api") as mock_register:
|
||||||
|
await poll_once(nc, settings)
|
||||||
|
mock_register.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_poll_once_ignores_non_band_audio_files(settings):
|
||||||
|
nc = _make_nc_mock()
|
||||||
|
nc.get_activities.return_value = [
|
||||||
|
{
|
||||||
|
"activity_id": 3,
|
||||||
|
"type": "file_created",
|
||||||
|
"subject": "created_self",
|
||||||
|
"objects": {"6": "Music/personal/track.mp3"},
|
||||||
}
|
}
|
||||||
]
|
]
|
||||||
|
|
||||||
@@ -108,11 +174,36 @@ async def test_poll_once_ignores_non_file_events(settings):
|
|||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_poll_once_empty_activities_does_nothing(settings):
|
async def test_poll_once_empty_activities_does_nothing(settings):
|
||||||
from watcher.nc_client import NextcloudWatcherClient
|
nc = _make_nc_mock()
|
||||||
|
|
||||||
nc = AsyncMock(spec=NextcloudWatcherClient)
|
|
||||||
nc.get_activities.return_value = []
|
nc.get_activities.return_value = []
|
||||||
|
|
||||||
with patch("watcher.event_loop.register_version_with_api") as mock_register:
|
with patch("watcher.event_loop.register_version_with_api") as mock_register:
|
||||||
await poll_once(nc, settings)
|
await poll_once(nc, settings)
|
||||||
mock_register.assert_not_called()
|
mock_register.assert_not_called()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_poll_once_advances_cursor(settings):
|
||||||
|
nc = _make_nc_mock()
|
||||||
|
nc.get_activities.return_value = [
|
||||||
|
{
|
||||||
|
"activity_id": 10,
|
||||||
|
"type": "file_created",
|
||||||
|
"subject": "created_self",
|
||||||
|
"objects": {"1": "bands/band/cover.jpg"}, # non-audio → skipped
|
||||||
|
},
|
||||||
|
{
|
||||||
|
"activity_id": 20,
|
||||||
|
"type": "file_created",
|
||||||
|
"subject": "created_self",
|
||||||
|
"objects": {"2": "bands/band/doc.pdf"}, # non-audio → skipped
|
||||||
|
},
|
||||||
|
]
|
||||||
|
|
||||||
|
import watcher.event_loop as el
|
||||||
|
el._last_activity_id = 0
|
||||||
|
with patch("watcher.event_loop.register_version_with_api"):
|
||||||
|
await poll_once(nc, settings)
|
||||||
|
|
||||||
|
# Cursor should advance to the highest seen activity_id
|
||||||
|
assert el._last_activity_id == 20
|
||||||
|
|||||||
@@ -26,6 +26,14 @@ interface BandInvite {
|
|||||||
expires_at: string;
|
expires_at: string;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
interface NcScanResult {
|
||||||
|
folder: string;
|
||||||
|
files_found: number;
|
||||||
|
imported: number;
|
||||||
|
skipped: number;
|
||||||
|
songs: SongSummary[];
|
||||||
|
}
|
||||||
|
|
||||||
export function BandPage() {
|
export function BandPage() {
|
||||||
const { bandId } = useParams<{ bandId: string }>();
|
const { bandId } = useParams<{ bandId: string }>();
|
||||||
const qc = useQueryClient();
|
const qc = useQueryClient();
|
||||||
@@ -67,15 +75,17 @@ export function BandPage() {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const scanMutation = useMutation({
|
const scanMutation = useMutation({
|
||||||
mutationFn: () => api.post<SongSummary[]>(`/bands/${bandId}/nc-scan`, {}),
|
mutationFn: () => api.post<NcScanResult>(`/bands/${bandId}/nc-scan`, {}),
|
||||||
onSuccess: (imported) => {
|
onSuccess: (result) => {
|
||||||
qc.invalidateQueries({ queryKey: ["songs", bandId] });
|
qc.invalidateQueries({ queryKey: ["songs", bandId] });
|
||||||
setScanMsg(
|
if (result.imported > 0) {
|
||||||
imported.length > 0
|
setScanMsg(`Imported ${result.imported} new song${result.imported !== 1 ? "s" : ""} from ${result.folder} (${result.skipped} already registered).`);
|
||||||
? `Imported ${imported.length} new song${imported.length !== 1 ? "s" : ""} from Nextcloud.`
|
} else if (result.files_found === 0) {
|
||||||
: "No new audio files found in Nextcloud."
|
setScanMsg(`No audio files found in ${result.folder}.`);
|
||||||
);
|
} else {
|
||||||
setTimeout(() => setScanMsg(null), 4000);
|
setScanMsg(`All ${result.files_found} file${result.files_found !== 1 ? "s" : ""} in ${result.folder} already registered.`);
|
||||||
|
}
|
||||||
|
setTimeout(() => setScanMsg(null), 6000);
|
||||||
},
|
},
|
||||||
onError: (err) => setScanMsg(err instanceof Error ? err.message : "Scan failed"),
|
onError: (err) => setScanMsg(err instanceof Error ? err.message : "Scan failed"),
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user