Skip to content

Commit

Permalink
changes: drop ACTION_NEW_STRING
Browse files Browse the repository at this point in the history
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
  • Loading branch information
nijel committed Oct 12, 2023
1 parent 18f9c91 commit 58e8562
Show file tree
Hide file tree
Showing 12 changed files with 145 additions and 102 deletions.
1 change: 1 addition & 0 deletions docs/changes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Not yet released.
* Highlight whitespace in :ref:`machine-translation`.
* Faster comment and component removal.
* Show disabled save button reason more prominently.
* New string notification can now be triggered for each string.

**Bug fixes**

Expand Down
28 changes: 28 additions & 0 deletions weblate/accounts/migrations/0002_new_string_notification.py
Original file line number Diff line number Diff line change
@@ -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
),
]
5 changes: 3 additions & 2 deletions weblate/accounts/notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ def get_context(
result["changes"] = changes
if subscription is not None:
result["unsubscribe_nonce"] = TimestampSigner().sign(subscription.pk)
result["user"] = subscription.user
result["subscription_user"] = subscription.user
if extracontext:
result.update(extracontext)
if change:
Expand Down Expand Up @@ -489,12 +489,13 @@ 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"
)
template_name = "new_string"
filter_languages = True
required_attr = "unit"


@register_notification
Expand Down
16 changes: 1 addition & 15 deletions weblate/accounts/tests/test_notifications.py
Original file line number Diff line number Diff line change
Expand Up @@ -195,27 +195,13 @@ 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
)
Change.objects.create(unit=self.get_unit(), action=Change.ACTION_NEW_UNIT)

# 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},
)

# Check mail
self.validate_notifications(
1, "[Weblate] New strings to translate in Test/Test — Czech"
)

def test_notify_new_translation(self):
Change.objects.create(
unit=self.get_unit(),
Expand Down
10 changes: 5 additions & 5 deletions weblate/api/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -968,7 +968,7 @@ def test_components(self):

def test_changes(self):
request = self.do_request("api:project-changes", self.project_kwargs)
self.assertEqual(request.data["count"], 33)
self.assertEqual(request.data["count"], 30)

def test_statistics(self):
request = self.do_request("api:project-statistics", self.project_kwargs)
Expand Down Expand Up @@ -1904,7 +1904,7 @@ def test_translations(self):

def test_changes(self):
request = self.do_request("api:component-changes", self.component_kwargs)
self.assertEqual(request.data["count"], 25)
self.assertEqual(request.data["count"], 22)

def test_screenshots(self):
request = self.do_request("api:component-screenshots", self.component_kwargs)
Expand Down Expand Up @@ -2648,7 +2648,7 @@ def test_statistics(self):

def test_changes(self):
request = self.do_request("api:translation-changes", self.translation_kwargs)
self.assertEqual(request.data["count"], 6)
self.assertEqual(request.data["count"], 5)

def test_units(self):
request = self.do_request("api:translation-units", self.translation_kwargs)
Expand Down Expand Up @@ -3576,15 +3576,15 @@ def test_units_delete(self):
class ChangeAPITest(APIBaseTest):
def test_list_changes(self):
response = self.client.get(reverse("api:change-list"))
self.assertEqual(response.data["count"], 33)
self.assertEqual(response.data["count"], 30)

def test_filter_changes_after(self):
"""Filter changes since timestamp."""
start = Change.objects.order().last().timestamp
response = self.client.get(
reverse("api:change-list"), {"timestamp_after": start.isoformat()}
)
self.assertEqual(response.data["count"], 33)
self.assertEqual(response.data["count"], 30)

def test_filter_changes_before(self):
"""Filter changes prior to timestamp."""
Expand Down
6 changes: 5 additions & 1 deletion weblate/templates/mail/new_string.html
Original file line number Diff line number Diff line change
Expand Up @@ -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 %}
2 changes: 1 addition & 1 deletion weblate/templates/mail/new_string_subject.txt
Original file line number Diff line number Diff line change
@@ -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 %}
2 changes: 1 addition & 1 deletion weblate/templates/mail/snippets/unit-screenshots.html
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
<h2>{% trans "Source string location" %}</h2>

<p>
{% get_location_links user.profile unit %}
{% get_location_links subscription_user.profile unit %}
</p>
{% endif %}

Expand Down
98 changes: 98 additions & 0 deletions weblate/trans/migrations/0006_alter_change_action.py
Original file line number Diff line number Diff line change
@@ -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),
]
38 changes: 2 additions & 36 deletions weblate/trans/models/change.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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):
Expand Down Expand Up @@ -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)

Expand Down
4 changes: 0 additions & 4 deletions weblate/trans/models/component.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading

0 comments on commit 58e8562

Please sign in to comment.