Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove general purpose dedupe toggle #33963

Merged
merged 10 commits into from
Feb 15, 2024
1 change: 1 addition & 0 deletions corehq/apps/accounting/bootstrap/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@
privileges.REGEX_FIELD_VALIDATION,
privileges.EXPORT_OWNERSHIP,
privileges.CASE_LIST_EXPLORER,
privileges.CASE_DEDUPE,
]


Expand Down
46 changes: 46 additions & 0 deletions corehq/apps/accounting/migrations/0089_dedupe_priv.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
from django.core.management import call_command
from django.db import migrations

from corehq.privileges import CASE_DEDUPE
from corehq.util.django_migrations import skip_on_fresh_install



@skip_on_fresh_install
def _add_dedupe_to_pro_and_above(apps, schema_editor):
call_command('cchq_prbac_bootstrap')
call_command(
'cchq_prbac_grandfather_privs',
CASE_DEDUPE,
skip_edition='Paused,Community,Standard',
noinput=True,
)


def _reverse(apps, schema_editor):
call_command(
'cchq_prbac_revoke_privs',
CASE_DEDUPE,
skip_edition='Paused,Community,Standard',
delete_privs=False,
check_privs_exist=True,
noinput=True,
)

from corehq.apps.hqadmin.management.commands.cchq_prbac_bootstrap import Command
Command.OLD_PRIVILEGES.append(CASE_DEDUPE)
call_command('cchq_prbac_bootstrap')


class Migration(migrations.Migration):

dependencies = [
('accounting', '0088_add_new_softwareplan_visibility'),
]

operations = [
migrations.RunPython(
_add_dedupe_to_pro_and_above,
reverse_code=_reverse,
),
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
from django.db import migrations
from corehq.util.django_migrations import skip_on_fresh_install

from corehq.toggles import CASE_DEDUPE, CASE_DEDUPE_UPDATES, NAMESPACE_DOMAIN
from corehq.toggles.models import Toggle


@skip_on_fresh_install
def add_dedupe_update_toggle(apps, schema_editor):
for domain in CASE_DEDUPE.get_enabled_domains():
CASE_DEDUPE_UPDATES.set(domain, enabled=True, namespace=NAMESPACE_DOMAIN)


def reverse(apps, schema_editor):
toggle = Toggle.cached_get(CASE_DEDUPE_UPDATES.slug)
toggle.delete()


class Migration(migrations.Migration):

dependencies = [
('data_interfaces', '0036_backfill_dedupe_match_values'),
]

operations = [
migrations.RunPython(add_dedupe_update_toggle, reverse_code=reverse),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this operation be done in corehq/apps/accounting/migrations/0089_dedupe_priv.py to avoid two separate DB migrations?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could be, but I think I'd be looking to unwind this in the future. It feels awkward that the code to get the data_interfaces app in a workable state lives in the accounting app. Ideally, I think both of these migrations ultimately belong in data_interfaces (or even better, a dedicated dedupe app)

]
6 changes: 5 additions & 1 deletion corehq/apps/data_interfaces/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
paginate_query,
paginate_query_across_partitioned_databases, create_unique_index_name,
)
from corehq import toggles
from corehq.util.log import with_progress_bar
from corehq.util.quickcache import quickcache
from corehq.util.test_utils import unit_testing_only
Expand Down Expand Up @@ -1168,7 +1169,10 @@ def _handle_case_duplicate_new(self, case, rule):
existing_duplicate.delete()
duplicate_ids = self._create_duplicates(case, rule, current_hash)

num_updates = self._update_duplicates(duplicate_ids, case, rule)
if toggles.CASE_DEDUPE_UPDATES.enabled(rule.domain):
num_updates = self._update_duplicates(duplicate_ids, case, rule)
else:
num_updates = 0
return CaseRuleActionResult(num_updates=num_updates)

def _create_duplicates(self, case, rule, current_hash):
Expand Down
3 changes: 0 additions & 3 deletions corehq/apps/data_interfaces/pillow.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
from corehq.form_processor.models import CommCareCase
from corehq.form_processor.models.forms import XFormInstance
from corehq.apps.hqcase.constants import UPDATE_REASON_RESAVE
from corehq.toggles import CASE_DEDUPE
from corehq.util.soft_assert import soft_assert


Expand All @@ -28,8 +27,6 @@ class CaseDeduplicationProcessor(PillowProcessor):

