Skip to content

Commit 404309e

Browse files
committed
fix(component): use technical user for background updates
The scheduled updates should get a proper user in the history. Fixes #20108
1 parent fa5681b commit 404309e

5 files changed

Lines changed: 99 additions & 14 deletions

File tree

weblate/trans/models/component.py

Lines changed: 40 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1724,13 +1724,16 @@ def unload_sources(self) -> None:
17241724
self._sources = {}
17251725
self._sources_prefetched = False
17261726

1727-
def _process_new_source(self, source: Unit, *, save: bool = True) -> Change:
1727+
def _process_new_source(
1728+
self, source: Unit, *, save: bool = True, user: User | None = None
1729+
) -> Change:
17281730
# Avoid fetching empty list of checks from the database
17291731
source.all_checks = []
17301732
source.source_updated = True
1733+
user = user or self.acting_user
17311734
change = source.generate_change(
1732-
self.acting_user,
1733-
self.acting_user,
1735+
user,
1736+
user,
17341737
ActionEvents.NEW_SOURCE,
17351738
check_new=False,
17361739
save=save,
@@ -1743,6 +1746,7 @@ def bulk_create_sources(
17431746
attributes_list: list[UnitAttributesDict],
17441747
*,
17451748
create_unit_change_action: ActionEvents = ActionEvents.NEW_UNIT_REPO,
1749+
user: User | None = None,
17461750
) -> None:
17471751
"""Ensure that all sources are stored in the database."""
17481752
# Make sure we load all the sources
@@ -1792,7 +1796,7 @@ def bulk_create_sources(
17921796
self._sources[unit.id_hash] = unit
17931797

17941798
# Postprocess and create change
1795-
change = self._process_new_source(unit, save=False)
1799+
change = self._process_new_source(unit, save=False, user=user)
17961800
if create_unit_change_action == ActionEvents.NEW_UNIT_UPLOAD:
17971801
change.action = ActionEvents.NEW_SOURCE_UPLOAD
17981802
elif create_unit_change_action == ActionEvents.NEW_UNIT_REPO:
@@ -2339,6 +2343,7 @@ def needs_commit_upstream(self) -> bool:
23392343
@perform_on_link
23402344
def do_update(self, request: AuthenticatedHttpRequest | None = None, method=None):
23412345
"""Perform repository update."""
2346+
user = self.get_update_user(request)
23422347
self.translations_progress = 0
23432348
self.translations_count = 0
23442349
# Hold lock all time here to avoid somebody writing between commit
@@ -2368,9 +2373,7 @@ def do_update(self, request: AuthenticatedHttpRequest | None = None, method=None
23682373

23692374
# commit possible pending changes if needed
23702375
if self.needs_commit_upstream():
2371-
self.commit_pending(
2372-
"update", request.user if request else None, skip_push=True
2373-
)
2376+
self.commit_pending("update", user, skip_push=True)
23742377

23752378
# update local branch
23762379
try:
@@ -2379,6 +2382,7 @@ def do_update(self, request: AuthenticatedHttpRequest | None = None, method=None
23792382
method=method,
23802383
skip_push=True,
23812384
parse_after_update=True,
2385+
user=user,
23822386
)
23832387
except RepositoryError:
23842388
result = False
@@ -2387,7 +2391,7 @@ def do_update(self, request: AuthenticatedHttpRequest | None = None, method=None
23872391
# create translation objects for all files
23882392
parse_error = None
23892393
try:
2390-
self.create_translations(request=request)
2394+
self.create_translations(request=request, user=user)
23912395
except FileParseError as error:
23922396
parse_error = error
23932397

@@ -2404,6 +2408,19 @@ def do_update(self, request: AuthenticatedHttpRequest | None = None, method=None
24042408

24052409
return result
24062410

2411+
def get_update_user(self, request: AuthenticatedHttpRequest | None) -> User:
2412+
"""Return user to credit for background update events."""
2413+
user = request.user if request else self.acting_user
2414+
if user is not None:
2415+
return user
2416+
2417+
# ruff: ignore[import-outside-top-level]
2418+
from weblate.auth.models import User
2419+
2420+
return User.objects.get_or_create_bot(
2421+
scope="weblate", name="update", verbose="Background update"
2422+
)
2423+
24072424
@perform_on_link
24082425
def push_if_needed(self, do_update=True) -> None:
24092426
"""
@@ -3436,11 +3453,13 @@ def update_branch(
34363453
method: str | None = None,
34373454
skip_push: bool = False,
34383455
parse_after_update: bool = False,
3456+
user: User | None = None,
34393457
) -> bool:
34403458
"""Update current branch to match remote (if possible)."""
34413459
if method is None:
34423460
method = self.merge_style
3443-
user = request.user if request else self.acting_user
3461+
if user is None:
3462+
user = request.user if request else self.acting_user
34443463
# run pre update hook
34453464
vcs_pre_update.send(sender=self.__class__, component=self)
34463465
for component in self.linked_children:
@@ -3779,6 +3798,7 @@ def create_translations(
37793798
force_scan: bool = False,
37803799
langs: list[str] | None = None,
37813800
request: AuthenticatedHttpRequest | None = None,
3801+
user: User | None = None,
37823802
changed_template: bool = False,
37833803
from_link: bool = False,
37843804
change: int | None = None,
@@ -3793,6 +3813,7 @@ def create_translations(
37933813
force_scan=force_scan,
37943814
langs=langs,
37953815
request=request,
3816+
user=user,
37963817
changed_template=changed_template,
37973818
from_link=from_link,
37983819
change=change,
@@ -3807,6 +3828,7 @@ def create_translations(
38073828
force_scan=force_scan,
38083829
langs=langs,
38093830
request=request,
3831+
user=user,
38103832
changed_template=changed_template,
38113833
from_link=from_link,
38123834
change=change,
@@ -3819,6 +3841,7 @@ def create_translations(
38193841
# ruff: ignore[import-outside-top-level]
38203842
from weblate.trans.tasks import perform_load
38213843

3844+
load_user = user or (request.user if request is not None else None)
38223845
self.queue_background_task(
38233846
perform_load,
38243847
pk=self.pk,
@@ -3828,7 +3851,7 @@ def create_translations(
38283851
changed_template=changed_template,
38293852
from_link=from_link,
38303853
change=change,
3831-
user_id=request.user.id if request is not None else None,
3854+
user_id=load_user.id if load_user is not None else None,
38323855
)
38333856
return False
38343857

@@ -3839,6 +3862,7 @@ def create_translations_immediate(
38393862
force_scan: bool = False,
38403863
langs: list[str] | None = None,
38413864
request: AuthenticatedHttpRequest | None = None,
3865+
user: User | None = None,
38423866
changed_template: bool = False,
38433867
from_link: bool = False,
38443868
change: int | None = None,
@@ -3859,6 +3883,7 @@ def create_translations_immediate(
38593883
force_scan=force_scan,
38603884
langs=langs,
38613885
request=request,
3886+
user=user,
38623887
changed_template=changed_template,
38633888
from_link=from_link,
38643889
change=change,
@@ -3891,6 +3916,7 @@ def _create_translations( # ruff: ignore[complex-structure, too-many-statements
38913916
force_scan: bool = False,
38923917
langs: list[str] | None = None,
38933918
request: AuthenticatedHttpRequest | None = None,
3919+
user: User | None = None,
38943920
changed_template: bool = False,
38953921
from_link: bool = False,
38963922
change: int | None = None,
@@ -3899,6 +3925,8 @@ def _create_translations( # ruff: ignore[complex-structure, too-many-statements
38993925
# ruff: ignore[import-outside-top-level]
39003926
from weblate.trans.tasks import update_enforced_checks
39013927

3928+
user = user or (request.user if request else self.acting_user)
3929+
39023930
self.store_background_task()
39033931

39043932
# Store the revision as add-ons might update it later
@@ -3994,6 +4022,7 @@ def _create_translations( # ruff: ignore[complex-structure, too-many-statements
39944022
path,
39954023
force,
39964024
request=request,
4025+
user=user,
39974026
change=change,
39984027
)
39994028
except InvalidTemplateError as error:
@@ -4031,7 +4060,6 @@ def _create_translations( # ruff: ignore[complex-structure, too-many-statements
40314060
"removing stale translations: %s",
40324061
",".join(trans.language.code for trans in todelete),
40334062
)
4034-
user = self.acting_user or (request.user if request else None)
40354063
Change.objects.bulk_create(
40364064
Change(
40374065
component=self,
@@ -4073,6 +4101,7 @@ def _create_translations( # ruff: ignore[complex-structure, too-many-statements
40734101
force_scan=force_scan,
40744102
langs=langs,
40754103
request=request,
4104+
user=user,
40764105
from_link=True,
40774106
)
40784107
except FileParseError as error:

weblate/trans/models/translation.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ def check_sync(
140140
path,
141141
force=False,
142142
request: AuthenticatedHttpRequest | None = None,
143+
user: User | None = None,
143144
change=None,
144145
):
145146
"""Parse translation meta info and updates translation object."""
@@ -153,7 +154,7 @@ def check_sync(
153154
translation.language_code = code
154155
translation.save(update_fields=["filename", "language_code"])
155156
force |= translation.sync_readonly_check_flag()
156-
translation.check_sync(force, request=request, change=change)
157+
translation.check_sync(force, request=request, change=change, author=user)
157158
return translation
158159

159160

@@ -665,6 +666,7 @@ def update_units_from_store( # ruff: ignore[complex-structure]
665666
if newunit.unit_attributes is not None
666667
],
667668
create_unit_change_action=self.create_unit_change_action,
669+
user=author or user,
668670
)
669671

670672
# Create/update translations
@@ -697,7 +699,7 @@ def check_sync(
697699
with start_span(op="translation.check_sync", name=self.full_slug):
698700
if change is None:
699701
change = ActionEvents.UPDATE
700-
user = None if request is None else request.user
702+
user = request.user if request is not None else author
701703

702704
details = {
703705
"filename": self.filename,

weblate/trans/tasks.py

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,9 +111,11 @@ def perform_load(
111111
user_id: int | None = None,
112112
) -> None:
113113
request: AuthenticatedHttpRequest | None = None
114+
user: User | None = None
114115
if user_id:
116+
user = User.objects.get(pk=user_id)
115117
request = AuthenticatedHttpRequest()
116-
request.user = User.objects.get(pk=user_id)
118+
request.user = user
117119
try:
118120
component = Component.objects.get(pk=pk)
119121
except Component.DoesNotExist:
@@ -127,6 +129,7 @@ def perform_load(
127129
from_link=from_link,
128130
change=change,
129131
request=request,
132+
user=user,
130133
)
131134

132135

weblate/trans/tests/test_component.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1537,6 +1537,7 @@ def test_create_translations_runs_immediately_inside_celery_task(self) -> None:
15371537
force_scan=False,
15381538
langs=None,
15391539
request=request,
1540+
user=None,
15401541
changed_template=False,
15411542
from_link=False,
15421543
change=None,

weblate/trans/tests/test_remote.py

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,14 @@ def test_background_push_uses_bot_user(self) -> None:
129129
self.assertTrue(change.user.is_bot)
130130
self.assertEqual(change.user.full_name, "Background push")
131131

132+
def assert_background_update_user(self, change) -> None:
133+
self.assertIsNotNone(change.user)
134+
self.assertEqual(change.user.username, "weblate:update")
135+
self.assertTrue(change.user.is_bot)
136+
self.assertEqual(change.user.full_name, "Background update")
137+
self.assertIsNotNone(change.author)
138+
self.assertEqual(change.author.username, "weblate:update")
139+
132140
def do_update_with_callbacks(self, translation) -> bool:
133141
translation = type(translation).objects.get(pk=translation.pk)
134142
with self.captureOnCommitCallbacks(execute=True):
@@ -194,6 +202,24 @@ def test_update(self) -> None:
194202
self.do_update_with_callbacks(translation)
195203
translation = self.component2.translation_set.get(language_code="cs")
196204
self.assertEqual(translation.stats.translated, 1)
205+
change = translation.change_set.filter(action=ActionEvents.UPDATE).latest(
206+
"timestamp"
207+
)
208+
self.assertEqual(change.user, self.user)
209+
self.assertEqual(change.author, self.user)
210+
211+
def test_background_update_uses_bot_user(self) -> None:
212+
"""Test background update attribution in case remote has changed."""
213+
self.push_first(False)
214+
215+
self.assertTrue(self.component2.do_update(None))
216+
217+
translation = self.component2.translation_set.get(language_code="cs")
218+
self.assertEqual(translation.stats.translated, 1)
219+
change = translation.change_set.filter(action=ActionEvents.UPDATE).latest(
220+
"timestamp"
221+
)
222+
self.assert_background_update_user(change)
197223

198224
def test_rebase(self) -> None:
199225
"""Testing of rebase."""
@@ -237,6 +263,30 @@ def test_new_unit(self) -> None:
237263

238264
translation = self.component2.translation_set.get(language_code="cs")
239265
self.assertEqual(translation.stats.all, 5)
266+
unit = translation.unit_set.get(source="Languages")
267+
change = unit.source_unit.change_set.filter(
268+
action=ActionEvents.NEW_SOURCE_REPO
269+
).latest("timestamp")
270+
self.assertEqual(change.user, self.user)
271+
self.assertEqual(change.author, self.user)
272+
273+
def test_background_new_unit_update_uses_bot_user(self) -> None:
274+
"""Test background update attribution for units added in a repository."""
275+
self.push_replace(EXTRA_PO, "a")
276+
277+
self.assertTrue(self.component2.do_update(None))
278+
279+
translation = self.component2.translation_set.get(language_code="cs")
280+
self.assertEqual(translation.stats.all, 5)
281+
unit = translation.unit_set.get(source="Languages")
282+
change = unit.change_set.filter(action=ActionEvents.NEW_UNIT_REPO).latest(
283+
"timestamp"
284+
)
285+
self.assert_background_update_user(change)
286+
change = unit.source_unit.change_set.filter(
287+
action=ActionEvents.NEW_SOURCE_REPO
288+
).latest("timestamp")
289+
self.assert_background_update_user(change)
240290

241291
def test_deleted_unit(self) -> None:
242292
"""Test removing several units from remote repo."""

0 commit comments

Comments
 (0)