Issue 416: fix sync_status reset bug.

lute/db/schema/baseline.sql

@@ -320,20 +320,6 @@ BEGIN
     );
 END
 ;
-CREATE TRIGGER trig_wordparents_after_delete_change_WoSyncStatus
--- created by db/schema/migrations_repeatable/trig_wordparents.sql
-BEFORE DELETE ON wordparents
-FOR EACH ROW
-BEGIN
-    UPDATE words
-    SET WoSyncStatus = 0
-    WHERE WoID IN
-    (
-      select WpWoID from wordparents
-      where WpParentWoID = old.WpParentWoID
-    );
-END
-;
 CREATE TRIGGER trig_words_after_update_WoStatus_if_following_parent
 -- created by db/schema/migrations_repeatable/trig_words.sql
 AFTER UPDATE OF WoStatus, WoSyncStatus ON words
@@ -388,4 +374,16 @@ BEGIN
     WHERE WoID = NEW.WoID;
 END
 ;
+CREATE TRIGGER trig_word_after_delete_change_WoSyncStatus_for_orphans
+-- created by db/schema/migrations_repeatable/trig_words.sql
+--
+-- If a term is deleted, any orphaned children must
+-- be updated to have WoSyncStatus = 0.
+AFTER DELETE ON words
+BEGIN
+    UPDATE words
+    SET WoSyncStatus = 0
+    WHERE WoID NOT IN (SELECT WpWoID FROM wordparents);
+END
+;
 COMMIT;

lute/db/schema/migrations_repeatable/trig_wordparents.sql

@@ -19,18 +19,39 @@ BEGIN
 END;
 
 
+-- Delete old bad trigger.
 DROP TRIGGER IF EXISTS trig_wordparents_after_delete_change_WoSyncStatus;
 
+/*
+-- This trigger isn't correct, per issue 416: When changing a term's
+-- parents, the existing parent might be deleted first before adding
+-- the new parent.  If this trigger gets fired before the new parent
+-- is assigned, the term will be set to WoSyncStatus = 0, even if the
+-- user wants the term to follow the new parent's status.  Since we
+-- can't say for sure when sqlalchemy will actually make database
+-- changes for child records (i.e., will adding new children happen
+-- before deleting old?), this trigger on its own isn't good enough.
+
+
 CREATE TRIGGER trig_wordparents_after_delete_change_WoSyncStatus
 -- created by db/schema/migrations_repeatable/trig_wordparents.sql
+--
+-- This is a data sanity method only: if all of a term's parents are deleted,
+-- then the term must have WoSyncStatus = 0.
+--
+-- Issue 416: We can't simply set WoSyncStatus = 0 on parent deletion,
+-- because the user may have _changed_ the parents, but still want to
+-- follow the status for the new parent.
 BEFORE DELETE ON wordparents
 FOR EACH ROW
 BEGIN
     UPDATE words
     SET WoSyncStatus = 0
-    WHERE WoID IN
-    (
-      select WpWoID from wordparents
-      where WpParentWoID = old.WpParentWoID
+    WHERE WoID = old.WpWoID
+    AND NOT EXISTS (
+        SELECT 1 FROM wordparents
+        WHERE WpWoID = OLD.WpWoID
+        AND WpParentWoID != OLD.WpParentWoID
     );
 END;
+*/

lute/db/schema/migrations_repeatable/trig_words.sql

@@ -59,3 +59,18 @@ BEGIN
     SET WoCreated = CURRENT_TIMESTAMP
     WHERE WoID = NEW.WoID;
 END;
+
+
+DROP TRIGGER IF EXISTS trig_word_after_delete_change_WoSyncStatus_for_orphans;
+
+CREATE TRIGGER trig_word_after_delete_change_WoSyncStatus_for_orphans
+-- created by db/schema/migrations_repeatable/trig_words.sql
+--
+-- If a term is deleted, any orphaned children must
+-- be updated to have WoSyncStatus = 0.
+AFTER DELETE ON words
+BEGIN
+    UPDATE words
+    SET WoSyncStatus = 0
+    WHERE WoID NOT IN (SELECT WpWoID FROM wordparents);
+END;

