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.

Sign up for GitHub

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 user data from couch user model #34805

Merged
merged 11 commits into from
Jun 27, 2024
Merged

Remove user data from couch user model #34805

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
07d5a70
Remove old migration
esoergel Jun 19, 2024
160d540
This is no longer a class attribute
esoergel Jun 20, 2024
45137f3
Check user data changes against SQL original
esoergel Jun 20, 2024
8e813e2
Log user data changes and not just from to_json
esoergel Jun 21, 2024
983b087
Remove unused user data field
esoergel Jun 21, 2024
d04391c
Just use ES results directly
esoergel Jun 21, 2024
5081177
Fix related doc lookups for user data
esoergel Jun 21, 2024
ca7845f
Remove db requirement for champ UCR tests
esoergel Jun 24, 2024
ab0ed8a
Remove old, unused user_data field
esoergel Jun 19, 2024
a181308
Make body of migration run only once
esoergel Jun 24, 2024
da40b4d
update help text
esoergel Jun 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 6 additions & 4 deletions corehq/apps/userreports/expressions/specs.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -600,19 +600,21 @@ def __call__(self, item, evaluation_context=None):
@staticmethod
@ucr_context_cache(vary_on=('related_doc_type', 'doc_id',))
def _get_document(related_doc_type, doc_id, evaluation_context):
domain = evaluation_context.root_doc['domain']
assert domain
document_store = get_document_store_for_doc_type(
evaluation_context.root_doc['domain'], related_doc_type,
load_source="related_doc_expression")
domain, related_doc_type, load_source="related_doc_expression")
try:
doc = document_store.get_document(doc_id)
except DocumentNotFoundError:
return None
if evaluation_context.root_doc['domain'] != doc.get('domain'):
if domain != doc.get('domain'):
return None
if related_doc_type == 'CommCareUser':
doc['user_data'] = CommCareUser.wrap(doc).get_user_data(domain).to_dict()
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess user data just hasn't been part of UCRs since the migration?

Copy link
Contributor

Choose a reason for hiding this comment

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

Do these expressions not apply to WebUsers too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess user data just hasn't been part of UCRs since the migration?

That's right - this has been broken since the migration.

You can't do a related doc lookup on a web user, since they don't have a top level domain attribute. For the same reason, they also don't have a user_data attribute (since it varies from one domain to the next), though I guess we could inject one in this context.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For what it's worth, this means of directly accessing the user_data from the raw JSON has been broken to some degree for years - user data is only supposed to be accessed via helper methods, as fields controlled by the profile aren't stored directly in the user document. That means that the profile wasn't accounted for in the API or in UCR. I just broke it further 😆

return doc

def get_value(self, doc_id, evaluation_context):
assert evaluation_context.root_doc['domain']
doc = self._get_document(self.related_doc_type, doc_id, evaluation_context)
# explicitly use a new evaluation context since this is a new document
return self._value_expression(doc, EvaluationContext(doc, 0))
Expand Down
19 changes: 19 additions & 0 deletions corehq/apps/userreports/tests/test_expressions.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1028,6 +1028,25 @@ def test_other_lookups(self):
doc = self._get_doc(user_id)
self.assertEqual(user_id, expression(doc, EvaluationContext(doc, 0)))

def test_user_data_lookup(self):
user = CommCareUser.create(self.domain, 'username', "123", None, None,
user_data={'favorite_color': 'indigo'})
self.addCleanup(user.delete, None, None)
expression = ExpressionFactory.from_spec({
"type": "related_doc",
"related_doc_type": 'CommCareUser',
"doc_id_expression": {
"type": "property_name",
"property_name": "related_id"
},
"value_expression": {
"type": "property_path",
"property_path": ["user_data", "favorite_color"],
},
})
doc = self._get_doc(user._id)
self.assertEqual('indigo', expression(doc, EvaluationContext(doc, 0)))

@staticmethod
def _get_expression(doc_type):
return ExpressionFactory.from_spec({
Expand Down
Loading