refactor to allow group deletions without schema mod

because group deletions are likely to be a semi-common operation (esp. for new users trying out shared material), deleting groups will no longer cause a full sync. in order to avoid syncing issues, we now allow cards/facts/etc to point to an invalid group, and in that case, we just treat them like they're in the default group

anki/consts.py

@@ -23,6 +23,7 @@ REV_CARDS_RANDOM = 2
 # removal types
 REM_CARD = 0
 REM_FACT = 1
+REM_GROUP = 2
 
 # count display
 COUNT_ANSWERED = 0

anki/groups.py

@@ -3,7 +3,7 @@
 # License: GNU AGPL, version 3 or later; http://www.gnu.org/licenses/agpl.html
 
 import simplejson
-from anki.utils import intTime
+from anki.utils import intTime, ids2str
 from anki.consts import *
 
 # fixmes:
@@ -12,6 +12,13 @@ from anki.consts import *
 # - when renaming a group, top level properties should be added or removed as
 #   appropriate
 
+# notes:
+# - it's difficult to enforce valid gids for models/facts/cards, as we
+#   may update the gid locally only to have it overwritten by a more recent
+#   change from somewhere else. to avoid this, we allow invalid gid
+#   references, and treat any invalid gids as the default group.
+# - deletions of group config force a full sync
+
 # configuration only available to top level groups
 defaultTopConf = {
     'newPerDay': 20,
@@ -114,16 +121,17 @@ class GroupManager(object):
             self.save(g)
             return int(id)
 
-    def rem(self, gid):
-        self.deck.modSchema()
-        self.deck.db.execute(
-            "update cards set gid=1,usn=?,mod=? where gid = ?",
-            gid, self.deck.usn(), intTime())
-        self.deck.db.execute(
-            "update facts set gid=1,usn=?,mod=? where gid = ?",
-            gid, self.deck.usn(), intTime())
-        self.deck.db.execute("delete from groups where id = ?", gid)
-        print "fixme: loop through models and update stale gid references"
+    def rem(self, gid, cardsToo=False):
+        "Remove the group. If cardsToo, delete any cards inside."
+        assert gid != 1
+        if not str(gid) in self.groups:
+            return
+        # delete cards too?
+        if cardsToo:
+            self.deck.remCards(self.cids(gid))
+        # delete the group and add a grave
+        del self.groups[str(gid)]
+        self.deck._logRem([gid], REM_GROUP)
 
     def allNames(self):
         "An unsorted list of all group names."
@@ -151,18 +159,20 @@ class GroupManager(object):
     #############################################################
 
     def name(self, gid):
-        return self.groups[str(gid)]['name']
+        return self.get(gid)['name']
 
     def conf(self, gid):
         return self.gconf[str(self.groups[str(gid)]['conf'])]
 
-    def get(self, gid):
+    def get(self, gid, default=True):
         id = str(gid)
         if id in self.groups:
             return self.groups[id]
+        elif default:
+            return self.groups['1']
 
     def setGroup(self, cids, gid):
-        self.db.execute(
+        self.deck.db.execute(
             "update cards set gid=?,usn=?,mod=? where id in "+
             ids2str(cids), gid, self.deck.usn(), intTime())
 
@@ -176,6 +186,15 @@ class GroupManager(object):
         self.gconf[str(g['id'])] = g
         self.save()
 
+    def sendHome(self, cids):
+        self.deck.db.execute("""
+update cards set gid=(select gid from facts f where f.id=fid),
+usn=?,mod=? where id in %s""" % ids2str(cids),
+                             self.deck.usn(), intTime(), gid)
+
+    def cids(self, gid):
+        return self.deck.db.list("select id from cards where gid=?", gid)
+
     # Group selection
     #############################################################
 

anki/models.py

@@ -11,7 +11,8 @@ from anki.consts import *
 # Models
 ##########################################################################
 
-# careful not to add any lists/dicts/etc here, as they aren't deep copied
+# - careful not to add any lists/dicts/etc here, as they aren't deep copied
+
 defaultModel = {
     'css': "",
     'sortf': 0,

anki/sync.py

@@ -159,7 +159,7 @@ class Syncer(object):
     def mergeGroups(self, rchg):
         # like models we rely on schema mod for deletes
         for r in rchg[0]:
-            l = self.deck.groups.get(r['id'])
+            l = self.deck.groups.get(r['id'], False)
             # if missing locally or server is newer, update
             if not l or r['mod'] > l['mod']:
                 self.deck.groups.update(r)

tests/test_deck.py

@@ -131,38 +131,6 @@ def test_upgrade():
     # now's a good time to test the integrity check too
     deck.fixIntegrity()
 
-def test_groups():
-    deck = getEmptyDeck()
-    # we start with a standard group
-    assert len(deck.groups.groups) == 1
-    # it should have an id of 1
-    assert deck.groups.name(1)
-    # create a new group
-    parentId = deck.groups.id("new group")
-    assert parentId
-    assert len(deck.groups.groups) == 2
-    # should get the same id
-    assert deck.groups.id("new group") == parentId
-    # by default, everything should be shown
-    assert not deck.groups.selected()
-    assert not deck.groups.active()
-    # and the default group is used
-    assert deck.groups.top()['id'] == 1
-    # we can select the default explicitly
-    deck.groups.select(1)
-    assert deck.groups.selected() == 1
-    assert deck.groups.active() == [1]
-    assert deck.groups.top()['id'] == 1
-    # let's create a child and select that
-    childId = deck.groups.id("new group::child")
-    deck.groups.select(childId)
-    assert deck.groups.selected() == childId
-    assert deck.groups.active() == [childId]
-    assert deck.groups.top()['id'] == parentId
-    # if we select the parent, the child gets included
-    deck.groups.select(parentId)
-    assert sorted(deck.groups.active()) == [parentId,  childId]
-
 def test_selective():
     deck = getEmptyDeck()
     f = deck.newFact()

tests/test_groups.py

@@ -0,0 +1,64 @@
+# coding: utf-8
+
+from tests.shared import assertException, getEmptyDeck, testDir
+
+def test_basic():
+    deck = getEmptyDeck()
+    # we start with a standard group
+    assert len(deck.groups.groups) == 1
+    # it should have an id of 1
+    assert deck.groups.name(1)
+    # create a new group
+    parentId = deck.groups.id("new group")
+    assert parentId
+    assert len(deck.groups.groups) == 2
+    # should get the same id
+    assert deck.groups.id("new group") == parentId
+    # by default, everything should be shown
+    assert not deck.groups.selected()
+    assert not deck.groups.active()
+    # and the default group is used
+    assert deck.groups.top()['id'] == 1
+    # we can select the default explicitly
+    deck.groups.select(1)
+    assert deck.groups.selected() == 1
+    assert deck.groups.active() == [1]
+    assert deck.groups.top()['id'] == 1
+    # let's create a child and select that
+    childId = deck.groups.id("new group::child")
+    deck.groups.select(childId)
+    assert deck.groups.selected() == childId
+    assert deck.groups.active() == [childId]
+    assert deck.groups.top()['id'] == parentId
+    # if we select the parent, the child gets included
+    deck.groups.select(parentId)
+    assert sorted(deck.groups.active()) == [parentId,  childId]
+
+def test_remove():
+    deck = getEmptyDeck()
+    # can't remove the default group
+    assertException(AssertionError, lambda: deck.groups.rem(1))
+    # create a new group, and add a fact/card to it
+    g1 = deck.groups.id("g1")
+    f = deck.newFact()
+    f['Front'] = u"1"
+    f.gid = g1
+    deck.addFact(f)
+    c = f.cards()[0]
+    assert c.gid == g1
+    # by default deleting the group leaves the cards with an invalid gid
+    assert deck.cardCount() == 1
+    deck.groups.rem(g1)
+    assert deck.cardCount() == 1
+    c.load()
+    assert c.gid == g1
+    # but if we try to get it, we get the default
+    assert deck.groups.name(c.gid) == "Default"
+    # let's create another group and explicitly set the card to it
+    g2 = deck.groups.id("g2")
+    c.gid = g2; c.flush()
+    # this time we'll delete the card/fact too
+    deck.groups.rem(g2, cardsToo=True)
+    assert deck.cardCount() == 0
+    assert deck.factCount() == 0
+