def process_change(self, change):
domain = change.metadata.domain
if not CASE_DEDUPE.enabled(domain):
return

if change.deleted:
# TODO: If a case gets deleted, we don't remove the duplicates for this case?
Expand Down
5 changes: 1 addition & 4 deletions corehq/apps/data_interfaces/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
from corehq.apps.case_importer.do_import import SubmitCaseBlockHandler, RowAndCase
from corehq.motech.repeaters.models import SQLRepeatRecord
from corehq.sql_db.util import get_db_aliases_for_partitioned_query
from corehq.toggles import CASE_DEDUPE, DISABLE_CASE_UPDATE_RULE_SCHEDULED_TASK
from corehq.toggles import DISABLE_CASE_UPDATE_RULE_SCHEDULED_TASK
from corehq.util.celery_utils import no_result_task
from corehq.util.decorators import serial_task
from corehq.util.log import send_HTML_email
Expand Down Expand Up @@ -66,9 +66,6 @@ def _progress_tracker(current, total):
@no_result_task(queue='case_rule_queue', acks_late=True,
soft_time_limit=15 * settings.CELERY_TASK_SOFT_TIME_LIMIT)
def reset_and_backfill_deduplicate_rule_task(domain, rule_id):
if not CASE_DEDUPE.enabled(domain):
return

try:
rule = AutomaticUpdateRule.objects.get(
id=rule_id,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@
{% crispy criteria_form %}
</div>
</div>
{% if update_actions_enabled %}
<fieldset {% if readonly %} disabled="disabled" {% endif %}>
<legend>{% trans "Actions" %}</legend>
<div data-bind="template: {name: 'update-case-property-action', foreach: propertiesToUpdate, as: 'propertyToUpdate'} "></div>
Expand All @@ -101,6 +102,7 @@
<input type="text" hidden name="properties_to_update" data-bind="value: serializedPropertiesToUpdate" />
</div>
</fieldset>
{% endif %}
{% if not readonly %}
<div class="form-actions">
<div class="col-xs-1">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
<div class="col-sm-8">
<p>
{% blocktrans %}
Create rules for finding and updating duplicate cases.
Create rules for finding duplicate cases.
Rules are run on each form submission.
When rules are created or edited the rule will be locked for further changes and a backfill process will be run.
{% endblocktrans %}
Expand Down
7 changes: 4 additions & 3 deletions corehq/apps/data_interfaces/tests/test_case_deduplication.py
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@ def test_find_duplicates_blank_case_properties(self):
find_duplicate_case_ids(self.domain, cases[0], ["name", "dob"], match_type="ANY"))


@flag_enabled('CASE_DEDUPE_UPDATES')
class CaseDeduplicationActionTest(TestCase):
def setUp(self):
super().setUp()
Expand Down Expand Up @@ -692,7 +693,6 @@ def test_rule_deletion(self):
self.rule.soft_delete()
self.assertEqual(CaseDuplicateNew.objects.filter(action=self.action).count(), 0)

@flag_enabled('CASE_DEDUPE')
def test_case_deletion(self):
"""Test that deleting cases also deletes Duplicate Relationships
"""
Expand Down Expand Up @@ -756,7 +756,7 @@ def test_update_parent(self):

@override_settings(RUN_UNKNOWN_USER_PILLOW=False)
@override_settings(RUN_FORM_META_PILLOW=False)
@flag_enabled('CASE_DEDUPE')
@flag_enabled('CASE_DEDUPE_UPDATES')
class DeduplicationPillowTest(TestCase):

@classmethod
Expand Down Expand Up @@ -882,6 +882,7 @@ def _configure_properties_to_update(self, rule, prop_map):
action.save()


@flag_enabled('CASE_DEDUPE_UPDATES')
@es_test(requires=[case_search_adapter, user_adapter])
class TestDeduplicationRuleRuns(TestCase):
def setUp(self):
Expand Down Expand Up @@ -1174,7 +1175,7 @@ def test_rule_with_user_as_owner(self):
self.assertEqual(refreshed_fake_cases[1].get_case_property('age'), '14')


@flag_enabled('CASE_DEDUPE')
@flag_enabled('CASE_DEDUPE_UPDATES')
@es_test(requires=[case_search_adapter], setup_class=True)
class DeduplicationBackfillTest(TestCase):
@classmethod
Expand Down
19 changes: 13 additions & 6 deletions corehq/apps/data_interfaces/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -991,7 +991,7 @@ def read_only_mode(self):
return True


