Skip to content

Commit 5939779

Browse files
committed
Add unique constraint to the exercise translations
This prevents from duplicating translations for exercises. Also, remove the corresponding health checks since this can't happen anymore.
1 parent 1d2d5f3 commit 5939779

File tree

5 files changed

+48
-78
lines changed

5 files changed

+48
-78
lines changed

wger/exercises/management/commands/exercises-health-check.py

Lines changed: 13 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -56,15 +56,6 @@ def add_arguments(self, parser):
5656
'accessed over the UI)',
5757
)
5858

59-
parser.add_argument(
60-
'--delete-duplicate-translations',
61-
action='store_true',
62-
dest='delete_duplicates',
63-
default=False,
64-
help='Delete duplicate translations (e.g. if an exercise has two French entries, '
65-
'the first one will be removed)',
66-
)
67-
6859
parser.add_argument(
6960
'--delete-no-english',
7061
action='store_true',
@@ -83,64 +74,33 @@ def add_arguments(self, parser):
8374

8475
def handle(self, **options):
8576
delete_untranslated = options['delete_untranslated'] or options['delete_all']
86-
delete_duplicates = options['delete_duplicates'] or options['delete_all']
8777
delete_no_english = options['delete_no_english'] or options['delete_all']
8878

8979
self.english = Language.objects.get(short_name=ENGLISH_SHORT_NAME)
9080

91-
for base in Exercise.objects.all():
92-
self.handle_untranslated(base, delete_untranslated)
93-
self.handle_no_english(base, delete_no_english)
94-
self.handle_duplicate_translations(base, delete_duplicates)
81+
for exercise in Exercise.objects.all():
82+
self.handle_untranslated(exercise, delete_untranslated)
83+
self.handle_no_english(exercise, delete_no_english)
9584

96-
def handle_untranslated(self, base: Exercise, delete: bool):
85+
def handle_untranslated(self, exercise: Exercise, delete: bool):
9786
"""
9887
Delete exercises without translations
9988
"""
100-
if not base.pk or base.translations.count():
89+
if not exercise.pk or exercise.translations.count():
10190
return
10291

103-
self.stdout.write(self.style.WARNING(f'Exercise {base.uuid} has no translations!'))
92+
self.stdout.write(self.style.WARNING(f'Exercise {exercise.uuid} has no translations!'))
10493
if delete:
105-
base.delete()
94+
exercise.delete()
10695
self.stdout.write(' -> deleted')
10796

108-
def handle_no_english(self, base: Exercise, delete: bool):
109-
if not base.pk or base.translations.filter(language=self.english).exists():
97+
def handle_no_english(self, exercise: Exercise, delete: bool):
98+
if not exercise.pk or exercise.translations.filter(language=self.english).exists():
11099
return
111100

112-
self.stdout.write(self.style.WARNING(f'Exercise {base.uuid} has no English translation!'))
101+
self.stdout.write(
102+
self.style.WARNING(f'Exercise {exercise.uuid} has no English translation!')
103+
)
113104
if delete:
114-
base.delete()
105+
exercise.delete()
115106
self.stdout.write(' -> deleted')
116-
117-
def handle_duplicate_translations(self, base: Exercise, delete: bool):
118-
if not base.pk:
119-
return
120-
121-
exercise_languages = base.translations.values_list('language', flat=True)
122-
duplicates = [
123-
Language.objects.get(pk=item)
124-
for item, count in collections.Counter(exercise_languages).items()
125-
if count > 1
126-
]
127-
128-
if not duplicates:
129-
return
130-
131-
warning = f'Exercise {base.uuid} has duplicate translations!'
132-
self.stdout.write(self.style.WARNING(warning))
133-
134-
# Output the duplicates
135-
for language in duplicates:
136-
translations = base.translations.filter(language=language)
137-
self.stdout.write(f'language {language.short_name}:')
138-
for translation in translations:
139-
self.stdout.write(f' * {translation.name} {translation.uuid}')
140-
self.stdout.write('')
141-
142-
# And delete them
143-
if delete:
144-
self.stdout.write(f' Deleting all but first {language.short_name} translation')
145-
for translation in translations[1:]:
146-
translation.delete()
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Generated by Django 5.2.7 on 2025-10-03 19:12
2+
3+
from django.db import migrations, models
4+
5+
6+
class Migration(migrations.Migration):
7+
dependencies = [
8+
('exercises', '0032_rename_exercise'),
9+
]
10+
11+
operations = [
12+
migrations.AddConstraint(
13+
model_name='translation',
14+
constraint=models.UniqueConstraint(
15+
fields=('exercise', 'language'),
16+
name='unique_exercise_language_translation',
17+
violation_error_message='A translation for this exercise and language already exists.',
18+
),
19+
),
20+
]

wger/exercises/models/translation.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,13 @@ class Meta:
105105
'name',
106106
]
107107
indexes = (GinIndex(fields=['name']),)
108+
constraints = [
109+
models.UniqueConstraint(
110+
fields=['exercise', 'language'],
111+
name='unique_exercise_language_translation',
112+
violation_error_message='A translation for this exercise and language already exists.',
113+
)
114+
]
108115

