changes: drop ACTION_NEW_STRING

It was really doing just aggregation which can now be handled in the
notification subsystem. Notification now triggers on ACTION_NEW_UNIT and
is doing digests for users who had instant notification up to now.

Fixes WeblateOrg#10153

docs/changes.rst

@@ -22,6 +22,7 @@ Not yet released.
 * Show plural form examples more prominently.
 * Highlight whitespace in :ref:`machine-translation`.
 * Faster comment and component removal.
+* New string notification can now be triggered for each string.
 
 **Bug fixes**
 

weblate/accounts/migrations/0002_new_string_notification.py

@@ -0,0 +1,28 @@
+# Copyright © Michal Čihař <[email protected]>
+#
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+# Generated by Django 4.2.5 on 2023-10-12 08:25
+
+from django.db import migrations
+
+
+def migrate_subscription(apps, schema_editor):
+    Subscription = apps.get_model("accounts", "Subscription")
+    # Change instant to daily, because this now has string granularity
+    Subscription.objects.filter(
+        notification="NewStringNotificaton", frequency=1
+    ).update(frequency=2)
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("accounts", "0001_squashed_weblate_5"),
+        ("trans", "0006_alter_change_action"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            migrate_subscription, migrations.RunPython.noop, elidable=True
+        ),
+    ]

weblate/accounts/notifications.py

