From d705bb52a940c19ae60d40ba7dfdfdad4ef006a7 Mon Sep 17 00:00:00 2001 From: Ilya Shakhat Date: Fri, 26 Sep 2014 18:48:53 +0400 Subject: [PATCH] Do not override company from user profile for closed period When user changes private email in Gerrit it affects all marks made by that user. If user has profile with closed periods and Gerrit email is corporate then all marks for that periods will be still counted as belonging to new company. The algorithm is changed to override profile for open period only (thus assuming the situation when user changed job and email but not updated the profile) Closes bug: 1365735 Change-Id: I3ba1bb98eadf4ecd2eff75fc88744ffab7362940 --- stackalytics/processor/record_processor.py | 8 +++--- tests/unit/test_record_processor.py | 33 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 4 deletions(-) diff --git a/stackalytics/processor/record_processor.py b/stackalytics/processor/record_processor.py index e288aa752..61cf3930c 100644 --- a/stackalytics/processor/record_processor.py +++ b/stackalytics/processor/record_processor.py @@ -78,8 +78,8 @@ class RecordProcessor(object): def _find_company(self, companies, date): for r in companies: if date < r['end_date']: - return r['company_name'] - return companies[-1]['company_name'] + return r['company_name'], 'strict' + return companies[-1]['company_name'], 'open' # may be overridden def _get_company_by_email(self, email): if not email: @@ -247,8 +247,8 @@ class RecordProcessor(object): if user.get('user_name'): record['author_name'] = user['user_name'] - company = self._find_company(user['companies'], record['date']) - if company != '*robots': + company, policy = self._find_company(user['companies'], record['date']) + if company != '*robots' and policy == 'open': company = (self._get_company_by_email(record.get('author_email')) or company) record['company_name'] = company diff --git a/tests/unit/test_record_processor.py b/tests/unit/test_record_processor.py index c8406dbe6..4de439b19 100644 --- a/tests/unit/test_record_processor.py +++ b/tests/unit/test_record_processor.py @@ -208,6 +208,39 @@ class TestRecordProcessor(testtools.TestCase): self.assertIn('johndoe@ibm.com', utils.load_user( record_processor_inst.runtime_storage_inst, 'john_doe')['emails']) + def test_process_commit_existing_user_old_job_not_overridden(self): + # User is known to LP, his email is new to us, and maps to other + # company. Have some record with new email, but from the period when + # he worked for other company. Should return other company as mentioned + # in profile instead of overriding + record_processor_inst = self.make_record_processor( + users=[ + {'user_id': 'john_doe', + 'launchpad_id': 'john_doe', + 'user_name': 'John Doe', + 'emails': ['johndoe@nec.co.jp'], + 'companies': [{'company_name': 'IBM', 'end_date': 1200000000}, + {'company_name': 'NEC', 'end_date': 0}]} + ], + companies=[{'company_name': 'IBM', 'domains': ['ibm.com']}, + {'company_name': 'NEC', 'domains': ['nec.com']}], + lp_info={'johndoe@nec.com': + {'name': 'john_doe', 'display_name': 'John Doe'}}) + + processed_commit = list(record_processor_inst.process( + generate_commits(author_email='johndoe@nec.com', + author_name='John Doe', + date=1000000000)))[0] + + expected_commit = { + 'launchpad_id': 'john_doe', + 'author_email': 'johndoe@nec.com', + 'author_name': 'John Doe', + 'company_name': 'IBM', + } + + self.assertRecordsMatch(expected_commit, processed_commit) + def test_process_commit_existing_user_new_email_unknown_company(self): # User is known to LP, but his email is new to us. Should match # the user and return company from user profile