Merge pull request #167 from jkingsman/migrate-favorites

Add favorites as contact field (dug)

tests/e2e/helpers/api.ts

@@ -77,6 +77,7 @@ export interface Channel {
   name: string;
   is_hashtag: boolean;
   on_radio: boolean;
+  favorite: boolean;
   flood_scope_override?: string | null;
 }
 
@@ -216,11 +217,8 @@ export function markAllRead(): Promise<{ status: string; timestamp: number }> {
 
 // --- Settings ---
 
-export type Favorite = { type: string; id: string };
-
 export interface AppSettings {
   max_radio_contacts: number;
-  favorites: Favorite[];
   auto_decrypt_dm_on_advert: boolean;
   last_message_times: Record<string, number>;
   advert_interval: number;

tests/e2e/specs/favorites.spec.ts

@@ -1,24 +1,11 @@
 import { test, expect } from '@playwright/test';
-import {
-  createChannel,
-  deleteChannel,
-  getSettings,
-  updateSettings,
-  type Favorite,
-} from '../helpers/api';
+import { createChannel, deleteChannel, getChannels } from '../helpers/api';
 
 test.describe('Favorites persistence', () => {
-  let originalFavorites: Favorite[] = [];
   let channelName = '';
   let channelKey = '';
 
   test.beforeAll(async () => {
-    const settings = await getSettings();
-    originalFavorites = settings.favorites ?? [];
-
-    // Start deterministic: no favorites
-    await updateSettings({ favorites: [] });
-
     channelName = `#e2efav${Date.now().toString().slice(-6)}`;
     const channel = await createChannel(channelName);
     channelKey = channel.key;
@@ -30,11 +17,6 @@ test.describe('Favorites persistence', () => {
     } catch {
       // Best-effort cleanup
     }
-    try {
-      await updateSettings({ favorites: originalFavorites });
-    } catch {
-      // Best-effort cleanup
-    }
   });
 
   test('add and remove favorite channel with persistence across reload', async ({ page }) => {
@@ -51,8 +33,8 @@ test.describe('Favorites persistence', () => {
     await expect(page.getByText('Favorites')).toBeVisible();
     await expect
       .poll(async () => {
-        const settings = await getSettings();
-        return settings.favorites.some((f) => f.type === 'channel' && f.id === channelKey);
+        const channels = await getChannels();
+        return channels.some((c) => c.key === channelKey && c.favorite);
       })
       .toBe(true);
 
@@ -66,8 +48,8 @@ test.describe('Favorites persistence', () => {
     await expect(page.getByTitle('Add to favorites')).toBeVisible();
     await expect
       .poll(async () => {
-        const settings = await getSettings();
-        return settings.favorites.some((f) => f.type === 'channel' && f.id === channelKey);
+        const channels = await getChannels();
+        return channels.some((c) => c.key === channelKey && c.favorite);
       })
       .toBe(false);
     await expect(page.getByText('Favorites')).not.toBeVisible();

tests/test_migrations.py

@@ -1224,8 +1224,8 @@ class TestMigration039:
 
             applied = await run_migrations(conn)
 
-            assert applied == 16
-            assert await get_version(conn) == 54
+            assert applied == 17
+            assert await get_version(conn) == 55
 
             cursor = await conn.execute(
                 """
@@ -1296,8 +1296,8 @@ class TestMigration039:
 
             applied = await run_migrations(conn)
 
-            assert applied == 16
-            assert await get_version(conn) == 54
+            assert applied == 17
+            assert await get_version(conn) == 55
 
             cursor = await conn.execute(
                 """
@@ -1363,8 +1363,8 @@ class TestMigration039:
 
             applied = await run_migrations(conn)
 
-            assert applied == 10
-            assert await get_version(conn) == 54
+            assert applied == 11
+            assert await get_version(conn) == 55
 
             cursor = await conn.execute(
                 """
@@ -1416,8 +1416,8 @@ class TestMigration040:
 
             applied = await run_migrations(conn)
 
-            assert applied == 15
-            assert await get_version(conn) == 54
+            assert applied == 16
+            assert await get_version(conn) == 55
 
             await conn.execute(
                 """
@@ -1478,8 +1478,8 @@ class TestMigration041:
 
             applied = await run_migrations(conn)
 
-            assert applied == 14
-            assert await get_version(conn) == 54
+            assert applied == 15
+            assert await get_version(conn) == 55
 
             await conn.execute(
                 """
@@ -1531,8 +1531,8 @@ class TestMigration042:
 
             applied = await run_migrations(conn)
 
-            assert applied == 13
-            assert await get_version(conn) == 54
+            assert applied == 14
+            assert await get_version(conn) == 55
 
             await conn.execute(
                 """
@@ -1671,8 +1671,8 @@ class TestMigration046:
 
             applied = await run_migrations(conn)
 
-            assert applied == 9
-            assert await get_version(conn) == 54
+            assert applied == 10
+            assert await get_version(conn) == 55
 
             cursor = await conn.execute(
                 """
@@ -1765,8 +1765,8 @@ class TestMigration047:
 
             applied = await run_migrations(conn)
 
-            assert applied == 8
-            assert await get_version(conn) == 54
+            assert applied == 9
+            assert await get_version(conn) == 55
 
             cursor = await conn.execute(
                 """

tests/test_radio_sync.py

@@ -13,7 +13,6 @@ from meshcore import EventType
 from meshcore.events import Event
 
 import app.radio_sync as radio_sync
-from app.models import Favorite
 from app.radio import RadioManager, radio_manager
 from app.radio_sync import (
     _message_poll_loop,
@@ -363,12 +362,8 @@ class TestSyncRecentContactsToRadio:
         """Favorite contacts not on radio are added via add_contact."""
         await _insert_contact(KEY_A, "Alice", last_contacted=2000)
         await _insert_contact(KEY_B, "Bob", last_contacted=1000)
-        await AppSettingsRepository.update(
-            favorites=[
-                Favorite(type="contact", id=KEY_A),
-                Favorite(type="contact", id=KEY_B),
-            ]
-        )
+        await ContactRepository.set_favorite(KEY_A, True)
+        await ContactRepository.set_favorite(KEY_B, True)
 
         mock_mc = MagicMock()
         mock_mc.get_contact_by_key_prefix = MagicMock(return_value=None)
@@ -390,9 +385,8 @@ class TestSyncRecentContactsToRadio:
         await _insert_contact("dd" * 32, "Dave", last_advert=3000)
         await _insert_contact("ee" * 32, "Eve", last_advert=2500)
 
-        await AppSettingsRepository.update(
-            max_radio_contacts=5, favorites=[Favorite(type="contact", id=KEY_A)]
-        )
+        await AppSettingsRepository.update(max_radio_contacts=5)
+        await ContactRepository.set_favorite(KEY_A, True)
 
         mock_mc = MagicMock()
         mock_mc.get_contact_by_key_prefix = MagicMock(return_value=None)
@@ -416,10 +410,9 @@ class TestSyncRecentContactsToRadio:
         for index, key in enumerate(favorite_keys):
             await _insert_contact(key, f"Favorite{index}", last_contacted=2000 - index)
 
-        await AppSettingsRepository.update(
-            max_radio_contacts=4,
-            favorites=[Favorite(type="contact", id=key) for key in favorite_keys],
-        )
+        await AppSettingsRepository.update(max_radio_contacts=4)
+        for key in favorite_keys:
+            await ContactRepository.set_favorite(key, True)
 
         mock_mc = MagicMock()
         mock_mc.get_contact_by_key_prefix = MagicMock(return_value=None)
@@ -498,7 +491,7 @@ class TestSyncAndOffloadAll:
         await _insert_contact(KEY_A, "Alice", last_advert=3000, contact_type=2)
         await _insert_contact(KEY_B, "Bob", last_advert=2000, contact_type=1)
 
-        await AppSettingsRepository.update(max_radio_contacts=1, favorites=[])
+        await AppSettingsRepository.update(max_radio_contacts=1)
 
         mock_mc = MagicMock()
         mock_mc.get_contact_by_key_prefix = MagicMock(return_value=None)
@@ -519,13 +512,8 @@ class TestSyncAndOffloadAll:
         await _insert_contact(KEY_A, "Alice", last_contacted=2000)
         await _insert_contact(KEY_B, "Bob", last_contacted=1000)
 
-        await AppSettingsRepository.update(
-            max_radio_contacts=2,
-            favorites=[
-                Favorite(type="contact", id=KEY_A),
-                Favorite(type="contact", id=KEY_A),
-            ],
-        )
+        await AppSettingsRepository.update(max_radio_contacts=2)
+        await ContactRepository.set_favorite(KEY_A, True)
 
         mock_mc = MagicMock()
         mock_mc.get_contact_by_key_prefix = MagicMock(return_value=None)
@@ -546,7 +534,7 @@ class TestSyncAndOffloadAll:
     async def test_skips_contacts_already_on_radio(self, test_db):
         """Contacts already on radio are counted but not re-added."""
         await _insert_contact(KEY_A, "Alice", on_radio=False)
-        await AppSettingsRepository.update(favorites=[Favorite(type="contact", id=KEY_A)])
+        await ContactRepository.set_favorite(KEY_A, True)
 
         mock_mc = MagicMock()
         mock_mc.get_contact_by_key_prefix = MagicMock(return_value=MagicMock())  # Found
@@ -606,7 +594,7 @@ class TestSyncAndOffloadAll:
     async def test_handles_add_failure(self, test_db):
         """Failed add_contact increments the failed counter."""
         await _insert_contact(KEY_A, "Alice")
-        await AppSettingsRepository.update(favorites=[Favorite(type="contact", id=KEY_A)])
+        await ContactRepository.set_favorite(KEY_A, True)
 
         mock_mc = MagicMock()
         mock_mc.get_contact_by_key_prefix = MagicMock(return_value=None)
@@ -632,7 +620,7 @@ class TestSyncAndOffloadAll:
             direct_path_len=2,
             direct_path_hash_mode=1,
         )
-        await AppSettingsRepository.update(favorites=[Favorite(type="contact", id=KEY_A)])
+        await ContactRepository.set_favorite(KEY_A, True)
 
         mock_mc = MagicMock()
         mock_mc.get_contact_by_key_prefix = MagicMock(return_value=None)
@@ -661,7 +649,7 @@ class TestSyncAndOffloadAll:
             direct_path_len=-125,
             direct_path_hash_mode=2,
         )
-        await AppSettingsRepository.update(favorites=[Favorite(type="contact", id=KEY_A)])
+        await ContactRepository.set_favorite(KEY_A, True)
 
         mock_mc = MagicMock()
         mock_mc.get_contact_by_key_prefix = MagicMock(return_value=None)
@@ -686,7 +674,7 @@ class TestSyncAndOffloadAll:
         so it passes mc directly to avoid deadlock (asyncio.Lock is not reentrant).
         """
         await _insert_contact(KEY_A, "Alice", last_contacted=2000)
-        await AppSettingsRepository.update(favorites=[Favorite(type="contact", id=KEY_A)])
+        await ContactRepository.set_favorite(KEY_A, True)
 
         mock_mc = MagicMock()
         mock_mc.get_contact_by_key_prefix = MagicMock(return_value=None)
@@ -726,7 +714,7 @@ class TestSyncAndOffloadAll:
         """If _meshcore is swapped between pre-check and lock acquisition,
         the function uses the new (post-lock) instance, not the stale one."""
         await _insert_contact(KEY_A, "Alice", last_contacted=2000)
-        await AppSettingsRepository.update(favorites=[Favorite(type="contact", id=KEY_A)])
+        await ContactRepository.set_favorite(KEY_A, True)
 
         old_mc = MagicMock(name="old_mc")
         new_mc = MagicMock(name="new_mc")

tests/test_repository.py

@@ -620,7 +620,6 @@ class TestAppSettingsRepository:
         mock_cursor.fetchone = AsyncMock(
             return_value={
                 "max_radio_contacts": 250,
-                "favorites": "{not-json",
                 "auto_decrypt_dm_on_advert": 1,
                 "last_message_times": "{also-not-json",
                 "advert_interval": None,
@@ -641,36 +640,10 @@ class TestAppSettingsRepository:
             settings = await AppSettingsRepository.get()
 
         assert settings.max_radio_contacts == 250
-        assert settings.favorites == []
         assert settings.last_message_times == {}
         assert settings.advert_interval == 0
         assert settings.last_advert_time == 0
 
-    @pytest.mark.asyncio
-    async def test_add_favorite_is_idempotent(self):
-        """Adding an existing favorite does not write duplicate entries."""
-        from app.models import AppSettings, Favorite
-
-        existing = AppSettings(favorites=[Favorite(type="contact", id="aa" * 32)])
-
-        with (
-            patch(
-                "app.repository.AppSettingsRepository.get",
-                new_callable=AsyncMock,
-                return_value=existing,
-            ),
-            patch(
-                "app.repository.AppSettingsRepository.update",
-                new_callable=AsyncMock,
-            ) as mock_update,
-        ):
-            from app.repository import AppSettingsRepository
-
-            result = await AppSettingsRepository.add_favorite("contact", "aa" * 32)
-
-        assert result == existing
-        mock_update.assert_not_awaited()
-
 
 class TestMessageRepositoryGetById:
     """Test MessageRepository.get_by_id method."""

tests/test_settings_router.py

@@ -133,6 +133,7 @@ class TestUpdateSettings:
 class TestToggleFavorite:
     @pytest.mark.asyncio
     async def test_adds_when_not_favorited(self, test_db):
+        await ContactRepository.upsert(ContactUpsert(public_key="aa" * 32, name="Alice"))
         request = FavoriteRequest(type="contact", id="aa" * 32)
         with (
             patch("app.radio_sync.ensure_contact_on_radio", new_callable=AsyncMock) as mock_sync,
@@ -141,16 +142,16 @@ class TestToggleFavorite:
             mock_create_task.side_effect = lambda coro: coro.close()
             result = await toggle_favorite(request)
 
-        assert len(result.favorites) == 1
-        assert result.favorites[0].type == "contact"
-        assert result.favorites[0].id == "aa" * 32
+        assert result.favorite is True
+        assert result.type == "contact"
+        assert result.id == "aa" * 32
         mock_sync.assert_called_once_with("aa" * 32, force=True)
         mock_create_task.assert_called_once()
 
     @pytest.mark.asyncio
     async def test_removes_when_already_favorited(self, test_db):
-        # Pre-add a favorite
-        await AppSettingsRepository.add_favorite("contact", "aa" * 32)
+        await ContactRepository.upsert(ContactUpsert(public_key="aa" * 32, name="Alice"))
+        await ContactRepository.set_favorite("aa" * 32, True)
 
         request = FavoriteRequest(type="contact", id="aa" * 32)
         with (
@@ -160,7 +161,7 @@ class TestToggleFavorite:
             mock_create_task.side_effect = lambda coro: coro.close()
             result = await toggle_favorite(request)
 
-        assert result.favorites == []
+        assert result.favorite is False
         mock_sync.assert_not_called()
         mock_create_task.assert_not_called()