@method_decorator(toggles.CASE_DEDUPE.required_decorator(), name='dispatch')
@method_decorator(requires_privilege_with_fallback(privileges.CASE_DEDUPE), name='dispatch')
class DeduplicationRuleListView(DataInterfaceSection, CRUDPaginatedViewMixin):
template_name = 'data_interfaces/list_deduplication_rules.html'
urlname = 'deduplication_rules'
Expand Down Expand Up @@ -1148,7 +1148,7 @@ def _get_duplicates_count(self, rule):
return CaseDuplicateNew.objects.filter(action=action).count()


@method_decorator(toggles.CASE_DEDUPE.required_decorator(), name='dispatch')
@method_decorator(requires_privilege_with_fallback(privileges.CASE_DEDUPE), name='dispatch')
class DeduplicationRuleCreateView(DataInterfaceSection):
template_name = "data_interfaces/edit_deduplication_rule.html"
urlname = 'add_deduplication_rule'
Expand All @@ -1161,6 +1161,7 @@ def page_context(self):
'all_case_properties': self.get_augmented_data_dict_props_by_case_type(self.domain),
'case_types': sorted(list(get_case_types_for_domain(self.domain))),
'criteria_form': self.case_filter_form,
'update_actions_enabled': toggles.CASE_DEDUPE.enabled(self.domain),
})
return context

Expand Down Expand Up @@ -1250,11 +1251,17 @@ def parse_params(self, request):
if prop
],
"include_closed": request.POST.get("include_closed") == "on",
"properties_to_update": [
}
if toggles.CASE_DEDUPE_UPDATES.enabled(self.domain):
properties_to_update = [
{"name": prop["name"], "value_type": prop["valueType"], "value": prop["value"]}
for prop in json.loads(request.POST.get("properties_to_update"))
],
}
]
else:
properties_to_update = []

action_params["properties_to_update"] = properties_to_update

return rule_params, action_params

def validate_rule_params(self, domain, rule_params, rule=None):
Expand Down Expand Up @@ -1307,7 +1314,7 @@ def _create_rule(self, domain, name, case_type):
)


@method_decorator(toggles.CASE_DEDUPE.required_decorator(), name='dispatch')
@method_decorator(requires_privilege_with_fallback(privileges.CASE_DEDUPE), name='dispatch')
class DeduplicationRuleEditView(DeduplicationRuleCreateView):
urlname = 'edit_deduplication_rule'
page_title = gettext_lazy("Edit Deduplication Rule")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,9 @@ def ensure_roles(self, roles, dry_run=False):
Role(slug=privileges.CASE_COPY,
name='Allow Case Copy',
description='Allow case copy from one user to another'),
Role(slug=privileges.CASE_DEDUPE,
name='Deduplication Rules',
description='Support for finding duplicate cases'),
]

