Add owned_by column for Auto-Sync schedule ownership
The Auto-Sync schedule board was detecting its own automations by
checking `group_name === 'Playlist Auto-Sync' || name.startsWith('Auto-Sync:')`.
That's fragile — renaming the row from the Automations page silently
hands ownership back to the read-only Automation Pipelines tab and the
board stops managing it.
This commit replaces the string convention with an explicit
`automations.owned_by` TEXT column:
- Migration `_add_automation_owned_by_column` adds the column and
backfills `'auto_sync'` for existing rows that match the legacy
`group_name`/`name`-prefix pattern, so users running the migration
don't lose their schedules.
- `database.create_automation` and `database.update_automation` accept
`owned_by` (the latter via its `allowed` kwarg set).
- `core/automation/api.py` forwards `owned_by` on both POST and PUT.
Missing field is left as None, preserving today's behavior for every
caller that doesn't opt in.
- The Auto-Sync schedule board posts `owned_by: 'auto_sync'` and the
detection helper now prefers that signal, falling back to the legacy
name/group convention so any hand-rolled rows still show up.
Tests: three new cases in `tests/automation/test_automation_api.py`
covering create-with-owned-by, create-without (defaults to None), and
update set/clear. The fake DB grew the matching kwarg.
This commit is contained in:
parent
feb6778af4
commit
9b086c5a65
4 changed files with 84 additions and 7 deletions
|
|
@ -172,9 +172,11 @@ def create_automation(
|
|||
return {'error': f'Signal cycle detected: {cycle_path}. This would cause an infinite loop.'}, 400
|
||||
|
||||
group_name = data.get('group_name') or None
|
||||
owned_by = data.get('owned_by') or None
|
||||
auto_id = database.create_automation(
|
||||
name, trigger_type, trigger_config, action_type, action_config,
|
||||
profile_id, notify_type, notify_config, then_actions_json, group_name,
|
||||
owned_by=owned_by,
|
||||
)
|
||||
if auto_id is None:
|
||||
return {'error': 'Failed to create automation'}, 500
|
||||
|
|
@ -217,6 +219,8 @@ def update_automation(
|
|||
update_fields['notify_config'] = json.dumps(data['notify_config'])
|
||||
if 'group_name' in data:
|
||||
update_fields['group_name'] = data['group_name'] or None
|
||||
if 'owned_by' in data:
|
||||
update_fields['owned_by'] = data['owned_by'] or None
|
||||
|
||||
if not update_fields:
|
||||
return {'error': 'No fields to update'}, 400
|
||||
|
|
|
|||
|
|
@ -539,6 +539,7 @@ class MusicDatabase:
|
|||
self._add_automation_system_column(cursor)
|
||||
self._add_automation_then_actions_column(cursor)
|
||||
self._add_automation_group_name_column(cursor)
|
||||
self._add_automation_owned_by_column(cursor)
|
||||
|
||||
# Library issues — user-reported problems with tracks/albums/artists
|
||||
cursor.execute("""
|
||||
|
|
@ -845,6 +846,28 @@ class MusicDatabase:
|
|||
except Exception as e:
|
||||
logger.error(f"Error adding automation group_name column: {e}")
|
||||
|
||||
def _add_automation_owned_by_column(self, cursor):
|
||||
"""Add owned_by column so feature surfaces (Auto-Sync schedule
|
||||
board, future pipeline groups) can recognize automations they
|
||||
manage without relying on fragile name-prefix string matches."""
|
||||
try:
|
||||
cursor.execute("PRAGMA table_info(automations)")
|
||||
cols = [c[1] for c in cursor.fetchall()]
|
||||
if 'owned_by' not in cols:
|
||||
cursor.execute("ALTER TABLE automations ADD COLUMN owned_by TEXT DEFAULT NULL")
|
||||
logger.info("Added owned_by column to automations table")
|
||||
# Backfill existing Auto-Sync automations created via the
|
||||
# name/group-prefix convention so the board keeps managing them.
|
||||
cursor.execute("""
|
||||
UPDATE automations
|
||||
SET owned_by = 'auto_sync'
|
||||
WHERE (group_name = 'Playlist Auto-Sync' OR name LIKE 'Auto-Sync:%')
|
||||
AND owned_by IS NULL
|
||||
""")
|
||||
logger.info(f"Backfilled {cursor.rowcount} existing Auto-Sync automations with owned_by='auto_sync'")
|
||||
except Exception as e:
|
||||
logger.error(f"Error adding automation owned_by column: {e}")
|
||||
|
||||
def _add_automation_then_actions_column(self, cursor):
|
||||
"""Add then_actions column to automations table and migrate existing notify data."""
|
||||
try:
|
||||
|
|
@ -12154,15 +12177,22 @@ class MusicDatabase:
|
|||
def create_automation(self, name: str, trigger_type: str, trigger_config: str,
|
||||
action_type: str, action_config: str, profile_id: int = 1,
|
||||
notify_type: str = None, notify_config: str = '{}',
|
||||
then_actions: str = '[]', group_name: str = None):
|
||||
"""Create a new automation. Returns the new automation ID or None."""
|
||||
then_actions: str = '[]', group_name: str = None,
|
||||
owned_by: str = None):
|
||||
"""Create a new automation. Returns the new automation ID or None.
|
||||
|
||||
``owned_by`` tags an automation as managed by a feature surface
|
||||
(e.g. ``'auto_sync'`` for entries the Playlist Auto-Sync board
|
||||
creates) so that surface can recognize its own rows without
|
||||
scraping the display name.
|
||||
"""
|
||||
try:
|
||||
with self._get_connection() as conn:
|
||||
cursor = conn.cursor()
|
||||
cursor.execute("""
|
||||
INSERT INTO automations (name, trigger_type, trigger_config, action_type, action_config, profile_id, notify_type, notify_config, then_actions, group_name)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
""", (name, trigger_type, trigger_config, action_type, action_config, profile_id, notify_type, notify_config, then_actions, group_name))
|
||||
INSERT INTO automations (name, trigger_type, trigger_config, action_type, action_config, profile_id, notify_type, notify_config, then_actions, group_name, owned_by)
|
||||
VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)
|
||||
""", (name, trigger_type, trigger_config, action_type, action_config, profile_id, notify_type, notify_config, then_actions, group_name, owned_by))
|
||||
conn.commit()
|
||||
return cursor.lastrowid
|
||||
except Exception as e:
|
||||
|
|
@ -12209,7 +12239,7 @@ class MusicDatabase:
|
|||
|
||||
def update_automation(self, automation_id: int, **kwargs) -> bool:
|
||||
"""Update automation fields."""
|
||||
allowed = {'name', 'enabled', 'trigger_type', 'trigger_config', 'action_type', 'action_config', 'next_run', 'notify_type', 'notify_config', 'last_result', 'is_system', 'then_actions', 'group_name'}
|
||||
allowed = {'name', 'enabled', 'trigger_type', 'trigger_config', 'action_type', 'action_config', 'next_run', 'notify_type', 'notify_config', 'last_result', 'is_system', 'then_actions', 'group_name', 'owned_by'}
|
||||
updates = {k: v for k, v in kwargs.items() if k in allowed}
|
||||
if not updates:
|
||||
return False
|
||||
|
|
|
|||
|
|
@ -28,7 +28,8 @@ class _FakeDB:
|
|||
return dict(self.automations[automation_id]) if automation_id in self.automations else None
|
||||
|
||||
def create_automation(self, name, trigger_type, trigger_config, action_type, action_config,
|
||||
profile_id, notify_type, notify_config, then_actions, group_name):
|
||||
profile_id, notify_type, notify_config, then_actions, group_name,
|
||||
owned_by=None):
|
||||
aid = self._next_id
|
||||
self._next_id += 1
|
||||
self.automations[aid] = {
|
||||
|
|
@ -37,6 +38,7 @@ class _FakeDB:
|
|||
'action_config': action_config, 'profile_id': profile_id,
|
||||
'notify_type': notify_type, 'notify_config': notify_config,
|
||||
'then_actions': then_actions, 'group_name': group_name,
|
||||
'owned_by': owned_by,
|
||||
'enabled': 1, 'is_system': 0,
|
||||
}
|
||||
return aid
|
||||
|
|
@ -339,6 +341,41 @@ def test_update_non_trigger_field_preserves_next_run():
|
|||
assert db.automations[1].get('next_run') == '2026-05-25 05:00:00'
|
||||
|
||||
|
||||
def test_create_with_owned_by_persists_marker():
|
||||
"""Auto-Sync schedule board posts `owned_by: 'auto_sync'` so it can
|
||||
recognize its own rows on subsequent reads without name-prefix
|
||||
string scraping."""
|
||||
db = _FakeDB()
|
||||
api.create_automation(db, _FakeEngine(), profile_id=1, data={
|
||||
'name': 'Auto-Sync: Discover Weekly',
|
||||
'trigger_type': 'schedule',
|
||||
'trigger_config': {'interval': 1, 'unit': 'hours'},
|
||||
'action_type': 'playlist_pipeline',
|
||||
'owned_by': 'auto_sync',
|
||||
})
|
||||
assert db.automations[1]['owned_by'] == 'auto_sync'
|
||||
|
||||
|
||||
def test_create_without_owned_by_defaults_to_none():
|
||||
db = _FakeDB()
|
||||
api.create_automation(db, _FakeEngine(), profile_id=1, data={
|
||||
'name': 'Plain', 'trigger_type': 'schedule', 'action_type': 'process_wishlist',
|
||||
})
|
||||
assert db.automations[1]['owned_by'] is None
|
||||
|
||||
|
||||
def test_update_can_set_or_clear_owned_by():
|
||||
db = _FakeDB()
|
||||
db.automations[1] = {
|
||||
'id': 1, 'name': 'a', 'enabled': 1, 'is_system': 0,
|
||||
'trigger_type': 'schedule', 'owned_by': None,
|
||||
}
|
||||
api.update_automation(db, _FakeEngine(), automation_id=1, data={'owned_by': 'auto_sync'})
|
||||
assert db.automations[1]['owned_by'] == 'auto_sync'
|
||||
api.update_automation(db, _FakeEngine(), automation_id=1, data={'owned_by': None})
|
||||
assert db.automations[1]['owned_by'] is None
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# batch_update_group
|
||||
# ---------------------------------------------------------------------------
|
||||
|
|
|
|||
|
|
@ -644,6 +644,11 @@ function autoSyncPlaylistIdFromAutomation(auto) {
|
|||
}
|
||||
|
||||
function autoSyncIsScheduleOwned(auto) {
|
||||
// Primary signal: the explicit owned_by flag the board writes on every
|
||||
// schedule it creates. Falls back to the legacy name/group convention
|
||||
// so rows created before the column existed (or hand-edited from the
|
||||
// Automations page) still get recognized after backfill.
|
||||
if (auto?.owned_by === 'auto_sync') return true;
|
||||
const group = auto?.group_name || '';
|
||||
const name = auto?.name || '';
|
||||
return group === 'Playlist Auto-Sync' || name.startsWith('Auto-Sync:');
|
||||
|
|
@ -970,6 +975,7 @@ async function saveAutoSyncPlaylistSchedule(playlistId, hours) {
|
|||
action_config: { playlist_id: String(playlistId), all: false },
|
||||
then_actions: [],
|
||||
group_name: 'Playlist Auto-Sync',
|
||||
owned_by: 'auto_sync',
|
||||
};
|
||||
try {
|
||||
const res = await fetch(existing ? `/api/automations/${existing.automation_id}` : '/api/automations', {
|
||||
|
|
|
|||
Loading…
Reference in a new issue