@@ -489,7 +489,7 @@ def get_context(
 
 @register_notification
 class NewStringNotificaton(Notification):
-    actions = (Change.ACTION_NEW_STRING,)
+    actions = (Change.ACTION_NEW_UNIT,)
     verbose = pgettext_lazy(
         "Notification name", "New string is available for translation"
     )

weblate/accounts/tests/test_notifications.py

@@ -195,25 +195,14 @@ def test_notify_parse_error(self):
         self.validate_notifications(3, "[Weblate] Parse error in Test/Test")
 
     def test_notify_new_string(self):
-        Change.objects.create(
-            translation=self.get_translation(), action=Change.ACTION_NEW_STRING
-        )
-
-        # Check mail
-        self.validate_notifications(
-            1, "[Weblate] New string to translate in Test/Test — Czech"
-        )
-
-    def test_notify_new_strings(self):
         Change.objects.create(
             translation=self.get_translation(),
-            action=Change.ACTION_NEW_STRING,
-            details={"count": 10},
+            action=Change.ACTION_NEW_UNIT,
         )
 
         # Check mail
         self.validate_notifications(
-            1, "[Weblate] New strings to translate in Test/Test — Czech"
+            1, "[Weblate] New string to translate in Test/Test — Czech"
         )
 
     def test_notify_new_translation(self):

weblate/templates/mail/new_string.html

@@ -3,7 +3,11 @@
 {% load i18n %}{% load translations %}
 
 {% block content %}
+{% include "mail/snippets/source-string.html" %}
+
 <div class="line buttons">
-    <a class="button" href="{{ change.get_absolute_url }}">{% trans "View" %}</a>
+    <a class="button" href="{{ unit.get_absolute_url }}">{% trans "Edit this string" %}</a>
 </div>
+
+{% include "mail/snippets/unit-screenshots.html" %}
 {% endblock %}

weblate/templates/mail/new_string_subject.txt

@@ -1,4 +1,4 @@
 {% load i18n %}
 {% autoescape off %}
-{% blocktrans count count=change.plural_count %}New string to translate in {{ translation }}{% plural %}New strings to translate in {{ translation }}{% endblocktrans %}
+{% blocktrans %}New string to translate in {{ translation }}{% endblocktrans %}
 {% endautoescape %}

weblate/trans/migrations/0006_alter_change_action.py

@@ -0,0 +1,98 @@
+# Copyright © Michal Čihař <[email protected]>
+#
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+# Generated by Django 4.2.5 on 2023-10-12 08:25
+
+from django.db import migrations, models
+
+ACTION_NEW_STRING = 44
+
+
+def migrate_changes(apps, schema_editor):
+    Change = apps.get_model("trans", "Change")
+    Change.objects.filter(action=ACTION_NEW_STRING).delete()
+
+
+class Migration(migrations.Migration):
+    dependencies = [
+        ("trans", "0005_alter_change_alert_alter_change_announcement_and_more"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="change",
+            name="action",
+            field=models.IntegerField(
+                choices=[
+                    (0, "Resource update"),
+                    (1, "Translation completed"),
+                    (2, "Translation changed"),
+                    (5, "New translation"),
+                    (3, "Comment added"),
+                    (4, "Suggestion added"),
+                    (6, "Automatic translation"),
+                    (7, "Suggestion accepted"),
+                    (8, "Translation reverted"),
+                    (9, "Translation uploaded"),
+                    (13, "New source string"),
+                    (14, "Component locked"),
+                    (15, "Component unlocked"),
+                    (17, "Committed changes"),
+                    (18, "Pushed changes"),
+                    (19, "Reset repository"),
+                    (20, "Merged repository"),
+                    (21, "Rebased repository"),
+                    (22, "Failed merge on repository"),
+                    (23, "Failed rebase on repository"),
+                    (28, "Failed push on repository"),
+                    (24, "Parse error"),
+                    (25, "Removed translation"),
+                    (26, "Suggestion removed"),
+                    (27, "Search and replace"),
+                    (29, "Suggestion removed during cleanup"),
+                    (30, "Source string changed"),
+                    (31, "New string added"),
+                    (32, "Bulk status change"),
+                    (33, "Changed visibility"),
+                    (34, "Added user"),
+                    (35, "Removed user"),
+                    (36, "Translation approved"),
+                    (37, "Marked for edit"),
+                    (38, "Removed component"),
+                    (39, "Removed project"),
+                    (41, "Renamed project"),
+                    (42, "Renamed component"),
+                    (43, "Moved component"),
+                    (45, "New contributor"),
+                    (46, "New announcement"),
+                    (47, "New alert"),
+                    (48, "Added new language"),
+                    (49, "Requested new language"),
+                    (50, "Created project"),
+                    (51, "Created component"),
+                    (52, "Invited user"),
+                    (53, "Received repository notification"),
+                    (54, "Replaced file by upload"),
+                    (55, "License changed"),
+                    (56, "Contributor agreement changed"),
+                    (57, "Screnshot added"),
+                    (58, "Screnshot uploaded"),
+                    (59, "String updated in the repository"),
+                    (60, "Add-on installed"),
+                    (61, "Add-on configuration changed"),
+                    (62, "Add-on uninstalled"),
+                    (63, "Removed string"),
+                    (64, "Removed comment"),
+                    (65, "Resolved comment"),
+                    (66, "Explanation updated"),
+                    (67, "Removed category"),
+                    (68, "Renamed category"),
+                    (69, "Moved category"),
+                    (70, "Could not save string"),
+                ],
+                default=2,
+            ),
+        ),
+        migrations.RunPython(migrate_changes, migrations.RunPython.noop, elidable=True),
+    ]

weblate/trans/models/change.py

@@ -11,14 +11,7 @@
 from django.db.models.base import post_save
 from django.utils import timezone
 from django.utils.html import escape, format_html
-from django.utils.translation import (
-    gettext,
-    gettext_lazy,
-    ngettext,
-    ngettext_lazy,
-    pgettext,
-    pgettext_lazy,
-)
+from django.utils.translation import gettext, gettext_lazy, pgettext, pgettext_lazy
 from rapidfuzz.distance import DamerauLevenshtein
 
 from weblate.lang.models import Language
@@ -266,7 +259,7 @@ class Change(models.Model, UserDisplayMixin):
     ACTION_RENAME_PROJECT = 41
     ACTION_RENAME_COMPONENT = 42
     ACTION_MOVE_COMPONENT = 43
-    ACTION_NEW_STRING = 44
+    # Used to be ACTION_NEW_STRING = 44
     ACTION_NEW_CONTRIBUTOR = 45
     ACTION_ANNOUNCEMENT = 46
     ACTION_ALERT = 47
@@ -373,12 +366,6 @@ class Change(models.Model, UserDisplayMixin):
         (ACTION_RENAME_COMPONENT, gettext_lazy("Renamed component")),
         # Translators: Name of event in the history
         (ACTION_MOVE_COMPONENT, gettext_lazy("Moved component")),
-        # Using pgettext to differentiate from the plural
-        # Translators: Name of event in the history
-        (
-            ACTION_NEW_STRING,
-            pgettext_lazy("Name of event in the history", "New string to translate"),
-        ),
         # Translators: Name of event in the history
         (ACTION_NEW_CONTRIBUTOR, gettext_lazy("New contributor")),
         # Translators: Name of event in the history
@@ -502,11 +489,6 @@ class Change(models.Model, UserDisplayMixin):
         ACTION_FAILED_PUSH,
     }
 