BOOTSTRAP_PLANS = [
Expand Down
3 changes: 1 addition & 2 deletions corehq/apps/users/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,7 @@ def tag_cases_as_deleted_and_remove_indices(domain, case_ids, deletion_id, delet
_remove_indices_from_deleted_cases_task.delay(domain, case_ids)
delete_phone_numbers_for_owners.delay(case_ids)
delete_schedule_instances_for_cases.delay(domain, case_ids)
if toggles.CASE_DEDUPE.enabled(domain):
delete_duplicates_for_cases.delay(case_ids)
delete_duplicates_for_cases.delay(case_ids)


@task(serializer='pickle', rate_limit=2, queue='background_queue', ignore_result=True, acks_late=True)
Expand Down
4 changes: 4 additions & 0 deletions corehq/privileges.py
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,8 @@

CASE_COPY = 'case_copy'

CASE_DEDUPE = 'case_deduplicate'

MAX_PRIVILEGES = [
LOOKUP_TABLES,
API_ACCESS,
Expand Down Expand Up @@ -181,6 +183,7 @@
DATA_DICTIONARY,
CASE_LIST_EXPLORER,
CASE_COPY,
CASE_DEDUPE,
]

# These are special privileges related to their own rates in a SoftwarePlanVersion
Expand Down Expand Up @@ -258,4 +261,5 @@ def get_name_from_privilege(cls, privilege):
DATA_DICTIONARY: _("Project level data dictionary of cases"),
CASE_LIST_EXPLORER: _("Case List Explorer"),
CASE_COPY: _("Allow case copy from one user to another"),
CASE_DEDUPE: _("Deduplication Rules"),
}.get(privilege, privilege)
2 changes: 1 addition & 1 deletion corehq/reports.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ def REPORTS(project):
if toggles.CASE_LIST_EXPLORER.enabled(project.name) or domain_can_access_case_list_explorer:
inspect_reports.append(CaseListExplorer)

if toggles.CASE_DEDUPE.enabled(project.name):
if domain_has_privilege(project.name, privileges.CASE_DEDUPE):
inspect_reports.append(DuplicateCasesExplorer)

deployments_reports = (
Expand Down
13 changes: 7 additions & 6 deletions corehq/tabs/tabclasses.py
Original file line number Diff line number Diff line change
Expand Up @@ -482,6 +482,11 @@ def can_edit_commcare_data(self):
def can_use_data_cleanup(self):
return domain_has_privilege(self.domain, privileges.DATA_CLEANUP)

@property
@memoized
def can_use_dedupe(self):
return domain_has_privilege(self.domain, privileges.CASE_DEDUPE)

@property
@memoized
def can_export_data(self):
Expand Down Expand Up @@ -574,11 +579,6 @@ def can_view_ecd_preview(self):
return (EXPLORE_CASE_DATA_PREVIEW.enabled_for_request(self._request)
and is_eligible_for_ecd_preview(self._request))

@property
@memoized
def can_deduplicate_cases(self):
return toggles.CASE_DEDUPE.enabled_for_request(self._request)

@property
def _can_view_geospatial(self):
return toggles.GEOSPATIAL.enabled(self.domain)
Expand Down Expand Up @@ -943,7 +943,7 @@ def _get_edit_section(self):
else:
edit_section = [(gettext_lazy('Edit Data'), [automatic_update_rule_list_view])]

if self.can_deduplicate_cases:
if self.can_use_dedupe:
from corehq.apps.data_interfaces.views import (
DeduplicationRuleListView,
)
Expand All @@ -952,6 +952,7 @@ def _get_edit_section(self):
'url': reverse(DeduplicationRuleListView.urlname, args=[self.domain]),
}
edit_section[0][1].append(deduplication_list_view)

return edit_section

def _get_explore_data_views(self):
Expand Down
14 changes: 11 additions & 3 deletions corehq/toggles/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -1469,9 +1469,9 @@ def _commtrackify(domain_name, toggle_is_enabled):
[NAMESPACE_DOMAIN],
)

CASE_DEDUPE = StaticToggle(
'case_dedupe',
'Case deduplication feature',
CASE_DEDUPE_UPDATES = StaticToggle(
'case_dedupe_updates',
'Allow Case deduplication update actions',
TAG_SOLUTIONS_LIMITED,
[NAMESPACE_DOMAIN],
help_link='https://confluence.dimagi.com/display/saas/Surfacing+Case+Duplicates+in+CommCare',
Expand Down Expand Up @@ -2692,6 +2692,14 @@ def domain_has_privilege_from_toggle(privilege_slug, domain):
help_link='https://confluence.dimagi.com/display/commcarepublic/Data+Dictionary'
)

CASE_DEDUPE = FrozenPrivilegeToggle(
privileges.CASE_DEDUPE,
'case_dedupe',
label='Case deduplication feature',
tag=TAG_SOLUTIONS_LIMITED,
namespaces=[NAMESPACE_DOMAIN],
help_link='https://confluence.dimagi.com/display/saas/Surfacing+Case+Duplicates+in+CommCare',
)

CUSTOM_DOMAIN_BANNER_ALERTS = StaticToggle(
slug='custom_domain_banners',
Expand Down
2 changes: 2 additions & 0 deletions migrations.lock
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,7 @@ accounting
0086_add_duplicate_invoice_id_to_invoice_model
0087_invoice_unique_constraints
0088_add_new_softwareplan_visibility
0089_dedupe_priv
admin
0001_initial
0002_logentry_remove_auto_add
Expand Down Expand Up @@ -339,6 +340,7 @@ data_interfaces
0034_case_name_actions
0035_add_case_duplicate_new
0036_backfill_dedupe_match_values
0037_add_dedupe_update_toggle
dhis2
0001_initial
0002_auto_20170322_1323
Expand Down