diff --git a/data/model/user.py b/data/model/user.py index b19623cb8..a4f52837f 100644 --- a/data/model/user.py +++ b/data/model/user.py @@ -436,10 +436,18 @@ def update_user_metadata(user, metadata=None): metadata = metadata if metadata is not None else {} with db_transaction(): - user.given_name = metadata.get('given_name') or user.given_name - user.family_name = metadata.get('family_name') or user.family_name - user.company = metadata.get('company') or user.company - user.location = metadata.get('location') or user.location + if 'given_name' in metadata: + user.given_name = metadata['given_name'] + + if 'family_name' in metadata: + user.family_name = metadata['family_name'] + + if 'company' in metadata: + user.company = metadata['company'] + + if 'location' in metadata: + user.location = metadata['location'] + user.save() # Remove any prompts associated with the user's metadata being needed. diff --git a/endpoints/api/test/test_user.py b/endpoints/api/test/test_user.py new file mode 100644 index 000000000..bf31b0b6d --- /dev/null +++ b/endpoints/api/test/test_user.py @@ -0,0 +1,42 @@ +import pytest + +from mock import patch + +from endpoints.api.test.shared import conduct_api_call +from endpoints.api.user import User +from endpoints.test.shared import client_with_identity +from features import FeatureNameValue + +from test.fixtures import * + + +def test_user_metadata_update(client): + with patch('features.USER_METADATA', FeatureNameValue('USER_METADATA', True)): + with client_with_identity('devtable', client) as cl: + metadata = { + 'given_name': 'Quay', + 'family_name': 'User', + 'location': 'NYC', + 'company': 'Red Hat', + } + + # Update all user metadata fields. + conduct_api_call(cl, User, 'PUT', None, body=metadata) + + # Test that they were successfully updated. + user = conduct_api_call(cl, User, 'GET', None).json + for field in metadata: + assert user.get(field) == metadata.get(field) + + # Now nullify one of the fields, and remove another. + metadata['company'] = None + location = metadata.pop('location') + + conduct_api_call(cl, User, 'PUT', None, body=metadata) + + user = conduct_api_call(cl, User, 'GET', None).json + for field in metadata: + assert user.get(field) == metadata.get(field) + + # The location field should be unchanged. + assert user.get('location') == location diff --git a/endpoints/api/user.py b/endpoints/api/user.py index dde7bb15f..b81021f78 100644 --- a/endpoints/api/user.py +++ b/endpoints/api/user.py @@ -236,19 +236,19 @@ class User(ApiResource): 'description': 'Custom email address for receiving invoices', }, 'given_name': { - 'type': 'string', + 'type': ['string', 'null'], 'description': 'The optional entered given name for the user', }, 'family_name': { - 'type': 'string', + 'type': ['string', 'null'], 'description': 'The optional entered family name for the user', }, 'company': { - 'type': 'string', + 'type': ['string', 'null'], 'description': 'The optional entered company for the user', }, 'location': { - 'type': 'string', + 'type': ['string', 'null'], 'description': 'The optional entered location for the user', }, }, @@ -367,22 +367,16 @@ class User(ApiResource): model.user.update_email(user, new_email, auto_verify=not features.MAILING) if features.USER_METADATA: - metadata_fields = ('given_name', 'family_name', 'company', 'location') - if any(field in user_data for field in metadata_fields): - model.user.update_user_metadata(user, { - 'given_name': user_data.get('given_name'), - 'family_name': user_data.get('family_name'), - 'company': user_data.get('company'), - 'location': user_data.get('location'), - }) + metadata = {} - ua_mdata_future = user_analytics.change_metadata( - user.email, - user_data.get('given_name'), - user_data.get('family_name'), - user_data.get('company'), - user_data.get('location'), - ) + for field in ('given_name', 'family_name', 'company', 'location'): + if field in user_data: + metadata[field] = user_data.get(field) + + if len(metadata) > 0: + model.user.update_user_metadata(user, metadata) + + ua_mdata_future = user_analytics.change_metadata(user.email, **metadata) ua_mdata_future.add_done_callback(build_error_callback('Change metadata failed')) # Check for username rename. A username can be renamed if the feature is enabled OR the user diff --git a/static/js/pages/user-view.js b/static/js/pages/user-view.js index d6521d885..f38211276 100644 --- a/static/js/pages/user-view.js +++ b/static/js/pages/user-view.js @@ -146,7 +146,7 @@ $scope.updateMetadataInfo = function(info, callback) { var details = {}; - details[info.field] = info.value; + details[info.field] = (info.value === '' ? null : info.value); var errorDisplay = ApiService.errorDisplay('Could not update ' + info.title, callback); @@ -248,4 +248,4 @@ }; } -})(); \ No newline at end of file +})(); diff --git a/static/partials/user-view.html b/static/partials/user-view.html index 9d9aaa550..590c29095 100644 --- a/static/partials/user-view.html +++ b/static/partials/user-view.html @@ -211,7 +211,7 @@ dialog-form="context.metadataform">
Please enter an updated {{ changeMetadataInfo.title }}: - +
diff --git a/util/saas/useranalytics.py b/util/saas/useranalytics.py index 2540ea171..049097739 100644 --- a/util/saas/useranalytics.py +++ b/util/saas/useranalytics.py @@ -97,7 +97,7 @@ class _MarketoAnalyticsClient(object): lookupField='id', ) - def change_metadata(self, email, given_name, family_name, company, location): + def change_metadata(self, email, given_name=None, family_name=None, company=None, location=None): lead_data = self._get_lead_metadata(given_name, family_name, company, location) if not lead_data: return