-    PLURAL_ACTIONS = {
-        ACTION_NEW_STRING: ngettext_lazy(
-            "New string to translate", "New strings to translate"
-        ),
-    }
     AUTO_ACTIONS = {
         # Translators: Name of event in the history
         ACTION_LOCK: gettext_lazy(
@@ -606,8 +588,6 @@ def get_absolute_url(self):
         if self.screenshot is not None:
             return self.screenshot.get_absolute_url()
         if self.translation is not None:
-            if self.action == self.ACTION_NEW_STRING:
-                return self.translation.get_translate_url() + "?q=is:untranslated"
             return self.translation.get_absolute_url()
         if self.component is not None:
             return self.component.get_absolute_url()
@@ -673,8 +653,6 @@ def auto_status(self):
         return self.details.get("auto", False)
 
     def get_action_display(self):
-        if self.action in self.PLURAL_ACTIONS:
-            return self.PLURAL_ACTIONS[self.action] % self.plural_count
         return str(self.ACTIONS_DICT.get(self.action, self.action))
 
     def get_state_display(self):
@@ -714,18 +692,6 @@ def get_details_display(self):  # noqa: C901
 
         details = self.details
 
-        if self.action == self.ACTION_NEW_STRING:
-            result = ngettext(
-                "%d new string to translate appeared in the translation.",
-                "%d new strings to translate appeared to the translation.",
-                self.plural_count,
-            )
-            try:
-                return result % self.plural_count
-            except TypeError:
-                # The string does not contain %d
-                return result
-
         if self.action in (self.ACTION_ANNOUNCEMENT, self.ACTION_AGREEMENT_CHANGE):
             return render_markdown(self.target)
 

weblate/trans/models/component.py

@@ -2484,10 +2484,6 @@ def _create_translations(  # noqa: C901
 
             transaction.on_commit(lambda: cleanup_component.delay(self.id))
 
-        # Send notifications on new string
-        for translation in translations.values():
-            translation.notify_new(request)
-
         if was_change:
             if self.needs_variants_update:
                 self.update_variants()

weblate/trans/models/translation.py

@@ -164,7 +164,6 @@ def __init__(self, *args, **kwargs):
         super().__init__(*args, **kwargs)
         self.stats = TranslationStats(self)
         self.addon_commit_files = []
-        self.was_new = 0
         self.reason = ""
         self._invalidate_scheduled = False
         self.update_changes = []
@@ -221,19 +220,6 @@ def clean(self):
                 % {"file": self.filename, "error": str(error)}
             )
 
-    def notify_new(self, request):
-        if self.was_new:
-            # Create change after flags has been updated and cache
-            # invalidated, otherwise we might be sending notification
-            # with outdated values
-            self.change_set.create(
-                action=Change.ACTION_NEW_STRING,
-                user=request.user if request else None,
-                author=request.user if request else None,
-                details={"count": self.was_new},
-            )
-            self.was_new = 0
-
     def get_url_path(self):
         return (*self.component.get_url_path(), self.language.code)
 
@@ -328,18 +314,6 @@ def sync_unit(
         ):
             newunit.update_from_unit(unit, pos, is_new)
 
-        # Check if unit is worth notification:
-        # - new and untranslated
-        # - newly untranslated
-        # - newly fuzzy
-        # - source string changed
-        if newunit.state < STATE_TRANSLATED and (
-            newunit.state != newunit.old_unit["state"]
-            or is_new
-            or newunit.source != newunit.old_unit["source"]
-        ):
-            self.was_new += 1
-
         # Store current unit ID
         updated[id_hash] = newunit
 
@@ -414,9 +388,6 @@ def check_sync(self, force=False, request=None, change=None):  # noqa: C901
                     self.plural = plural
                     self.save(update_fields=["plural"])
 
-                # Was there change?
-                self.was_new = 0
-
                 # Select all current units for update
                 dbunits = {
                     unit.id_hash: unit
@@ -1171,8 +1142,6 @@ def handle_add_upload(self, request, store, fuzzy: str = ""):
             )
             existing.add(idkey)
             accepted += 1
-        self.was_new = accepted
-        self.notify_new(request)
         component.invalidate_cache()
         if component.needs_variants_update:
             component.update_variants()
@@ -1351,7 +1320,6 @@ def handle_store_change(self, request, user, previous_revision: str, change=None
             self.component.invalidate_cache()
         else:
             self.check_sync(request=request, change=change)
-            self.notify_new(request)
             self.invalidate_cache()
         # Trigger post-update signal
         self.component.trigger_post_update(previous_revision, False)
@@ -1521,8 +1489,6 @@ def add_unit(  # noqa: C901
                 )
             component.invalidate_cache()
             component_post_update.send(sender=self.__class__, component=component)
-            self.was_new = 1
-            self.notify_new(request)
         return result
 
     def notify_deletion(self, unit, user):
@@ -1599,7 +1565,6 @@ def sync_terminology(self):
         if not self.is_source or not self.component.manage_units:
             return
         expected_count = self.component.translation_set.count()
-        self.was_new = 0
         for source in self.component.get_all_sources():
             # Is the string a terminology
             if "terminology" not in source.all_flags:
@@ -1615,8 +1580,6 @@ def sync_terminology(self):
                 is_batch_update=True,
                 skip_existing=True,
             )
-            self.was_new += 1
-        self.notify_new(None)
 
     def validate_new_unit_data(
         self,