Refactor: add priority and description fields to conditions
This commit is contained in:
parent
0f7e4effd8
commit
b03d4ab42e
8 changed files with 308 additions and 14 deletions
17
API.md
17
API.md
|
|
@ -1511,12 +1511,18 @@ or an error:
|
|||
"filter": "...",
|
||||
"cli": "...",
|
||||
"extras": {},
|
||||
"enabled": true
|
||||
"enabled": true,
|
||||
"priority": 0,
|
||||
"description": "What this condition does"
|
||||
},
|
||||
...
|
||||
]
|
||||
```
|
||||
|
||||
**Notes**:
|
||||
- Conditions are evaluated in priority order (higher priority first).
|
||||
- Priority defaults to 0 when not specified.
|
||||
|
||||
---
|
||||
|
||||
### PUT /api/conditions
|
||||
|
|
@ -1531,7 +1537,9 @@ or an error:
|
|||
"filter": "availability = 'needs_auth' & channel_id = 'channel_id'",
|
||||
"cli": "--proxy http://myproxy.com:8080",
|
||||
"extras": {},
|
||||
"enabled": true
|
||||
"enabled": true,
|
||||
"priority": 10,
|
||||
"description": "Apply proxy for region-locked videos"
|
||||
},
|
||||
...
|
||||
]
|
||||
|
|
@ -1546,7 +1554,9 @@ or an error:
|
|||
"filter": "availability = 'needs_auth' & channel_id = 'channel_id'",
|
||||
"cli": "--proxy http://myproxy.com:8080",
|
||||
"extras": {},
|
||||
"enabled": true
|
||||
"enabled": true,
|
||||
"priority": 10,
|
||||
"description": "Apply proxy for region-locked videos"
|
||||
},
|
||||
...
|
||||
]
|
||||
|
|
@ -1555,6 +1565,7 @@ or an error:
|
|||
**Notes**:
|
||||
- Disabled conditions (`enabled: false`) will be stored but ignored during matching.
|
||||
- All conditions are enabled by default when the `enabled` field is not provided.
|
||||
- Priority determines check order. Higher priority conditions are checked first.
|
||||
|
||||
---
|
||||
|
||||
|
|
|
|||
|
|
@ -37,6 +37,12 @@ class Condition:
|
|||
enabled: bool = True
|
||||
"""Whether the condition is enabled."""
|
||||
|
||||
priority: int = 0
|
||||
"""Priority of the condition."""
|
||||
|
||||
description: str = ""
|
||||
"""A description of what the condition does."""
|
||||
|
||||
def serialize(self) -> dict:
|
||||
return self.__dict__
|
||||
|
||||
|
|
@ -145,6 +151,14 @@ class Conditions(metaclass=Singleton):
|
|||
item["enabled"] = True
|
||||
need_save = True
|
||||
|
||||
if "priority" not in item:
|
||||
item["priority"] = 0
|
||||
need_save = True
|
||||
|
||||
if "description" not in item:
|
||||
item["description"] = ""
|
||||
need_save = True
|
||||
|
||||
item: Condition = init_class(Condition, item)
|
||||
|
||||
self._items.append(item)
|
||||
|
|
@ -220,6 +234,23 @@ class Conditions(metaclass=Singleton):
|
|||
msg = "Extras must be a dictionary."
|
||||
raise ValueError(msg)
|
||||
|
||||
if item.get("enabled") is not None and not isinstance(item.get("enabled"), bool):
|
||||
msg = "Enabled must be a boolean."
|
||||
raise ValueError(msg)
|
||||
|
||||
if item.get("priority") is not None:
|
||||
priority = item.get("priority")
|
||||
if not isinstance(priority, int):
|
||||
msg = "Priority must be an integer."
|
||||
raise ValueError(msg)
|
||||
if priority < 0:
|
||||
msg = "Priority must be >= 0."
|
||||
raise ValueError(msg)
|
||||
|
||||
if item.get("description") and not isinstance(item.get("description"), str):
|
||||
msg = "Description must be a string."
|
||||
raise ValueError(msg)
|
||||
|
||||
return True
|
||||
|
||||
def save(self, items: list[Condition | dict]) -> "Conditions":
|
||||
|
|
@ -305,7 +336,7 @@ class Conditions(metaclass=Singleton):
|
|||
if len(self._items) < 1 or not info or not isinstance(info, dict) or len(info) < 1:
|
||||
return None
|
||||
|
||||
for item in self.get_all():
|
||||
for item in sorted(self.get_all(), key=lambda x: x.priority, reverse=True):
|
||||
if not item.enabled:
|
||||
continue
|
||||
|
||||
|
|
|
|||
|
|
@ -97,6 +97,12 @@ async def conditions_add(request: Request, encoder: Encoder, notify: EventBus) -
|
|||
if "enabled" not in item:
|
||||
item["enabled"] = True
|
||||
|
||||
if "priority" not in item:
|
||||
item["priority"] = 0
|
||||
|
||||
if "description" not in item:
|
||||
item["description"] = ""
|
||||
|
||||
if not item.get("id", None) or not validate_uuid(item.get("id"), version=4):
|
||||
item["id"] = str(uuid.uuid4())
|
||||
|
||||
|
|
|
|||
|
|
@ -39,6 +39,17 @@
|
|||
"type": "boolean",
|
||||
"description": "Whether the condition is enabled.",
|
||||
"default": true
|
||||
},
|
||||
"priority": {
|
||||
"type": "integer",
|
||||
"description": "Priority of the condition.",
|
||||
"default": 0,
|
||||
"minimum": 0
|
||||
},
|
||||
"description": {
|
||||
"type": "string",
|
||||
"description": "A description of what the condition does.",
|
||||
"default": ""
|
||||
}
|
||||
},
|
||||
"examples": [
|
||||
|
|
@ -49,7 +60,9 @@
|
|||
"extras": {
|
||||
"ignore_download": true
|
||||
},
|
||||
"enabled": true
|
||||
"enabled": true,
|
||||
"priority": 10,
|
||||
"description": "Ignore downloads for videos longer than 1 hour."
|
||||
},
|
||||
{
|
||||
"id": "b3c4d5e6-7890-3456-1cde-f23456789012",
|
||||
|
|
@ -58,7 +71,9 @@
|
|||
"extras": {
|
||||
"set_preset": "audio-only"
|
||||
},
|
||||
"enabled": false
|
||||
"enabled": false,
|
||||
"priority": 0,
|
||||
"description": "Apply audio-only preset for videos in the Music category."
|
||||
}
|
||||
]
|
||||
}
|
||||
|
|
|
|||
|
|
@ -212,6 +212,8 @@ class TestConditions:
|
|||
"cli": "--format worst",
|
||||
"extras": {"category": "short"},
|
||||
"enabled": True,
|
||||
"priority": 0,
|
||||
"description": "Download short videos",
|
||||
},
|
||||
{
|
||||
"id": str(uuid.uuid4()),
|
||||
|
|
@ -220,6 +222,8 @@ class TestConditions:
|
|||
"cli": "--audio-quality 0",
|
||||
"extras": {"type": "audio"},
|
||||
"enabled": False,
|
||||
"priority": 5,
|
||||
"description": "High priority music videos",
|
||||
},
|
||||
]
|
||||
|
||||
|
|
@ -237,11 +241,15 @@ class TestConditions:
|
|||
assert conditions._items[0].cli == "--format worst"
|
||||
assert conditions._items[0].extras == {"category": "short"}
|
||||
assert conditions._items[0].enabled is True
|
||||
assert conditions._items[0].priority == 0
|
||||
assert conditions._items[0].description == "Download short videos"
|
||||
|
||||
# Check second condition
|
||||
assert conditions._items[1].name == "music_videos"
|
||||
assert conditions._items[1].filter == "title ~= 'music'"
|
||||
assert conditions._items[1].enabled is False
|
||||
assert conditions._items[1].priority == 5
|
||||
assert conditions._items[1].description == "High priority music videos"
|
||||
|
||||
def test_load_conditions_without_id(self):
|
||||
"""Test loading conditions that don't have ID (should generate ID)."""
|
||||
|
|
@ -305,6 +313,48 @@ class TestConditions:
|
|||
# Should call save due to changes
|
||||
mock_save.assert_called_once()
|
||||
|
||||
def test_load_conditions_without_priority(self):
|
||||
"""Test loading conditions that don't have priority field (should default to 0)."""
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
file_path = Path(temp_dir) / "no_priority_conditions.json"
|
||||
|
||||
test_data = [{"id": str(uuid.uuid4()), "name": "no_priority_test", "filter": "duration > 60", "extras": {}}]
|
||||
|
||||
file_path.write_text(json.dumps(test_data))
|
||||
|
||||
with patch.object(Conditions, "save") as mock_save:
|
||||
conditions = Conditions(file=file_path)
|
||||
conditions.load()
|
||||
|
||||
# Should have generated priority=0
|
||||
assert len(conditions._items) == 1
|
||||
assert conditions._items[0].priority == 0
|
||||
|
||||
# Should call save due to changes
|
||||
mock_save.assert_called_once()
|
||||
|
||||
def test_load_conditions_without_description(self):
|
||||
"""Test loading conditions that don't have description field (should default to empty string)."""
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
file_path = Path(temp_dir) / "no_description_conditions.json"
|
||||
|
||||
test_data = [
|
||||
{"id": str(uuid.uuid4()), "name": "no_description_test", "filter": "duration > 60", "extras": {}}
|
||||
]
|
||||
|
||||
file_path.write_text(json.dumps(test_data))
|
||||
|
||||
with patch.object(Conditions, "save") as mock_save:
|
||||
conditions = Conditions(file=file_path)
|
||||
conditions.load()
|
||||
|
||||
# Should have generated description=''
|
||||
assert len(conditions._items) == 1
|
||||
assert conditions._items[0].description == ""
|
||||
|
||||
# Should call save due to changes
|
||||
mock_save.assert_called_once()
|
||||
|
||||
def test_load_invalid_json(self):
|
||||
"""Test loading file with invalid JSON."""
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
|
|
@ -449,6 +499,74 @@ class TestConditions:
|
|||
with pytest.raises(ValueError, match="Extras must be a dictionary"):
|
||||
conditions.validate(invalid_condition)
|
||||
|
||||
def test_validate_invalid_enabled_type(self):
|
||||
"""Test validating condition with non-boolean enabled field."""
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
file_path = Path(temp_dir) / "invalid_enabled.json"
|
||||
conditions = Conditions(file=file_path)
|
||||
|
||||
invalid_condition = {
|
||||
"id": str(uuid.uuid4()),
|
||||
"name": "invalid_enabled_test",
|
||||
"filter": "duration > 60",
|
||||
"extras": {},
|
||||
"enabled": "yes", # Should be boolean, not string
|
||||
}
|
||||
|
||||
with pytest.raises(ValueError, match="Enabled must be a boolean"):
|
||||
conditions.validate(invalid_condition)
|
||||
|
||||
def test_validate_invalid_priority_type(self):
|
||||
"""Test validating condition with non-integer priority field."""
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
file_path = Path(temp_dir) / "invalid_priority_type.json"
|
||||
conditions = Conditions(file=file_path)
|
||||
|
||||
invalid_condition = {
|
||||
"id": str(uuid.uuid4()),
|
||||
"name": "invalid_priority_type_test",
|
||||
"filter": "duration > 60",
|
||||
"extras": {},
|
||||
"priority": "10", # Should be integer, not string
|
||||
}
|
||||
|
||||
with pytest.raises(ValueError, match="Priority must be an integer"):
|
||||
conditions.validate(invalid_condition)
|
||||
|
||||
def test_validate_invalid_priority_negative(self):
|
||||
"""Test validating condition with negative priority."""
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
file_path = Path(temp_dir) / "invalid_priority_negative.json"
|
||||
conditions = Conditions(file=file_path)
|
||||
|
||||
invalid_condition = {
|
||||
"id": str(uuid.uuid4()),
|
||||
"name": "invalid_priority_negative_test",
|
||||
"filter": "duration > 60",
|
||||
"extras": {},
|
||||
"priority": -5, # Should be >= 0
|
||||
}
|
||||
|
||||
with pytest.raises(ValueError, match="Priority must be >= 0"):
|
||||
conditions.validate(invalid_condition)
|
||||
|
||||
def test_validate_invalid_description_type(self):
|
||||
"""Test validating condition with non-string description field."""
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
file_path = Path(temp_dir) / "invalid_description.json"
|
||||
conditions = Conditions(file=file_path)
|
||||
|
||||
invalid_condition = {
|
||||
"id": str(uuid.uuid4()),
|
||||
"name": "invalid_description_test",
|
||||
"filter": "duration > 60",
|
||||
"extras": {},
|
||||
"description": 123, # Should be string, not integer
|
||||
}
|
||||
|
||||
with pytest.raises(ValueError, match="Description must be a string"):
|
||||
conditions.validate(invalid_condition)
|
||||
|
||||
def test_validate_invalid_item_type(self):
|
||||
"""Test validating invalid item type."""
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
|
|
@ -661,6 +779,32 @@ class TestConditions:
|
|||
# Should only call match_str once for enabled condition
|
||||
mock_match_str.assert_called_once_with("duration > 120", info_dict)
|
||||
|
||||
@patch("app.library.conditions.match_str")
|
||||
def test_match_priority_sorting(self, mock_match_str):
|
||||
"""Test that conditions are evaluated by priority (highest first)."""
|
||||
# All filters will match, so we test that highest priority wins
|
||||
mock_match_str.return_value = True
|
||||
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
file_path = Path(temp_dir) / "priority_test.json"
|
||||
conditions = Conditions(file=file_path)
|
||||
|
||||
# Add conditions with different priorities (not in priority order)
|
||||
low_priority = Condition(name="low", filter="duration > 60", priority=1)
|
||||
high_priority = Condition(name="high", filter="duration > 60", priority=10)
|
||||
default_priority = Condition(name="default", filter="duration > 60", priority=0)
|
||||
|
||||
# Add in wrong order to verify sorting
|
||||
conditions._items = [low_priority, default_priority, high_priority]
|
||||
|
||||
info_dict = {"duration": 120}
|
||||
result = conditions.match(info_dict)
|
||||
|
||||
# Should match high_priority first (priority=10)
|
||||
assert result is high_priority
|
||||
# Should only call match_str once for highest priority condition
|
||||
mock_match_str.assert_called_once_with("duration > 60", info_dict)
|
||||
|
||||
def test_match_empty_filter(self):
|
||||
"""Test matching with condition that has empty filter."""
|
||||
with tempfile.TemporaryDirectory() as temp_dir:
|
||||
|
|
|
|||
|
|
@ -49,7 +49,7 @@
|
|||
</span>
|
||||
</div>
|
||||
|
||||
<div class="column is-9-tablet is-12-mobile">
|
||||
<div class="column is-12">
|
||||
<div class="field">
|
||||
<label class="label is-inline" for="name">
|
||||
<span class="icon"><i class="fa-solid fa-tag" /></span>
|
||||
|
|
@ -66,7 +66,7 @@
|
|||
</div>
|
||||
</div>
|
||||
|
||||
<div class="column is-3-tablet is-12-mobile">
|
||||
<div class="column is-6-tablet is-12-mobile">
|
||||
<div class="field">
|
||||
<label class="label is-inline" for="enabled">
|
||||
<span class="icon"><i class="fa-solid fa-power-off" /></span>
|
||||
|
|
@ -86,6 +86,23 @@
|
|||
</div>
|
||||
</div>
|
||||
|
||||
<div class="column is-6-tablet is-12-mobile">
|
||||
<div class="field">
|
||||
<label class="label is-inline" for="priority">
|
||||
<span class="icon"><i class="fa-solid fa-sort-numeric-down" /></span>
|
||||
Priority
|
||||
</label>
|
||||
<div class="control">
|
||||
<input type="number" class="input" id="priority" v-model.number="form.priority"
|
||||
:disabled="addInProgress" min="0" placeholder="0">
|
||||
</div>
|
||||
<span class="help">
|
||||
<span class="icon"><i class="fa-solid fa-info" /></span>
|
||||
<span class="is-bold">Higher priority conditions are checked first.</span>
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div class="column is-12">
|
||||
<div class="field">
|
||||
<label class="label is-inline" for="filter">
|
||||
|
|
@ -133,7 +150,7 @@
|
|||
<div class="field">
|
||||
<label class="label is-inline">
|
||||
<span class="icon"><i class="fa-solid fa-plus-square" /></span>
|
||||
Extra Options
|
||||
Extra/Custom Options
|
||||
</label>
|
||||
<div class="content">
|
||||
<div v-if="form.extras && Object.keys(form.extras).length > 0" class="mb-3">
|
||||
|
|
@ -179,7 +196,7 @@
|
|||
<ul>
|
||||
<li>For advanced users only. This feature is meant to be expanded later.</li>
|
||||
<li>Keys must be lowercase with underscores (e.g., custom_field).</li>
|
||||
<li>You must click on Add to actually add the option.</li>
|
||||
<li class="has-text-danger is-bold">You must click on Add to actually add the option.</li>
|
||||
<li>The key <code>ignore_download</code> with value of <code>true</code> will instruct
|
||||
<b>YTPTube</b> to ignore the download and directly mark the item as archived. this is useful
|
||||
to skip certain kind of downloads.
|
||||
|
|
@ -194,6 +211,24 @@
|
|||
</span>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
<div class="column is-12">
|
||||
<div class="field">
|
||||
<label class="label is-inline" for="description">
|
||||
<span class="icon"><i class="fa-solid fa-comment" /></span>
|
||||
Description
|
||||
</label>
|
||||
<div class="control">
|
||||
<textarea class="textarea" id="description" v-model="form.description" :disabled="addInProgress"
|
||||
placeholder="Describe what this condition does" />
|
||||
</div>
|
||||
<span class="help is-bold">
|
||||
<span class="icon"><i class="fa-solid fa-info" /></span>
|
||||
<span>Use this field to help understand the purpose of this condition.</span>
|
||||
</span>
|
||||
</div>
|
||||
</div>
|
||||
|
||||
</div>
|
||||
</div>
|
||||
|
||||
|
|
@ -333,11 +368,22 @@ const test_data = ref<{
|
|||
const showOptions = ref<boolean>(false)
|
||||
const ytDlpOpt = ref<AutoCompleteOptions>([])
|
||||
|
||||
// Initialize extras if not present
|
||||
if (!form.extras) {
|
||||
form.extras = {}
|
||||
}
|
||||
|
||||
if (form.enabled === undefined) {
|
||||
form.enabled = true
|
||||
}
|
||||
|
||||
if (form.priority === undefined) {
|
||||
form.priority = 0
|
||||
}
|
||||
|
||||
if (form.description === undefined) {
|
||||
form.description = ''
|
||||
}
|
||||
|
||||
watch(() => config.ytdlp_options, newOptions => ytDlpOpt.value = newOptions
|
||||
.filter(opt => !opt.ignored)
|
||||
.flatMap(opt => opt.flags.filter(flag => flag.startsWith('--'))
|
||||
|
|
@ -468,6 +514,14 @@ const importItem = async (): Promise<void> => {
|
|||
form.enabled = item.enabled
|
||||
}
|
||||
|
||||
if (item.priority !== undefined) {
|
||||
form.priority = item.priority
|
||||
}
|
||||
|
||||
if (item.description !== undefined) {
|
||||
form.description = item.description
|
||||
}
|
||||
|
||||
import_string.value = ''
|
||||
showImport.value = false
|
||||
} catch (e: any) {
|
||||
|
|
|
|||
|
|
@ -51,6 +51,12 @@
|
|||
<div class="card-header-title is-text-overflow is-block" v-text="cond.name" />
|
||||
<div class="card-header-icon">
|
||||
<div class="field is-grouped">
|
||||
<div class="control" v-if="cond.priority > 0">
|
||||
<span class="tag is-dark">
|
||||
<span class="icon"><i class="fa-solid fa-sort-numeric-down" /></span>
|
||||
<span v-text="cond.priority" />
|
||||
</span>
|
||||
</div>
|
||||
<div class="control" @click="toggleEnabled(cond)">
|
||||
<span class="icon" :class="cond.enabled ? 'has-text-success' : 'has-text-danger'"
|
||||
v-tooltip="`Condition is ${cond.enabled !== false ? 'enabled' : 'disabled'}. Click to toggle.`">
|
||||
|
|
@ -83,6 +89,11 @@
|
|||
</span>
|
||||
</span>
|
||||
</p>
|
||||
<p class="is-clickable" :class="{ 'is-text-overflow': !isExpanded(cond.id, 'description') }"
|
||||
v-if="cond.description" @click="toggleExpand(cond.id, 'description')">
|
||||
<span class="icon"><i class="fa-solid fa-comment" /></span>
|
||||
<span>{{ cond.description }}</span>
|
||||
</p>
|
||||
</div>
|
||||
</div>
|
||||
<div class="card-footer mt-auto">
|
||||
|
|
@ -154,7 +165,27 @@ const toggleForm = ref(false)
|
|||
const isLoading = ref(false)
|
||||
const initialLoad = ref(true)
|
||||
const addInProgress = ref(false)
|
||||
const remove_keys = ['in_progress', 'raw']
|
||||
const remove_keys = ['raw', 'toggle_description']
|
||||
const expandedItems = ref<Record<string, Set<string>>>({})
|
||||
|
||||
const toggleExpand = (itemId: string | undefined, field: string) => {
|
||||
if (!itemId) return
|
||||
|
||||
if (!expandedItems.value[itemId]) {
|
||||
expandedItems.value[itemId] = new Set()
|
||||
}
|
||||
|
||||
if (expandedItems.value[itemId].has(field)) {
|
||||
expandedItems.value[itemId].delete(field)
|
||||
} else {
|
||||
expandedItems.value[itemId].add(field)
|
||||
}
|
||||
}
|
||||
|
||||
const isExpanded = (itemId: string | undefined, field: string): boolean => {
|
||||
if (!itemId) return false
|
||||
return expandedItems.value[itemId]?.has(field) ?? false
|
||||
}
|
||||
|
||||
watch(() => socket.isConnected, async () => {
|
||||
if (socket.isConnected && initialLoad.value) {
|
||||
|
|
@ -203,11 +234,11 @@ const updateItems = async (newItems: ConditionItem[]): Promise<boolean> => {
|
|||
try {
|
||||
addInProgress.value = true
|
||||
|
||||
const validItems = newItems.map(({ id, name, filter, cli, extras, enabled }) => {
|
||||
const validItems = newItems.map(({ id, name, filter, cli, extras, enabled, priority, description }) => {
|
||||
if (!name || !filter) {
|
||||
throw new Error('Name and filter are required.')
|
||||
}
|
||||
return { id, name, filter, cli, extras, enabled }
|
||||
return { id, name, filter, cli, extras, enabled, priority, description }
|
||||
})
|
||||
|
||||
const response = await request('/api/conditions', {
|
||||
|
|
|
|||
2
ui/app/types/conditions.d.ts
vendored
2
ui/app/types/conditions.d.ts
vendored
|
|
@ -5,6 +5,8 @@ export type ConditionItem = {
|
|||
cli: string
|
||||
extras: Record<string, any>
|
||||
enabled: boolean
|
||||
priority: number
|
||||
description: string
|
||||
}
|
||||
|
||||
export type ImportedConditionItem = ConditionItem & {
|
||||
|
|
|
|||
Loading…
Reference in a new issue