Fix sync - updating source string from VCS not working

pontoon/base/models/entity.py

@@ -453,7 +453,7 @@ def bulk_update(self, objs, fields, batch_size=None):
                 obj.word_count = get_word_count(obj.string)
             if "word_count" not in fields:
                 fields.append("word_count")
-        super().bulk_update(objs, fields=fields, batch_size=batch_size)
+        return super().bulk_update(objs, fields=fields, batch_size=batch_size)
 
 
 class Entity(DirtyFieldsMixin, models.Model):

pontoon/sync/core/entities.py

@@ -3,6 +3,7 @@
 from collections import defaultdict
 from datetime import datetime
 from os.path import exists, isfile, join, relpath, splitext
+from typing import Optional
 
 from moz.l10n.paths import L10nConfigPaths, L10nDiscoverPaths
 
@@ -162,23 +163,22 @@ def update_resources(
         obsolete_entities, ["obsolete", "date_obsoleted"]
     )
 
-    mod_count = Entity.objects.bulk_update(
-        (
-            ent
-            for key, ent in next_entities.items()
-            if key in prev_entities.keys() & next_entities.keys()
-            and not entities_same(ent, prev_entities[key])
-        ),
-        [
-            "string",
-            "string_plural",
-            "comment",
-            "source",
-            "group_comment",
-            "resource_comment",
-            "context",
-        ],
-    )
+    mod_fields = [
+        "string",
+        "string_plural",
+        "comment",
+        "source",
+        "group_comment",
+        "resource_comment",
+        "context",
+    ]
+    mod_entities = [
+        ent
+        for key, next_ent in next_entities.items()
+        if key in prev_entities.keys() & next_entities.keys()
+        and (ent := entity_update(prev_entities[key], next_ent, mod_fields))
+    ]
+    mod_count = Entity.objects.bulk_update(mod_entities, mod_fields)
 
     # FIXME: Entity order should be updated on insertion
     # https://github.com/mozilla/pontoon/issues/2115
@@ -321,16 +321,16 @@ def entity_from_source(
     )
 
 
-def entities_same(a: Entity, b: Entity) -> bool:
-    return (
-        a.string == b.string
-        and a.string_plural == b.string_plural
-        and a.comment == b.comment
-        and a.source == b.source
-        and a.group_comment == b.group_comment
-        and a.resource_comment == b.resource_comment
-        and a.context == b.context
-    )
+def entity_update(
+    current: Entity, update_from: Entity, fields: list[str]
+) -> Optional[Entity]:
+    updated = False
+    for field in fields:
+        value = getattr(update_from, field)
+        if getattr(current, field) != value:
+            setattr(current, field, value)
+            updated = True
+    return current if updated else None
 
 
 def get_db_path(paths: L10nConfigPaths | L10nDiscoverPaths, file_path: str) -> str:

pontoon/sync/tests/test_entities.py

@@ -259,3 +259,77 @@ def test_update_resource():
         update_stats(project)
         project.refresh_from_db()
         assert (project.total_strings, project.approved_strings) == (8, 7)
+
+
+@pytest.mark.django_db
+def test_change_entities():
+    with TemporaryDirectory() as root:
+        # Database setup
+        settings.MEDIA_ROOT = root
+        locale = LocaleFactory.create(code="fr-Test")
+        locale_map = {locale.code: locale}
+        repo = RepositoryFactory(url="http://example.com/repo")
+        project = ProjectFactory.create(
+            name="test-change", locales=[locale], repositories=[repo]
+        )
+        res = ResourceFactory.create(
+            project=project, path="res.ftl", format="ftl", total_strings=3
+        )
+        for i in (1, 2, 3):
+            entity = EntityFactory.create(
+                resource=res,
+                key=f"key-{i}",
+                string=f"key-{i} = Message {i}\n",
+            )
+            TranslationFactory.create(
+                entity=entity,
+                locale=locale,
+                string=f"key-{i} = Translation {i}\n",
+                active=True,
+                approved=True,
+            )
+
+        # Filesystem setup
+        res_ftl = dedent(
+            """
+            key-1 = Message 1
+            key-2 = Fixed message 2
+            # New comment
+            key-3 = Message 3
+            """
+        )
+        makedirs(repo.checkout_path)
+        build_file_tree(
+            repo.checkout_path,
+            {
+                "en-US": {"res.ftl": res_ftl},
+                "fr-Test": {"res.ftl": ""},
+            },
+        )
+
+        # Paths setup
+        mock_checkout = Mock(
+            Checkout,
+            path=repo.checkout_path,
+            changed=[join("en-US", "res.ftl")],
+            removed=[],
+            renamed=[],
+        )
+        paths = find_paths(project, Checkouts(mock_checkout, mock_checkout))
+
+        # Test sync
+        assert sync_entities_from_repo(
+            project, locale_map, mock_checkout, paths, now
+        ) == (0, {"res.ftl"}, set())
+        assert set(
+            (ent.string, ent.comment) for ent in Entity.objects.filter(resource=res)
+        ) == {
+            ("key-1 = Message 1\n", ""),
+            ("key-2 = Fixed message 2\n", ""),
+            ("key-3 = Message 3\n", "New comment"),
+        }
+
+        # Test stats
+        update_stats(project)
+        project.refresh_from_db()
+        assert (project.total_strings, project.approved_strings) == (3, 3)