lute/models/term.py

@@ -281,6 +281,8 @@ def add_parent(self, parent):
             return
         if parent not in self.parents:
             self.parents.append(parent)
+        if len(self.parents) > 1:
+            self.sync_status = False
 
     def get_current_image(self, strip_jpeg=True):
         "Get the current (first) image for the term."

tests/unit/term/test_Term_status_follow.py

@@ -309,14 +309,109 @@ def test_adding_new_term_does_not_change_family_if_multiple_parents(
     assert_statuses(expected, "updated")
 
 
+def assert_sync_statuses(expected, msg):
+    "Check the sync statuses of terms."
+    lines = [
+        s.strip().replace(":", ";") for s in expected.split("\n") if s.strip() != ""
+    ]
+    sql = "select WoText from words where WoSyncStatus = 1 order by WoText"
+    assert_sql_result(sql, lines, msg)
+
+
 def test_deleting_parent_deactivates_sync_status(term_family, app_context):
     "No more parent = no more follow."
 
-    sql = "select WoText from words where WoSyncStatus = 1"
-    assert_sql_result(sql, ["Byes", "b/1/yes", "c/1/yes"], "before delete")
+    expected = """
+    Byes
+    b/1/yes
+    c/1/yes
+    """
+    assert_sync_statuses(expected, "before delete")
 
     f = term_family
     db.session.delete(f.A)
     db.session.commit()
 
-    assert_sql_result(sql, ["b/1/yes", "c/1/yes"], "only direct children changed")
+    expected = """
+    b/1/yes
+    c/1/yes
+    """
+    assert_sync_statuses(expected, "after delete")
+
+
+def test_adding_extra_parents_unsets_sync_status(term_family, app_context):
+    "Can't follow multiple people."
+    c1 = DBTerm.find(term_family.c1.id)
+    assert c1.sync_status is True, "following C"
+
+    c1.add_parent(term_family.B)
+    assert c1.sync_status is False, "Can't follow 2 parents"
+
+
+def test_changing_parent_keeps_sync_status(term_family, app_context):
+    "Can't follow multiple people."
+    c1 = DBTerm.find(term_family.c1.id)
+    assert c1.sync_status is True, "following C"
+
+    c1.remove_all_parents()
+    c1.add_parent(term_family.B)
+    assert c1.sync_status is True, "Still following"
+
+    db.session.add(c1)
+    db.session.commit()
+
+    expected = """
+    Byes
+    b/1/yes
+    c/1/yes
+    """
+    assert_sync_statuses(expected, "after change")
+
+
+def test_remove_parent_doesnt_affect_other_linked_terms(term_family, app_context):
+    "Issue 417."
+
+    b3 = DBTerm(term_family.B.language, "b3yes")
+    b3.add_parent(term_family.B)
+    b3.sync_status = True
+    db.session.add(b3)
+    db.session.commit()
+
+    expected = """
+    Byes
+    b/1/yes
+    b/3/yes
+    c/1/yes
+    """
+    assert_sync_statuses(expected, "set up")
+
+    b1 = term_family.b1
+    b1.remove_all_parents()
+    b1.sync_status = False
+    db.session.add(b1)
+    db.session.commit()
+
+    expected = """
+    Byes
+    b/3/yes
+    c/1/yes
+    """
+    assert_sync_statuses(expected, "set up")
+
+
+def test_remove_non_linked_parent_leaves_other_linked_terms(term_family, app_context):
+    "Issue 417."
+
+    # term b2 is not linked to its parent b
+    b2 = term_family.b2
+    b2.remove_all_parents()
+    b2.sync_status = False
+    db.session.add(b2)
+    db.session.commit()
+
+    expected = """
+    Byes
+    b/1/yes
+    c/1/yes
+    """
+    assert_sync_statuses(expected, "b1 still linked")