Skip to content

Commit

Permalink
Merge pull request #2960 from akvo/issue/2951-disallow-multiple-resul…
Browse files Browse the repository at this point in the history
…ts-framework-children

[#2951] Disallow multiple result/indicator/period children per project
Code reviewed
  • Loading branch information
zzgvh committed Nov 16, 2017
2 parents ca5ed8c + 547cebb commit f6cd29a
Show file tree
Hide file tree
Showing 8 changed files with 212 additions and 3 deletions.
29 changes: 29 additions & 0 deletions akvo/rest/serializers/rsr_serializer.py
Expand Up @@ -30,3 +30,32 @@ def __init__(self, *args, **kwargs):
ValidXMLTextField: NonNullCharField,
}
)

def get_uniqueness_extra_kwargs(self, field_names, declared_fields, extra_kwargs):
"""Reimplemented to work around https://github.com/encode/django-rest-framework/pull/4192/
*NOTE*: This is a hack to prevent ReadOnlyFields that are a part of
unique_together constraints to be marked as HiddenFields. This bug was
fixed in the PR above (DRF version 3.4.0), but we will need to upgrade
Django to >=1.8. This hack can be removed once that is done.
"""
extra_kwargs, hidden_fields = super(BaseRSRSerializer, self).get_uniqueness_extra_kwargs(
field_names, declared_fields, extra_kwargs
)

model = getattr(self.Meta, 'model')
unique_constraint_names = set()

for parent_class in [model] + list(model._meta.parents.keys()):
for unique_together_list in parent_class._meta.unique_together:
if set(field_names).issuperset(set(unique_together_list)):
unique_constraint_names |= set(unique_together_list)

for field_name in hidden_fields.keys():
if field_name not in unique_constraint_names:
continue
if field_name in self._declared_fields and self._declared_fields[field_name].read_only:
hidden_fields.pop(field_name)

return extra_kwargs, hidden_fields
7 changes: 4 additions & 3 deletions akvo/rsr/management/commands/fix_orphaned_periods.py
Expand Up @@ -70,10 +70,12 @@ def handle(self, *args, **options):
if len(args) == 1 and args[0] == 'indicator':
indicators = find_orphaned_indicators()
periods = []
self.stdout.write('Fixing {} orphaned indicators'.format(len(indicators)))

elif len(args) == 1 and args[0] == 'indicator_period':
indicators = []
periods = find_orphaned_periods()
self.stdout.write('Fixing {} orphaned periods'.format(len(periods)))

elif len(args) == 3 and args[0] == 'indicator':
indicators = [(int(args[1]), int(args[2]))]
Expand All @@ -95,7 +97,7 @@ def handle(self, *args, **options):
child_indicator.parent_indicator = parent_indicator
child_indicator.save()
# Any additional missing data is taken care of by saving the parent.
parent_indicator.save()
# parent_indicator.save()

if verbosity > 1:
self.stdout.write('{} indicator made parent of {}'.format(parent_id, child_id))
Expand All @@ -110,8 +112,7 @@ def handle(self, *args, **options):
child_period.parent_period = parent_period
child_period.save()
# Any additional missing data is taken care of by saving the parent.
parent_period.save()
pprint_period_lineage(child_period)
# parent_period.save()
if parent_period.indicator.periods.count() != child_period.indicator.periods.count():
print 'No. of periods mismatch with parent :: '
pprint_period_lineage(parent_period)
Expand Down
123 changes: 123 additions & 0 deletions akvo/rsr/migrations/0113_auto_20171116_0804.py

Large diffs are not rendered by default.

27 changes: 27 additions & 0 deletions akvo/rsr/migrations/0114_auto_20171110_1158.py
@@ -0,0 +1,27 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations

def delete_duplicate_instances(apps, schema_editor):
duplicate_id = 19656
Indicator = apps.get_model('rsr', 'Indicator')
Indicator.objects.filter(id__in=[duplicate_id]).delete()

dummy_project_id = 5701
Result = apps.get_model('rsr', 'Result')
Result.objects.filter(project__id=dummy_project_id).delete()


class Migration(migrations.Migration):

dependencies = [
('rsr', '0113_auto_20171116_0804'),
]

operations = [
migrations.RunPython(
delete_duplicate_instances,
reverse_code=lambda x, y: None
),
]
26 changes: 26 additions & 0 deletions akvo/rsr/migrations/0115_auto_20171110_1159.py
@@ -0,0 +1,26 @@
# -*- coding: utf-8 -*-
from __future__ import unicode_literals

from django.db import models, migrations


class Migration(migrations.Migration):

dependencies = [
('rsr', '0114_auto_20171110_1158'),
]

operations = [
migrations.AlterUniqueTogether(
name='indicator',
unique_together=set([('result', 'parent_indicator')]),
),
migrations.AlterUniqueTogether(
name='indicatorperiod',
unique_together=set([('indicator', 'parent_period')]),
),
migrations.AlterUniqueTogether(
name='result',
unique_together=set([('project', 'parent_result')]),
),
]
1 change: 1 addition & 0 deletions akvo/rsr/models/result/indicator.py
Expand Up @@ -200,6 +200,7 @@ class Meta:
ordering = ['order', 'id']
verbose_name = _(u'indicator')
verbose_name_plural = _(u'indicators')
unique_together = ('result', 'parent_indicator')


# Add default indicator periods if necessary
Expand Down
1 change: 1 addition & 0 deletions akvo/rsr/models/result/indicator_period.py
Expand Up @@ -444,3 +444,4 @@ class Meta:
verbose_name = _(u'indicator period')
verbose_name_plural = _(u'indicator periods')
ordering = ['period_start']
unique_together = ('indicator', 'parent_period')
1 change: 1 addition & 0 deletions akvo/rsr/models/result/result.py
Expand Up @@ -157,3 +157,4 @@ class Meta:
ordering = ['order', 'id']
verbose_name = _(u'result')
verbose_name_plural = _(u'results')
unique_together = ('project', 'parent_result')

0 comments on commit f6cd29a

Please sign in to comment.