109116
def get_absolute_url(self):
110117
"""

wger/exercises/tests/test_exercise_translation.py

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,10 @@
3030
Muscle,
3131
Translation,
3232
)
33-
from wger.utils.constants import CC_BY_SA_4_LICENSE_ID
33+
from wger.utils.constants import (
34+
CC_0_LICENSE_ID,
35+
CC_BY_SA_4_LICENSE_ID,
36+
)
3437

3538

3639
class ExerciseRepresentationTestCase(WgerTestCase):
@@ -161,7 +164,7 @@ class ExerciseTranslationCustomApiTestCase(ExerciseCrudApiTestCase):
161164
'name': 'A new name',
162165
'description': 'The wild boar is a suid native to much of Eurasia and North Africa',
163166
'language': 1,
164-
'exercise': 2,
167+
'exercise': 1,
165168
}
166169

167170
def get_resource_name(self):
@@ -172,6 +175,7 @@ def test_cant_change_exercise_id(self):
172175
Test that it is not possible to change the exercise id of an existing
173176
translation.
174177
"""
178+
Translation.objects.filter(exercise_id=2).delete()
175179
translation = Translation.objects.get(pk=self.pk)
176180
self.assertEqual(translation.exercise_id, 1)
177181

@@ -217,7 +221,7 @@ def test_cant_set_license(self):
217221
Test that it is not possible to set the license for a newly created
218222
exercise translation (the license is always set to the default)
219223
"""
220-
self.data['license'] = 3
224+
self.data['license'] = CC_0_LICENSE_ID
221225

222226
self.authenticate('trainer1')
223227
response = self.client.post(self.url, data=self.data)
@@ -241,7 +245,7 @@ def test_patch_clean_html(self):
241245
def test_post_only_one_language_per_base(self):
242246
"""
243247
Test that it's not possible to add a second translation for the same
244-
base in the same language.
248+
exercise in the same language.
245249
"""
246250
self.authenticate('trainer1')
247251
response = self.client.post(self.url, data=self.data)

wger/exercises/tests/test_management_commands.py

Lines changed: 0 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -115,24 +115,3 @@ def test_fix_no_english_translation(self):
115115
call_command('exercises-health-check', '--delete-no-english', stdout=self.out)
116116
self.assertIn('-> deleted', self.out.getvalue())
117117
self.assertRaises(Exercise.DoesNotExist, Exercise.objects.get, pk=1)
118-
119-
def test_find_duplicate_translations(self):
120-
exercise = Translation.objects.get(pk=1)
121-
exercise.language_id = 3
122-
exercise.save()
123-
124-
call_command('exercises-health-check', stdout=self.out)
125-
self.assertIn(
126-
'Exercise acad3949-36fb-4481-9a72-be2ddae2bc05 has duplicate translations!',
127-
self.out.getvalue(),
128-
)
129-
self.assertNotIn('-> deleted', self.out.getvalue())
130-
131-
def test_fix_duplicate_translations(self):
132-
exercise = Translation.objects.get(pk=1)
133-
exercise.language_id = 3
134-
exercise.save()
135-
136-
call_command('exercises-health-check', '--delete-duplicate-translations', stdout=self.out)
137-
self.assertIn('Deleting all but first fr translation', self.out.getvalue())
138-
self.assertRaises(Translation.DoesNotExist, Translation.objects.get, pk=5)

0 commit comments

Comments
 (0)