Remove general purpose dedupe toggle #33963
Conversation
corehq/toggles/__init__.py
Outdated
| @@ -1464,7 +1464,7 @@ def _commtrackify(domain_name, toggle_is_enabled): | |||
|
|
|||
| CASE_DEDUPE = StaticToggle( | |||
There was a problem hiding this comment.
do we want this to be a FrozenPrivilegeToggle so that new users can't be added to it?
There was a problem hiding this comment.
do we want this to be a
FrozenPrivilegeToggleso that new users can't be added to it?
Thanks for pointing this class out to me. The immediate problem is that I don't believe there is an associated privilege for dedupe, which I'm making sure isn't an oversight on my part. Beyond that, I'm not sure this is a great fit. It sounds like FrozenPrivilegeToggle is meant to duplicate privilege functionality for domains that don't have the privilege. In this case, this toggle is meant to be for domains that are grandfathered into still allowing case updates, which the GA feature will not allow
There was a problem hiding this comment.
this toggle is meant to be for domains that are grandfathered into...
I think this is exactly what FrozenPrivilegeToggle was intended for.
There was a problem hiding this comment.
Can you clarify further? From that CEP, my understanding is that GA'd features become privileges on software plans. Existing domains that use the GA'd functionality may not belong to those software plans, and so FrozenPrivilegeToggle is intended to bridge that gap. But, whether it is the privilege or the toggle, both are supporting the exact same workflow.
That last part is why I don't believe FrozenPrivilegeToggle is a good fit here. We are discussing different functionality: the GA'd feature will not allow updates, while the existing toggle still will (in fact, the toggle's sole purpose will be to support updates). I think its also reasonable to suspect that this update functionality will be open to domains being added and removed, because while we're GA-ing 75% of the feature, I imagine Solutions still wants to support clients that require 100% of the feature.
mjriley
requested review from
a team,
calellowitz,
stephherbers,
orangejenny,
mkangia,
AddisonDunn,
AmitPhulera,
dannyroberts,
kaapstorm and
snopoke
as code owners
dimagimon
added
Core Compatibility
dimagimon
removed
dependencies
dimagimon
removed
Risk: Medium
mjriley
removed request for
snopoke,
kaapstorm,
orangejenny,
mkangia,
AddisonDunn,
calellowitz,
AmitPhulera,
stephherbers and
a team
| ] | ||
|
|
||
| operations = [ | ||
| migrations.RunPython(add_dedupe_update_toggle, reverse_code=reverse), |
There was a problem hiding this comment.
Could this operation be done in corehq/apps/accounting/migrations/0089_dedupe_priv.py to avoid two separate DB migrations?
There was a problem hiding this comment.
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)
Product Description
This should show dedupe functionality to users without the dedupe toggle enabled. Users with the existing dedupe toggle will still maintain access to using dedupe updates.
Technical Summary
Associated ticket: https://dimagi-dev.atlassian.net/browse/SAAS-14958
Feature Flag
This removes most of the functionality for the
CASE_DEDUPEfeature flagSafety Assurance
Safety story
Currently I've only done local testing and re-run the tests. I'd like to verify this on staging. As the functionality to remove update functionality is intended to be reversed in phase 2, I'm not holding myself to writing test suites for this functionality
Automated test coverage
Mentioned above, the new functionality here, where rules do not allow updates unless they have the feature flag, is not backed up by tests because this functionality is not meant to stick around long-term.
QA Plan
TBD
Rollback instructions
Labels & Review