mirror of
https://github.com/alexta69/metube.git
synced 2026-09-21 13:35:01 +00:00
feat: let subscriptions be renamed from the list (#1044)
A subscription's name is captured once at subscribe time from the feed's own title, so playlists — particularly the UULF-prefixed channel-uploads playlists — all come back named "Videos" and stay that way. Adds an inline editor on the Name cell, mirroring the existing title-filter edit next to it. The update route already whitelisted `name` and update_subscription already applied it, so this is mostly the missing UI. The backend side is validation: the old `str(changes["name"])` accepted any type, any length and any whitespace, for a value that is persisted and broadcast to every connected client. validate_subscription_name now requires a string, collapses interior whitespace to keep the label single-line, and caps it at 200 characters. The name is display-only — it is used for the subscription list and log lines, never for download paths — so renaming cannot move where files land.
This commit is contained in:
+27
-2
@@ -287,6 +287,24 @@ def validate_title_regex(value: Any) -> str:
|
||||
return s
|
||||
|
||||
|
||||
# The name is a display label the user picks; it is persisted and broadcast to
|
||||
# every connected client, so keep it a bounded single-line string.
|
||||
SUBSCRIPTION_NAME_MAX_LENGTH = 200
|
||||
|
||||
|
||||
def validate_subscription_name(value: Any) -> str:
|
||||
"""Return a stored subscription name, or raise ValueError if unusable."""
|
||||
if not isinstance(value, str):
|
||||
raise ValueError("name must be a string")
|
||||
# Collapse newlines/tabs so a pasted title can't break the table layout.
|
||||
name = " ".join(value.split())
|
||||
if not name:
|
||||
raise ValueError("name must not be empty")
|
||||
if len(name) > SUBSCRIPTION_NAME_MAX_LENGTH:
|
||||
raise ValueError(f"name must be at most {SUBSCRIPTION_NAME_MAX_LENGTH} characters")
|
||||
return name
|
||||
|
||||
|
||||
def _coerce_bool(value: Any) -> bool:
|
||||
"""Accept JSON booleans and common string forms used by API clients."""
|
||||
if isinstance(value, bool):
|
||||
@@ -674,6 +692,13 @@ class SubscriptionManager:
|
||||
return {"status": "ok"}
|
||||
|
||||
async def update_subscription(self, sub_id: str, changes: dict) -> dict:
|
||||
validated_name: Optional[str] = None
|
||||
if "name" in changes:
|
||||
try:
|
||||
validated_name = validate_subscription_name(changes["name"])
|
||||
except ValueError as exc:
|
||||
return {"status": "error", "msg": str(exc)}
|
||||
|
||||
validated_tr: Optional[str] = None
|
||||
if "title_regex" in changes:
|
||||
try:
|
||||
@@ -722,8 +747,8 @@ class SubscriptionManager:
|
||||
sub.enabled = validated_enabled
|
||||
if interval_set:
|
||||
sub.check_interval_minutes = validated_interval
|
||||
if "name" in changes and changes["name"]:
|
||||
sub.name = str(changes["name"])
|
||||
if validated_name is not None:
|
||||
sub.name = validated_name
|
||||
if validated_tr is not None:
|
||||
sub.title_regex = validated_tr
|
||||
if skip_so_set:
|
||||
|
||||
@@ -821,6 +821,76 @@ class SubscriptionPersistenceTests(unittest.IsolatedAsyncioTestCase):
|
||||
self.assertEqual(upd["subscription"]["title_regex"], "foo|bar")
|
||||
self.assertEqual(mgr.list_all()[0].title_regex, "foo|bar")
|
||||
|
||||
async def _add_one_subscription(self, mgr):
|
||||
with patch(
|
||||
"subscriptions.extract_flat_playlist",
|
||||
return_value=(
|
||||
{"_type": "channel", "title": "Videos"},
|
||||
[{"id": "v1", "title": "One", "webpage_url": "https://example.com/v1"}],
|
||||
),
|
||||
):
|
||||
result = await mgr.add_subscription(
|
||||
"https://example.com/playlist?list=UULFabc",
|
||||
check_interval_minutes=60,
|
||||
download_type="video",
|
||||
codec="auto",
|
||||
format="any",
|
||||
quality="best",
|
||||
folder="",
|
||||
custom_name_prefix="",
|
||||
auto_start=True,
|
||||
playlist_item_limit=0,
|
||||
split_by_chapters=False,
|
||||
chapter_template="",
|
||||
subtitle_language="en",
|
||||
subtitle_mode="prefer_manual",
|
||||
)
|
||||
return result["subscription"]["id"]
|
||||
|
||||
async def test_update_subscription_renames(self):
|
||||
"""Issue #1044: UULF-style uploads playlists all come back named 'Videos',
|
||||
so the user needs to be able to relabel them."""
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
mgr = SubscriptionManager(_Config(tmp), _Queue(), _Notifier())
|
||||
sub_id = await self._add_one_subscription(mgr)
|
||||
self.assertEqual(mgr.list_all()[0].name, "Videos")
|
||||
|
||||
upd = await mgr.update_subscription(sub_id, {"name": " Jane's uploads \n"})
|
||||
self.assertEqual(upd["status"], "ok")
|
||||
# Surrounding and interior whitespace is collapsed to keep the name
|
||||
# a single-line label.
|
||||
self.assertEqual(upd["subscription"]["name"], "Jane's uploads")
|
||||
self.assertEqual(mgr.list_all()[0].name, "Jane's uploads")
|
||||
|
||||
async def test_update_subscription_rename_survives_reload(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
cfg = _Config(tmp)
|
||||
mgr = SubscriptionManager(cfg, _Queue(), _Notifier())
|
||||
sub_id = await self._add_one_subscription(mgr)
|
||||
await mgr.update_subscription(sub_id, {"name": "Renamed"})
|
||||
|
||||
reloaded = SubscriptionManager(cfg, _Queue(), _Notifier())
|
||||
self.assertEqual(reloaded.get(sub_id).name, "Renamed")
|
||||
|
||||
async def test_update_subscription_rejects_unusable_name(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
mgr = SubscriptionManager(_Config(tmp), _Queue(), _Notifier())
|
||||
sub_id = await self._add_one_subscription(mgr)
|
||||
|
||||
for bad in ("", " ", "\n\t", 42, None, ["a"], "x" * 201):
|
||||
upd = await mgr.update_subscription(sub_id, {"name": bad})
|
||||
self.assertEqual(upd["status"], "error", f"expected {bad!r} to be rejected")
|
||||
self.assertEqual(mgr.list_all()[0].name, "Videos")
|
||||
|
||||
async def test_update_subscription_accepts_name_at_length_limit(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
mgr = SubscriptionManager(_Config(tmp), _Queue(), _Notifier())
|
||||
sub_id = await self._add_one_subscription(mgr)
|
||||
|
||||
upd = await mgr.update_subscription(sub_id, {"name": "x" * 200})
|
||||
self.assertEqual(upd["status"], "ok")
|
||||
self.assertEqual(mgr.list_all()[0].name, "x" * 200)
|
||||
|
||||
async def test_update_subscription_skip_subscriber_only(self):
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
queue = _Queue()
|
||||
|
||||
+27
-1
@@ -958,7 +958,33 @@
|
||||
[disabled]="downloads.loading"
|
||||
[attr.aria-label]="'Select subscription ' + entry[1].name" />
|
||||
</td>
|
||||
<td>{{ entry[1].name }}</td>
|
||||
<td>
|
||||
@if (editingNameId === entry[0]) {
|
||||
<div class="d-flex flex-wrap gap-1 align-items-center">
|
||||
<input type="text"
|
||||
class="form-control form-control-sm flex-grow-1"
|
||||
[name]="'subName' + entry[0]"
|
||||
[(ngModel)]="nameEditDraft"
|
||||
[maxlength]="subscriptionNameMaxLength"
|
||||
[disabled]="downloads.loading"
|
||||
[attr.aria-label]="'Subscription name for ' + entry[1].name" />
|
||||
<button type="button" class="btn btn-sm btn-outline-secondary"
|
||||
(click)="saveName(entry[0])"
|
||||
[disabled]="downloads.loading">Save</button>
|
||||
<button type="button" class="btn btn-sm btn-outline-secondary"
|
||||
(click)="cancelEditName()"
|
||||
[disabled]="downloads.loading">Cancel</button>
|
||||
</div>
|
||||
} @else {
|
||||
<div class="d-flex flex-wrap gap-1 align-items-center">
|
||||
<span class="text-break">{{ entry[1].name }}</span>
|
||||
<button type="button" class="btn btn-link btn-sm p-0"
|
||||
(click)="beginEditName(entry[0], entry[1].name)"
|
||||
[disabled]="downloads.loading"
|
||||
ngbTooltip="Rename this subscription (display name only; does not affect the download folder)">Edit</button>
|
||||
</div>
|
||||
}
|
||||
</td>
|
||||
<td class="text-break"><a [href]="entry[1].url" target="_blank" rel="noopener">{{ entry[1].url }}</a></td>
|
||||
<td>
|
||||
@if (editingTitleRegexId === entry[0]) {
|
||||
|
||||
+37
-1
@@ -81,7 +81,10 @@ class SubscriptionsServiceStub {
|
||||
return of({});
|
||||
}
|
||||
|
||||
update() {
|
||||
updateCalls: [string, unknown][] = [];
|
||||
|
||||
update(id: string, changes: unknown) {
|
||||
this.updateCalls.push([id, changes]);
|
||||
return of({ status: 'ok' as const });
|
||||
}
|
||||
|
||||
@@ -315,4 +318,37 @@ describe('App', () => {
|
||||
expect(errorSpy).toHaveBeenCalledWith('Invalid subscription title filter (regex)');
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('renames a subscription and closes the inline editor', () => {
|
||||
const fixture = TestBed.createComponent(App);
|
||||
const app = fixture.componentInstance;
|
||||
const subs = TestBed.inject(SubscriptionsService) as unknown as SubscriptionsServiceStub;
|
||||
|
||||
app.beginEditName('sub1', 'Videos');
|
||||
expect(app.editingNameId).toBe('sub1');
|
||||
expect(app.nameEditDraft).toBe('Videos');
|
||||
|
||||
app.nameEditDraft = ' Jane uploads ';
|
||||
app.saveName('sub1');
|
||||
|
||||
expect(subs.updateCalls).toEqual([['sub1', { name: 'Jane uploads' }]]);
|
||||
expect(app.editingNameId).toBeNull();
|
||||
});
|
||||
|
||||
it('blocks renaming a subscription to an empty name', () => {
|
||||
const toasts = TestBed.inject(ToastService);
|
||||
const errorSpy = vi.spyOn(toasts, 'error').mockImplementation(() => undefined);
|
||||
const fixture = TestBed.createComponent(App);
|
||||
const app = fixture.componentInstance;
|
||||
const subs = TestBed.inject(SubscriptionsService) as unknown as SubscriptionsServiceStub;
|
||||
|
||||
app.beginEditName('sub1', 'Videos');
|
||||
app.nameEditDraft = ' ';
|
||||
app.saveName('sub1');
|
||||
|
||||
expect(subs.updateCalls.length).toBe(0);
|
||||
expect(app.editingNameId).toBe('sub1');
|
||||
expect(errorSpy).toHaveBeenCalledWith('Subscription name must not be empty');
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
});
|
||||
|
||||
@@ -102,6 +102,9 @@ export class App implements AfterViewInit, OnInit, OnDestroy {
|
||||
skipSubscriberOnly = false;
|
||||
editingTitleRegexId: string | null = null;
|
||||
titleRegexEditDraft = '';
|
||||
editingNameId: string | null = null;
|
||||
nameEditDraft = '';
|
||||
readonly subscriptionNameMaxLength = 200;
|
||||
cachedSubs: [string, SubscriptionRow][] = [];
|
||||
selectedSubscriptionIds = new Set<string>();
|
||||
checkingSubscriptionIds = new Set<string>();
|
||||
@@ -663,6 +666,34 @@ export class App implements AfterViewInit, OnInit, OnDestroy {
|
||||
});
|
||||
}
|
||||
|
||||
beginEditName(id: string, current: string | undefined) {
|
||||
this.editingNameId = id;
|
||||
this.nameEditDraft = current ?? '';
|
||||
this.cdr.markForCheck();
|
||||
}
|
||||
|
||||
cancelEditName() {
|
||||
this.editingNameId = null;
|
||||
this.nameEditDraft = '';
|
||||
this.cdr.markForCheck();
|
||||
}
|
||||
|
||||
saveName(id: string) {
|
||||
const name = (this.nameEditDraft || '').trim();
|
||||
if (!name) {
|
||||
this.toasts.error('Subscription name must not be empty');
|
||||
return;
|
||||
}
|
||||
this.subscriptionsSvc.update(id, { name }).subscribe((res) => {
|
||||
const error = this.getStatusError(res);
|
||||
if (error) {
|
||||
this.toasts.error(error || 'Update subscription failed');
|
||||
return;
|
||||
}
|
||||
this.cancelEditName();
|
||||
});
|
||||
}
|
||||
|
||||
deleteSubscription(id: string) {
|
||||
this.subscriptionsSvc.delete([id]).subscribe((res) => {
|
||||
const error = this.getStatusError(res);
|
||||
|
||||
Reference in New Issue
Block a user