Merge branch 'main' into avm99963-monorail
Merged commit 34d8229ae2b51fb1a15bd208e6fe6185c94f6266
GitOrigin-RevId: 7ee0917f93a577e475f8e09526dd144d245593f4
diff --git a/businesslogic/test/work_env_test.py b/businesslogic/test/work_env_test.py
index dd93bf7..5b87a13 100644
--- a/businesslogic/test/work_env_test.py
+++ b/businesslogic/test/work_env_test.py
@@ -1,7 +1,6 @@
-# Copyright 2017 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style
-# license that can be found in the LICENSE file or at
-# https://developers.google.com/open-source/licenses/bsd
+# Copyright 2017 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
"""Tests for the WorkEnv class."""
from __future__ import print_function
@@ -10,12 +9,14 @@
import copy
import logging
-import sys
import unittest
import mock
+import six
+import sys
from google.appengine.api import memcache
from google.appengine.ext import testbed
+import pytest
import settings
from businesslogic import work_env
@@ -28,10 +29,10 @@
from framework import permissions
from framework import sorting
from features import send_notifications
-from proto import features_pb2
-from proto import project_pb2
-from proto import tracker_pb2
-from proto import user_pb2
+from mrproto import features_pb2
+from mrproto import project_pb2
+from mrproto import tracker_pb2
+from mrproto import user_pb2
from services import config_svc
from services import features_svc
from services import issue_svc
@@ -46,6 +47,8 @@
from testing import testing_helpers
from tracker import tracker_bizobj
from tracker import tracker_constants
+from redirect import redirectissue
+from api.api_proto import common_pb2
def _Issue(project_id, local_id):
@@ -150,8 +153,8 @@
issue_delta_pairs = [(issue, delta)]
self.SignIn(user_id=self.user_1.user_id)
- with self.assertRaisesRegexp(permissions.PermissionException,
- r'.+int_field\n.+enum_field'):
+ with self.assertRaisesRegex(permissions.PermissionException,
+ r'.+int_field\n.+enum_field'):
with self.work_env as we:
we._AssertUserCanModifyIssues(issue_delta_pairs, True)
@@ -244,8 +247,8 @@
(issue_4, tracker_pb2.IssueDelta(owner_id=self.user_2.user_id)))
self.SignIn(user_id=self.user_1.user_id)
- with self.assertRaisesRegexp(permissions.PermissionException,
- '\n'.join(error_messages_re)):
+ with self.assertRaisesRegex(permissions.PermissionException,
+ '\n'.join(error_messages_re)):
with self.work_env as we:
we._AssertUserCanModifyIssues(
issue_delta_pairs, False, comment_content='ping')
@@ -537,9 +540,10 @@
def AddUserProjects(self):
project_states = {
- 'live': project_pb2.ProjectState.LIVE,
'archived': project_pb2.ProjectState.ARCHIVED,
- 'deletable': project_pb2.ProjectState.DELETABLE}
+ 'live': project_pb2.ProjectState.LIVE,
+ 'deletable': project_pb2.ProjectState.DELETABLE
+ }
projects = {}
for name, state in project_states.items():
@@ -547,9 +551,8 @@
'owner-' + name, state=state, owner_ids=[222])
projects['committer-'+name] = self.services.project.TestAddProject(
'committer-' + name, state=state, committer_ids=[222])
- projects['contributor-'+name] = self.services.project.TestAddProject(
- 'contributor-' + name, state=state)
- projects['contributor-'+name].contributor_ids = [222]
+ projects['contributor-' + name] = self.services.project.TestAddProject(
+ 'contributor-' + name, state=state, contrib_ids=[222])
projects['members-only'] = self.services.project.TestAddProject(
'members-only', owner_ids=[222])
@@ -558,6 +561,7 @@
return projects
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testGatherProjectMembershipsForUser_OtherUser(self):
"""We can get the projects in which a user has a role.
Member only projects are hidden."""
@@ -570,6 +574,7 @@
self.assertEqual([projects['committer-live'].project_id], committer)
self.assertEqual([projects['contributor-live'].project_id], contrib)
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testGatherProjectMembershipsForUser_OwnUser(self):
"""We can get the projects in which the logged in user has a role. """
projects = self.AddUserProjects()
@@ -586,6 +591,7 @@
self.assertEqual([projects['committer-live'].project_id], committer)
self.assertEqual([projects['contributor-live'].project_id], contrib)
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testGatherProjectMembershipsForUser_Admin(self):
"""Admins can see all project roles another user has. """
projects = self.AddUserProjects()
@@ -602,6 +608,7 @@
self.assertEqual([projects['committer-live'].project_id], committer)
self.assertEqual([projects['contributor-live'].project_id], contrib)
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testGetUserRolesInAllProjects_OtherUsers(self):
"""We can get the projects in which the user has a role."""
projects = self.AddUserProjects()
@@ -609,17 +616,17 @@
with self.work_env as we:
owner, member, contrib = we.GetUserRolesInAllProjects({222})
- by_name = lambda project: project.project_name
- self.assertEqual(
- [projects['owner-live']],
- sorted(list(owner.values()), key=by_name))
- self.assertEqual(
- [projects['committer-live']],
- sorted(list(member.values()), key=by_name))
- self.assertEqual(
- [projects['contributor-live']],
- sorted(list(contrib.values()), key=by_name))
+ self.assertCountEqual(
+ [projects['owner-archived'], projects['owner-live']],
+ list(owner.values()))
+ self.assertCountEqual(
+ [projects['committer-archived'], projects['committer-live']],
+ list(member.values()))
+ self.assertCountEqual(
+ [projects['contributor-archived'], projects['contributor-live']],
+ list(contrib.values()))
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testGetUserRolesInAllProjects_OwnUser(self):
"""We can get the projects in which the user has a role."""
projects = self.AddUserProjects()
@@ -628,18 +635,19 @@
with self.work_env as we:
owner, member, contrib = we.GetUserRolesInAllProjects({222})
- by_name = lambda project: project.project_name
- self.assertEqual(
- [projects['members-only'], projects['owner-archived'],
- projects['owner-live']],
- sorted(list(owner.values()), key=by_name))
- self.assertEqual(
+ self.assertCountEqual(
+ [
+ projects['members-only'], projects['owner-archived'],
+ projects['owner-live']
+ ], list(owner.values()))
+ self.assertCountEqual(
[projects['committer-archived'], projects['committer-live']],
- sorted(list(member.values()), key=by_name))
- self.assertEqual(
+ list(member.values()))
+ self.assertCountEqual(
[projects['contributor-archived'], projects['contributor-live']],
- sorted(list(contrib.values()), key=by_name))
+ list(contrib.values()))
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testGetUserRolesInAllProjects_Admin(self):
"""We can get the projects in which the user has a role."""
projects = self.AddUserProjects()
@@ -648,22 +656,28 @@
with self.work_env as we:
owner, member, contrib = we.GetUserRolesInAllProjects({222})
- by_name = lambda project: project.project_name
- self.assertEqual(
- [projects['members-only'], projects['owner-archived'],
- projects['owner-deletable'], projects['owner-live']],
- sorted(list(owner.values()), key=by_name))
- self.assertEqual(
- [projects['committer-archived'], projects['committer-deletable'],
- projects['committer-live']],
- sorted(list(member.values()), key=by_name))
- self.assertEqual(
- [projects['contributor-archived'], projects['contributor-deletable'],
- projects['contributor-live']],
- sorted(list(contrib.values()), key=by_name))
+ self.assertCountEqual(
+ [
+ projects['members-only'], projects['owner-archived'],
+ projects['owner-deletable'], projects['owner-live']
+ ], list(owner.values()))
+ self.assertCountEqual(
+ [
+ projects['committer-archived'], projects['committer-deletable'],
+ projects['committer-live']
+ ], list(member.values()))
+ self.assertCountEqual(
+ [
+ projects['contributor-archived'], projects['contributor-deletable'],
+ projects['contributor-live']
+ ], list(contrib.values()))
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testGetUserProjects_OnlyLiveOfOtherUsers(self):
- """Regular users should only see live projects of other users."""
+ """
+ Regular users should only see permitted projects of other users,
+ regardless of state.
+ """
projects = self.AddUserProjects()
self.SignIn()
@@ -671,10 +685,11 @@
owner, archived, member, contrib = we.GetUserProjects({222})
self.assertEqual([projects['owner-live']], owner)
- self.assertEqual([], archived)
+ self.assertEqual([projects['owner-archived']], archived)
self.assertEqual([projects['committer-live']], member)
self.assertEqual([projects['contributor-live']], contrib)
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testGetUserProjects_AdminSeesAll(self):
"""Admins should see all projects from other users."""
projects = self.AddUserProjects()
@@ -688,6 +703,7 @@
self.assertEqual([projects['committer-live']], member)
self.assertEqual([projects['contributor-live']], contrib)
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testGetUserProjects_UserSeesOwnProjects(self):
"""Users should see all own projects."""
projects = self.AddUserProjects()
@@ -809,11 +825,9 @@
self.SignIn()
we.StarProject(project1.project_id, True)
we.StarProject(project2.project_id, True)
- self.assertItemsEqual(
- [project1, project2], we.ListStarredProjects())
+ six.assertCountEqual(self, [project1, project2], we.ListStarredProjects())
project2.access = project_pb2.ProjectAccess.MEMBERS_ONLY
- self.assertItemsEqual(
- [project1], we.ListStarredProjects())
+ six.assertCountEqual(self, [project1], we.ListStarredProjects())
def testListStarredProjects_ViewingOther(self):
"""A user can view their own starred projects, if they still have access."""
@@ -825,11 +839,12 @@
we.StarProject(project2.project_id, True)
self.SignIn(user_id=111)
self.assertEqual([], we.ListStarredProjects())
- self.assertItemsEqual(
- [project1, project2], we.ListStarredProjects(viewed_user_id=222))
+ six.assertCountEqual(
+ self, [project1, project2],
+ we.ListStarredProjects(viewed_user_id=222))
project2.access = project_pb2.ProjectAccess.MEMBERS_ONLY
- self.assertItemsEqual(
- [project1], we.ListStarredProjects(viewed_user_id=222))
+ six.assertCountEqual(
+ self, [project1], we.ListStarredProjects(viewed_user_id=222))
def testGetProjectConfig_Normal(self):
"""We can get an existing config by project_id."""
@@ -875,6 +890,7 @@
self.services.template.GetProjectTemplates.assert_called_once_with(
self.mr.cnxn, self.project.project_id)
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testListComponentDefs(self):
project = self.services.project.TestAddProject(
'Greece', owner_ids=[self.user_1.user_id])
@@ -1311,8 +1327,8 @@
def testCreateIssue_OnwerValidation(self, _fake_pasicn, _fake_pasibn):
"""We validate the owner."""
self.SignIn(user_id=111)
- with self.assertRaisesRegexp(exceptions.InputException,
- 'Issue owner must be a project member'):
+ with self.assertRaisesRegex(exceptions.InputException,
+ 'Issue owner must be a project member'):
with self.work_env as we:
# user_id 222 is not a project member
we.CreateIssue(789, 'sum', 'New', 222, [333], ['Hot'], [], [], 'desc')
@@ -1589,6 +1605,36 @@
self.assertEqual(
comments[2].content, 'Moved issue dest:1 back to issue proj:1 again.')
+ @mock.patch('services.tracker_fulltext.IndexIssues')
+ @mock.patch('services.tracker_fulltext.UnindexIssues')
+ def testMoveIssue_AllowedRestrictions(self, mock_unindex, mock_index):
+ """We can move restricted issues on allowed projects and labels."""
+ issue = fake.MakeTestIssue(
+ 789, 1, 'sum', 'New', 111, issue_id=78901, project_name='WebRTC')
+ issue.labels = ['Restrict-View-SecurityTeam']
+ self.services.issue.TestAddIssue(issue)
+ self.project.owner_ids = [111]
+ target_project = self.services.project.TestAddProject(
+ 'Chromium', project_id=988, committer_ids=[111])
+
+ self.SignIn(user_id=111)
+ with self.work_env as we:
+ moved_issue = we.MoveIssue(issue, target_project)
+
+ self.assertEqual(moved_issue.project_name, 'Chromium')
+ self.assertEqual(moved_issue.local_id, 1)
+
+ moved_issue = self.services.issue.GetIssueByLocalID(
+ 'cnxn', target_project.project_id, 1)
+ self.assertEqual(target_project.project_id, moved_issue.project_id)
+ self.assertEqual(issue.summary, moved_issue.summary)
+ self.assertEqual(moved_issue.reporter_id, 111)
+
+ mock_unindex.assert_called_once_with([issue.issue_id])
+ mock_index.assert_called_once_with(
+ self.mr.cnxn, [issue], self.services.user, self.services.issue,
+ self.services.config)
+
def testMoveIssue_Anon(self):
"""Anon can't move issues."""
issue = fake.MakeTestIssue(789, 1, 'sum', 'New', 111, issue_id=78901)
@@ -1852,7 +1898,7 @@
issue_3.labels = ['Restrict-View-CoreTeam']
issue_3.project_name = 'farm-proj'
self.services.issue.TestAddIssue(issue_3)
- with self.assertRaisesRegexp(
+ with self.assertRaisesRegex(
permissions.PermissionException,
'User is not allowed to view issue: farm-proj:1.\n' +
'User is not allowed to view issue: farm-proj:3.'):
@@ -1863,8 +1909,8 @@
"""We reject attempts to get a non-existent issue."""
issue_1 = fake.MakeTestIssue(789, 1, 'sum', 'New', 111, issue_id=78901)
self.services.issue.TestAddIssue(issue_1)
- with self.assertRaisesRegexp(exceptions.NoSuchIssueException,
- 'No such issue: 78902\nNo such issue: 78903'):
+ with self.assertRaisesRegex(exceptions.NoSuchIssueException,
+ 'No such issue: 78902\nNo such issue: 78903'):
with self.work_env as we:
_actual = we.GetIssuesDict([78901, 78902, 78903])
@@ -1973,6 +2019,38 @@
with self.work_env as we:
_actual = we.GetIssueByLocalID(789, 1)
+ def testExtractMigratedIdFromLabels(self):
+ with self.work_env as we:
+ actual = we.ExtractMigratedIdFromLabels(
+ ['test-label', 'migrated-to-b-123', 'cob-migrated-to-b-456'])
+ self.assertEqual('123', actual)
+
+ def testExtractMigratedIdFromLabels_NoMigrationLabel(self):
+ with self.work_env as we:
+ actual = we.ExtractMigratedIdFromLabels(['test-label', 'to-b-123'])
+ self.assertEqual(None, actual)
+
+ @mock.patch("redirect.redirectissue.RedirectIssue.Get")
+ def testGetIssueMigratedID_FromDatastore(self, mockRedirect):
+ mockRedirect.return_value = '123'
+ with self.work_env as we:
+ actual = we.GetIssueMigratedID('test', '1', ['migrated-to-b-999'])
+ self.assertEqual('123', actual)
+
+ @mock.patch("redirect.redirectissue.RedirectIssue.Get")
+ def testGetIssueMigratedID_FromLabels(self, mockRedirect):
+ mockRedirect.return_value = None
+ with self.work_env as we:
+ actual = we.GetIssueMigratedID('test', '1', ['migrated-to-b-999'])
+ self.assertEqual('999', actual)
+
+ @mock.patch("redirect.redirectissue.RedirectIssue.Get")
+ def testGetIssueMigratedID_None(self, mockRedirect):
+ mockRedirect.return_value = None
+ with self.work_env as we:
+ actual = we.GetIssueMigratedID('test', '1', None)
+ self.assertEqual(None, actual)
+
def testGetRelatedIssueRefs_None(self):
"""We handle issues that have no related issues."""
issue = fake.MakeTestIssue(789, 1, 'sum', 'New', 111)
@@ -2360,8 +2438,8 @@
set_on=2345,
approver_ids_add=[9876])
- with self.assertRaisesRegexp(exceptions.InputException,
- 'users/9876: User does not exist.'):
+ with self.assertRaisesRegex(exceptions.InputException,
+ 'users/9876: User does not exist.'):
comment = 'stuff'
self.work_env.UpdateIssueApproval(78901, 24, delta, comment, False)
@@ -2642,6 +2720,29 @@
'features.send_notifications.PrepareAndSendIssueBlockingNotification')
@mock.patch(
'features.send_notifications.PrepareAndSendIssueChangeNotification')
+ def testUpdateIssue_FreezeLabels(self, fake_pasicn, fake_pasibn):
+ """We cannot add new labels."""
+ self.SignIn()
+ issue = fake.MakeTestIssue(789, 1, 'summary', 'Available', 111)
+ self.services.issue.TestAddIssue(issue)
+ delta = tracker_pb2.IssueDelta(labels_add=['freeze_new_label'])
+
+ with self.assertRaisesRegex(
+ exceptions.InputException,
+ ('The creation of new labels is blocked for the Chromium project'
+ ' in Monorail. To continue with editing your issue, please'
+ ' remove: freeze_new_label label\\(s\\)')):
+ comment = 'Fake comment'
+ with self.work_env as we:
+ we.UpdateIssue(issue, delta, comment)
+
+ fake_pasicn.assert_not_called()
+ fake_pasibn.assert_not_called()
+
+ @mock.patch(
+ 'features.send_notifications.PrepareAndSendIssueBlockingNotification')
+ @mock.patch(
+ 'features.send_notifications.PrepareAndSendIssueChangeNotification')
def testUpdateIssue_EditTooLongComment(self, fake_pasicn, fake_pasibn):
"""We cannot edit an issue description with too long a comment."""
self.SignIn(222)
@@ -2837,16 +2938,14 @@
with self.work_env as we:
with self.assertRaises(exceptions.InputException) as cm:
we.UpdateIssue(issue, delta, '')
- self.assertEqual('Issue owner user ID not found.',
- cm.exception.message)
+ self.assertEqual('Issue owner user ID not found.', str(cm.exception))
# Not a member
delta = tracker_pb2.IssueDelta(owner_id=222)
with self.work_env as we:
with self.assertRaises(exceptions.InputException) as cm:
we.UpdateIssue(issue, delta, '')
- self.assertEqual('Issue owner must be a project member.',
- cm.exception.message)
+ self.assertEqual('Issue owner must be a project member.', str(cm.exception))
@mock.patch(
'features.send_notifications.PrepareAndSendIssueBlockingNotification')
@@ -2878,7 +2977,7 @@
# Original issue marked as duplicate.
self.assertEqual('Duplicate', merged_issue.status)
# Target issue has original issue's CCs.
- self.assertEqual([444, 333, 222, 111], merged_into_issue.cc_ids)
+ self.assertEqual([111, 222, 333, 444], merged_into_issue.cc_ids)
# A comment was added to the target issue.
merged_into_issue_comment = merged_into_issue_comments[-1]
self.assertEqual(
@@ -2886,10 +2985,10 @@
merged_into_issue_comment.content)
source_starrers = self.services.issue_star.LookupItemStarrers(
'cnxn', merged_issue.issue_id)
- self.assertItemsEqual([111, 222, 333], source_starrers)
+ six.assertCountEqual(self, [111, 222, 333], source_starrers)
target_starrers = self.services.issue_star.LookupItemStarrers(
'cnxn', merged_into_issue.issue_id)
- self.assertItemsEqual([111, 222, 333, 555], target_starrers)
+ six.assertCountEqual(self, [111, 222, 333, 555], target_starrers)
# Notifications should be sent for both
# the merged issue and the merged_into issue.
merged_issue_comments = self.services.issue.GetCommentsForIssue(
@@ -2938,6 +3037,20 @@
with self.assertRaises(permissions.PermissionException):
we.UpdateIssue(issue, delta, '')
+ # Archived project only editable by Owner 111.
+ self.services.project.TestAddProject(
+ 'proj',
+ project_id=779,
+ owner_ids=[111],
+ state=project_pb2.ProjectState.ARCHIVED)
+ issue3 = fake.MakeTestIssue(
+ 779, 1, 'issue in archived project', 'Available', 111)
+ delta = tracker_pb2.IssueDelta(
+ merged_into=issue3.issue_id, status='Duplicate')
+ with self.work_env as we:
+ with self.assertRaises(permissions.PermissionException):
+ we.UpdateIssue(issue, delta, '')
+
# Original issue still available.
self.assertEqual('Available', issue.status)
# Target issue was not modified.
@@ -2958,7 +3071,7 @@
with self.work_env as we:
with self.assertRaises(exceptions.InputException) as cm:
we.UpdateIssue(issue, delta, '')
- self.assertEqual('Cannot merge an issue into itself.', cm.exception.message)
+ self.assertEqual('Cannot merge an issue into itself.', str(cm.exception))
# Original issue still available.
self.assertEqual('Available', issue.status)
@@ -2976,6 +3089,7 @@
issue = fake.MakeTestIssue(789, 1, 'summary', 'Available', 111)
upstream_issue = fake.MakeTestIssue(789, 2, 'umbrella', 'Available', 111)
self.services.issue.TestAddIssue(issue)
+ self.services.issue.TestAddIssue(upstream_issue)
delta = tracker_pb2.IssueDelta(blocked_on_add=[upstream_issue.issue_id])
with self.work_env as we:
@@ -2995,6 +3109,60 @@
'features.send_notifications.PrepareAndSendIssueBlockingNotification')
@mock.patch(
'features.send_notifications.PrepareAndSendIssueChangeNotification')
+ def testUpdateIssue_BlockOnRestrictedIssue(self, fake_pasicn, fake_pasibn):
+ """We cannot block an issue on an issue we cannot view and edit."""
+ self.SignIn(user_id=self.user_3.user_id)
+ issue = fake.MakeTestIssue(789, 1, 'summary', 'Available', 111)
+ issue2 = fake.MakeTestIssue(789, 2, 'summary2', 'Available', 111)
+ self.services.issue.TestAddIssue(issue)
+ self.services.issue.TestAddIssue(issue2)
+
+ issue2.labels = ['Restrict-View-Foo']
+ delta = tracker_pb2.IssueDelta(blocked_on_add=[issue2.issue_id])
+ with self.work_env as we:
+ with self.assertRaises(permissions.PermissionException):
+ we.UpdateIssue(issue, delta, '')
+ issue2.labels = ['Restrict-EditIssue-Foo']
+ with self.work_env as we:
+ with self.assertRaises(permissions.PermissionException):
+ we.UpdateIssue(issue, delta, '')
+
+ delta = tracker_pb2.IssueDelta(blocking_add=[issue2.issue_id])
+ issue2.labels = ['Restrict-View-Bar']
+ with self.work_env as we:
+ with self.assertRaises(permissions.PermissionException):
+ we.UpdateIssue(issue, delta, '')
+ issue2.labels = ['Restrict-EditIssue-Bar']
+ with self.work_env as we:
+ with self.assertRaises(permissions.PermissionException):
+ we.UpdateIssue(issue, delta, '')
+
+ # Archived project only editable by Owner 111.
+ self.services.project.TestAddProject(
+ 'proj',
+ project_id=779,
+ owner_ids=[111],
+ state=project_pb2.ProjectState.ARCHIVED)
+ issue3 = fake.MakeTestIssue(
+ 779, 1, 'issue in archived project', 'Available', 111)
+ delta = tracker_pb2.IssueDelta(blocking_add=[issue3.issue_id])
+ with self.work_env as we:
+ with self.assertRaises(permissions.PermissionException):
+ we.UpdateIssue(issue, delta, '')
+
+ # Original issue was not modified.
+ self.assertEqual(0, len(issue.blocked_on_iids))
+ self.assertEqual(0, len(issue.blocking_iids))
+ # No comment was added.
+ comments = self.services.issue.GetCommentsForIssue('cnxn', issue.issue_id)
+ self.assertEqual(1, len(comments))
+ fake_pasicn.assert_not_called()
+ fake_pasibn.assert_not_called()
+
+ @mock.patch(
+ 'features.send_notifications.PrepareAndSendIssueBlockingNotification')
+ @mock.patch(
+ 'features.send_notifications.PrepareAndSendIssueChangeNotification')
def testUpdateIssue_BlockOnItself(self, fake_pasicn, fake_pasibn):
"""We cannot block an issue on itself."""
self.SignIn()
@@ -3005,13 +3173,13 @@
with self.work_env as we:
with self.assertRaises(exceptions.InputException) as cm:
we.UpdateIssue(issue, delta, '')
- self.assertEqual('Cannot block an issue on itself.', cm.exception.message)
+ self.assertEqual('Cannot block an issue on itself.', str(cm.exception))
delta = tracker_pb2.IssueDelta(blocking_add=[issue.issue_id])
with self.work_env as we:
with self.assertRaises(exceptions.InputException) as cm:
we.UpdateIssue(issue, delta, '')
- self.assertEqual('Cannot block an issue on itself.', cm.exception.message)
+ self.assertEqual('Cannot block an issue on itself.', str(cm.exception))
# Original issue was not modified.
self.assertEqual(0, len(issue.blocked_on_iids))
@@ -3290,6 +3458,7 @@
exp_issue.derived_owner_id = 0
exp_issue.modified_timestamp = self.PAST_TIME
+ exp_issue.migration_modified_timestamp = self.PAST_TIME
# Check we successfully updated the issue in our services layer.
self.assertEqual(exp_issue, self.services.issue.GetIssue(
@@ -3425,8 +3594,8 @@
default_project_name=issue_shared_b.project_name)]
exp_issue_empty.blocking_iids.append(issue_shared_b.issue_id)
- added_refs = [(issue_shared_b.project_name, issue_shared_b.local_id),
- (issue_shared_a.project_name, issue_shared_a.local_id)]
+ added_refs = [(issue_shared_a.project_name, issue_shared_a.local_id),
+ (issue_shared_b.project_name, issue_shared_b.local_id)]
exp_amendments_empty_imp.append(tracker_bizobj.MakeBlockingAmendment(
added_refs, [], default_project_name=issue_empty.project_name))
@@ -3595,6 +3764,7 @@
exp_issue.derived_owner_id = 0
exp_issue.modified_timestamp = self.PAST_TIME
+ exp_issue.migration_modified_timestamp = self.PAST_TIME
self.assertEqual(
exp_issue, self.services.issue.GetIssue(self.cnxn, exp_issue.issue_id))
@@ -3713,6 +3883,7 @@
send_email=send_email)
exp_issue.modified_timestamp = self.PAST_TIME
+ exp_issue.migration_modified_timestamp = self.PAST_TIME
exp_issue.component_modified_timestamp = self.PAST_TIME
exp_issue.component_ids = [self.component_id_2]
@@ -3758,6 +3929,7 @@
send_email=send_email)
exp_issue.modified_timestamp = self.PAST_TIME
+ exp_issue.migration_modified_timestamp = self.PAST_TIME
exp_issue.status_modified_timestamp = self.PAST_TIME
exp_issue.closed_timestamp = self.PAST_TIME
exp_issue.status = 'Fixed'
@@ -3801,6 +3973,7 @@
exp_issue = copy.deepcopy(issue)
exp_issue.cc_ids.extend(delta.cc_ids_add)
exp_issue.modified_timestamp = self.PAST_TIME
+ exp_issue.migration_modified_timestamp = self.PAST_TIME
return issue, exp_amendments, exp_issue
# We expect fake_bulk_notify to send these issues' notifications.
@@ -3999,6 +4172,7 @@
exp_issue = copy.deepcopy(issue)
exp_issue.modified_timestamp = self.PAST_TIME
+ exp_issue.migration_modified_timestamp = self.PAST_TIME
exp_issue.assume_stale = False
self.services.issue.TestAddIssue(issue)
@@ -4052,6 +4226,7 @@
exp_issue = copy.deepcopy(issue)
exp_issue.modified_timestamp = self.PAST_TIME
+ exp_issue.migration_modified_timestamp = self.PAST_TIME
exp_issue.assume_stale = False
self.services.issue.TestAddIssue(issue)
@@ -4068,7 +4243,7 @@
self.SignIn(self.user_1.user_id)
upload = framework_helpers.AttachmentUpload(
- 'BEAR-necessities', 'Forget about your worries and your strife',
+ 'BEAR-necessities', b'Forget about your worries and your strife',
'text/plain')
with self.work_env as we:
@@ -4281,7 +4456,7 @@
789, idx, 'sum', 'New', 111, project_name='proj', issue_id=1000+idx))
self.services.issue.TestAddIssue(issues[-1])
parent_issue.blocked_on_iids.append(issues[-1].issue_id)
- next_rank = sys.maxint
+ next_rank = sys.maxsize
if parent_issue.blocked_on_ranks:
next_rank = parent_issue.blocked_on_ranks[-1] - 1
parent_issue.blocked_on_ranks.append(next_rank)
@@ -4304,7 +4479,7 @@
789, idx, 'sum', 'New', 111, project_name='proj', issue_id=1000+idx))
self.services.issue.TestAddIssue(issues[-1])
parent_issue.blocked_on_iids.append(issues[-1].issue_id)
- next_rank = sys.maxint
+ next_rank = sys.maxsize
if parent_issue.blocked_on_ranks:
next_rank = parent_issue.blocked_on_ranks[-1] - 1
parent_issue.blocked_on_ranks.append(next_rank)
@@ -4941,9 +5116,8 @@
# Now, star a couple of issues.
we.StarIssue(issue1, True)
we.StarIssue(issue2, True)
- self.assertItemsEqual(
- [issue1.issue_id, issue2.issue_id],
- we.ListStarredIssueIDs())
+ six.assertCountEqual(
+ self, [issue1.issue_id, issue2.issue_id], we.ListStarredIssueIDs())
# Check that there is no cross-talk between users.
self.SignIn(user_id=222)
@@ -5003,22 +5177,21 @@
public_group_id, private_group_id = self.setUpUserGroups()
self.SignIn(user_id=555)
with self.work_env as we:
- self.assertItemsEqual(
- we.GetMemberships(111), [public_group_id, private_group_id])
+ six.assertCountEqual(
+ self, we.GetMemberships(111), [public_group_id, private_group_id])
def testGetMemeberships_UserHasNoPerm(self):
public_group_id, _ = self.setUpUserGroups()
self.SignIn(user_id=666)
with self.work_env as we:
- self.assertItemsEqual(
- we.GetMemberships(111), [public_group_id])
+ six.assertCountEqual(self, we.GetMemberships(111), [public_group_id])
def testGetMemeberships_GetOwnMembership(self):
public_group_id, private_group_id = self.setUpUserGroups()
self.SignIn(user_id=111)
with self.work_env as we:
- self.assertItemsEqual(
- we.GetMemberships(111), [public_group_id, private_group_id])
+ six.assertCountEqual(
+ self, we.GetMemberships(111), [public_group_id, private_group_id])
def testListReferencedUsers(self):
"""We return the list of User PBs for the given existing user emails."""
@@ -5028,7 +5201,7 @@
# We ignore emails that are empty or belong to non-existent users.
users, linked_user_ids = we.ListReferencedUsers(
['test4@example.com', 'test5@example.com', 'test6@example.com', ''])
- self.assertItemsEqual(users, [user5, user6])
+ six.assertCountEqual(self, users, [user5, user6])
self.assertEqual(linked_user_ids, [])
def testListReferencedUsers_Linked(self):
@@ -5041,8 +5214,8 @@
# We ignore emails that are empty or belong to non-existent users.
users, linked_user_ids = we.ListReferencedUsers(
['test4@example.com', 'test5@example.com', 'test6@example.com', ''])
- self.assertItemsEqual(users, [user5, user6])
- self.assertItemsEqual(linked_user_ids, [555, 666, 777])
+ six.assertCountEqual(self, users, [user5, user6])
+ six.assertCountEqual(self, linked_user_ids, [555, 666, 777])
def testStarUser_Normal(self):
"""We can star and unstar a user."""
@@ -5155,7 +5328,7 @@
with self.work_env as we:
with self.assertRaises(exceptions.InputException) as cm:
we.InviteLinkedParent('x@example.com')
- self.assertEqual('Linked account names must match', cm.exception.message)
+ self.assertEqual('Linked account names must match', str(cm.exception))
@mock.patch('settings.linkable_domains', {'example.com': ['other.com']})
def testInviteLinkedParent_BadDomain(self):
@@ -5164,8 +5337,7 @@
with self.work_env as we:
with self.assertRaises(exceptions.InputException) as cm:
we.InviteLinkedParent('user_111@hacker.com')
- self.assertEqual(
- 'Linked account unsupported domain', cm.exception.message)
+ self.assertEqual('Linked account unsupported domain', str(cm.exception))
@mock.patch('settings.linkable_domains', {'example.com': ['other.com']})
def testInviteLinkedParent_NoSuchParent(self):
@@ -5599,36 +5771,36 @@
self.mr.cnxn, user_2.user_id))
# Assert users expunged in quick edits and saved queries
- self.assertItemsEqual(
- self.services.features.expunged_users_in_quick_edits, user_ids)
- self.assertItemsEqual(
- self.services.features.expunged_users_in_saved_queries, user_ids)
+ six.assertCountEqual(
+ self, self.services.features.expunged_users_in_quick_edits, user_ids)
+ six.assertCountEqual(
+ self, self.services.features.expunged_users_in_saved_queries, user_ids)
# Assert users expunged in templates and configs
self.assertIsNone(template.owner_id)
- self.assertItemsEqual(
- self.services.config.expunged_users_in_configs, user_ids)
+ six.assertCountEqual(
+ self, self.services.config.expunged_users_in_configs, user_ids)
# Assert users expunged in projects
self.assertEqual(project1.owner_ids, [])
self.assertEqual(project2.contributor_ids, [])
# Assert users expunged in issues
- self.assertItemsEqual(
- self.services.issue.expunged_users_in_issues, user_ids)
+ six.assertCountEqual(
+ self, self.services.issue.expunged_users_in_issues, user_ids)
self.assertTrue(self.services.issue.enqueue_issues_called)
# Assert users expunged in spam
- self.assertItemsEqual(
- self.services.spam.expunged_users_in_spam, user_ids)
+ six.assertCountEqual(
+ self, self.services.spam.expunged_users_in_spam, user_ids)
# Assert users expunged in hotlists
- self.assertItemsEqual(
- self.services.features.expunged_users_in_hotlists, user_ids)
+ six.assertCountEqual(
+ self, self.services.features.expunged_users_in_hotlists, user_ids)
# Assert users expunged in groups
- self.assertItemsEqual(
- self.services.usergroup.expunged_users_in_groups, user_ids)
+ six.assertCountEqual(
+ self, self.services.usergroup.expunged_users_in_groups, user_ids)
# Assert filter rules expunged
self.assertEqual(
@@ -5682,22 +5854,22 @@
self.services.user.TestAddUser('4@test.com', 444)
- self.assertItemsEqual(
- [user_1.email, user_2.email, user_3.email],
+ six.assertCountEqual(
+ self, [user_1.email, user_2.email, user_3.email],
self.services.user.GetAllUserEmailsBatch(self.mr.cnxn, limit=3))
- self.assertItemsEqual(
- [user_5.email, user_6.email],
+ six.assertCountEqual(
+ self, [user_5.email, user_6.email],
self.services.user.GetAllUserEmailsBatch(
self.mr.cnxn, limit=3, offset=4))
# Test existence of deleted user does not change results.
self.services.user.TestAddUser(
'', framework_constants.DELETED_USER_ID)
- self.assertItemsEqual(
- [user_1.email, user_2.email, user_3.email],
+ six.assertCountEqual(
+ self, [user_1.email, user_2.email, user_3.email],
self.services.user.GetAllUserEmailsBatch(self.mr.cnxn, limit=3))
- self.assertItemsEqual(
- [user_5.email, user_6.email],
+ six.assertCountEqual(
+ self, [user_5.email, user_6.email],
self.services.user.GetAllUserEmailsBatch(
self.mr.cnxn, limit=3, offset=4))
@@ -5810,6 +5982,7 @@
self.assertEqual(updated_hotlist, self.hotlist)
fake_update_hotlist.assert_not_called()
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testUpdateHotlist_HotlistNotFound(self):
"""Error is thrown when a hotlist is not found."""
self.SignIn(user_id=self.user_1.user_id)
@@ -6536,6 +6709,7 @@
self.assertEqual(0, len(hotlists))
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testListRecentlyVisitedHotlists(self):
hotlists = [
self.work_env.services.features.CreateHotlist(
@@ -6567,6 +6741,7 @@
with self.work_env as we:
self.assertEqual([], we.ListRecentlyVisitedHotlists())
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testListStarredHotlists(self):
hotlists = [
self.work_env.services.features.CreateHotlist(
@@ -6669,6 +6844,7 @@
with self.work_env as we:
we.GetHotlistStarCount(None)
+ @pytest.mark.skip(reason='Test is flaky (https://crbug.com/monorail/12052)')
def testGetHotlistStarCount_NoSuchHotlist(self):
with self.assertRaises(features_svc.NoSuchHotlistException):
with self.work_env as we:
@@ -7351,35 +7527,3 @@
# FUTURE: UpdateHotlist()
# FUTURE: DeleteHotlist()
-
- def setUpExpungeUsersFromStars(self):
- config = fake.MakeTestConfig(789, [], [])
- self.work_env.services.project_star.SetStarsBatch(
- self.cnxn, 789, [222, 444, 555], True)
- self.work_env.services.issue_star.SetStarsBatch(
- self.cnxn, self.services, config, 78901, [222, 444, 666], True)
- self.work_env.services.hotlist_star.SetStarsBatch(
- self.cnxn, 1678, [222, 444, 555], True)
- self.work_env.services.user_star.SetStarsBatch(
- self.cnxn, 888, [222, 333, 777], True)
- self.work_env.services.user_star.SetStarsBatch(
- self.cnxn, 999, [111, 222, 333], True)
-
- def testExpungeUsersFromStars(self):
- self.setUpExpungeUsersFromStars()
- user_ids = [999, 222, 555]
- self.work_env.expungeUsersFromStars(user_ids)
- self.assertEqual(
- self.work_env.services.project_star.LookupItemStarrers(self.cnxn, 789),
- [444])
- self.assertEqual(
- self.work_env.services.issue_star.LookupItemStarrers(self.cnxn, 78901),
- [444, 666])
- self.assertEqual(
- self.work_env.services.hotlist_star.LookupItemStarrers(self.cnxn, 1678),
- [444])
- self.assertEqual(
- self.work_env.services.user_star.LookupItemStarrers(self.cnxn, 888),
- [333, 777])
- self.assertEqual(
- self.work_env.services.user_star.expunged_item_ids, [999, 222, 555])
diff --git a/businesslogic/work_env.py b/businesslogic/work_env.py
index d15d67f..58d52cb 100644
--- a/businesslogic/work_env.py
+++ b/businesslogic/work_env.py
@@ -1,7 +1,6 @@
-# Copyright 2017 The Chromium Authors. All rights reserved.
-# Use of this source code is governed by a BSD-style
-# license that can be found in the LICENSE file or at
-# https://developers.google.com/open-source/licenses/bsd
+# Copyright 2017 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
"""WorkEnv is a context manager and API for high-level operations.
@@ -68,6 +67,7 @@
from framework import framework_helpers
from framework import framework_views
from framework import permissions
+from redirect import redirectissue
from search import frontendsearchpipeline
from services import features_svc
from services import tracker_fulltext
@@ -79,10 +79,10 @@
from tracker import tracker_constants
from tracker import tracker_helpers
from project import project_helpers
-from proto import features_pb2
-from proto import project_pb2
-from proto import tracker_pb2
-from proto import user_pb2
+from mrproto import features_pb2
+from mrproto import project_pb2
+from mrproto import tracker_pb2
+from mrproto import user_pb2
# TODO(jrobbins): break this file into one facade plus ~5
@@ -176,7 +176,8 @@
permitted = self._UserCanUsePermInIssue(issue, perm)
if not permitted:
raise permissions.PermissionException(
- 'User lacks permission %r in issue' % perm)
+ 'User lacks permission %r in issue %s %d', perm, issue.project_name,
+ issue.local_id)
def _AssertUserCanModifyIssues(
self, issue_delta_pairs, is_description_change, comment_content=None):
@@ -216,6 +217,15 @@
delta.merged_into, use_cache=False, allow_viewing_deleted=True)
self._AssertPermInIssue(merged_into_issue, permissions.EDIT_ISSUE)
+ # User cannot modify blocking issues on issues they cannot edit.
+ all_block = (
+ delta.blocked_on_add + delta.blocking_add +
+ delta.blocked_on_remove + delta.blocking_remove)
+ for block_iid in all_block:
+ blocked_issue = self.GetIssue(
+ block_iid, use_cache=False, allow_viewing_deleted=True)
+ self._AssertPermInIssue(blocked_issue, permissions.EDIT_ISSUE)
+
# User cannot change values for restricted fields they cannot edit.
field_ids = [fv.field_id for fv in delta.field_vals_add]
field_ids.extend([fv.field_id for fv in delta.field_vals_remove])
@@ -225,7 +235,7 @@
self._AssertUserCanEditFieldsAndEnumMaskedLabels(
project, config, field_ids, labels)
except permissions.PermissionException as e:
- err_agg.AddErrorMessage(e.message)
+ err_agg.AddErrorMessage(str(e))
if issue_perms.HasPerm(permissions.EDIT_ISSUE, self.mc.auth.user_id,
project):
@@ -324,7 +334,7 @@
try:
self._AssertUserCanEditValueForFieldDef(project, fd)
except permissions.PermissionException as e:
- err_agg.AddErrorMessage(e.message)
+ err_agg.AddErrorMessage(str(e))
def _AssertUserCanViewFieldDef(self, project, field):
"""Make sure the user may view the field."""
@@ -395,9 +405,12 @@
# the results are filtered by permission to view each project.
with self.mc.profiler.Phase('list projects for %r' % self.mc.auth.user_id):
- project_ids = self.services.project.GetVisibleLiveProjects(
- self.mc.cnxn, self.mc.auth.user_pb, self.mc.auth.effective_ids,
- domain=domain, use_cache=use_cache)
+ project_ids = self.services.project.GetVisibleProjects(
+ self.mc.cnxn,
+ self.mc.auth.user_pb,
+ self.mc.auth.effective_ids,
+ domain=domain,
+ use_cache=use_cache)
return project_ids
@@ -935,7 +948,7 @@
'Ancestor path %s is invalid.' % ancestor_path)
project_perms = permissions.GetPermissions(
self.mc.auth.user_pb, self.mc.auth.effective_ids, project)
- if not permissions.CanEditComponentDef(
+ if not permissions.CanEditComponentDefLegacy(
self.mc.auth.effective_ids, project_perms, project, ancestor_def,
config):
raise permissions.PermissionException(
@@ -976,7 +989,7 @@
project_perms = permissions.GetPermissions(
self.mc.auth.user_pb, self.mc.auth.effective_ids, project)
- if not permissions.CanEditComponentDef(
+ if not permissions.CanEditComponentDefLegacy(
self.mc.auth.effective_ids, project_perms, project, component_def,
config):
raise permissions.PermissionException(
@@ -1031,14 +1044,14 @@
owner_id, # type: int
cc_ids, # type: Sequence[int]
labels, # type: Sequence[str]
- field_values, # type: Sequence[proto.tracker_pb2.FieldValue]
+ field_values, # type: Sequence[mrproto.tracker_pb2.FieldValue]
component_ids, # type: Sequence[int]
marked_description, # type: str
blocked_on=None, # type: Sequence[int]
blocking=None, # type: Sequence[int]
attachments=None, # type: Sequence[Tuple[str, str, str]]
- phases=None, # type: Sequence[proto.tracker_pb2.Phase]
- approval_values=None, # type: Sequence[proto.tracker_pb2.ApprovalValue]
+ phases=None, # type: Sequence[mrproto.tracker_pb2.Phase]
+ approval_values=None, # type: Sequence[mrproto.tracker_pb2.ApprovalValue]
send_email=True, # type: bool
reporter_id=None, # type: int
timestamp=None, # type: int
@@ -1046,7 +1059,8 @@
dangling_blocking=None, # type: Sequence[DanglingIssueRef]
raise_filter_errors=True, # type: bool
):
- # type: (...) -> (proto.tracker_pb2.Issue, proto.tracker_pb2.IssueComment)
+ # type: (...) ->
+ # (mrproto.tracker_pb2.Issue, mrproto.tracker_pb2.IssueComment)
"""Create and store a new issue with all the given information.
Args:
@@ -1134,6 +1148,7 @@
issue.owner_modified_timestamp = timestamp
issue.status_modified_timestamp = timestamp
issue.component_modified_timestamp = timestamp
+ issue.migration_modified_timestamp = timestamp
# Validate the issue
tracker_helpers.AssertValidIssueForCreate(
@@ -1273,7 +1288,18 @@
self._AssertPermInIssue(issue, permissions.DELETE_ISSUE)
self._AssertPermInProject(permissions.EDIT_ISSUE, target_project)
- if permissions.GetRestrictions(issue):
+ restrictions = permissions.GetRestrictions(issue)
+ # Issues with allowed labels may move between allowed projects.
+ # Context: https://crbug.com/monorail/11894
+ allowed_project_names = ['chromium', 'webrtc']
+ allowed_labels = frozenset(
+ ['restrict-view-securityteam', 'restrict-view-securitynotify'])
+ if (target_project.project_name.lower()
+ in allowed_project_names) and (issue.project_name.lower()
+ in allowed_project_names):
+ restrictions = set(restrictions) - allowed_labels
+
+ if restrictions:
raise exceptions.InputException(
'Issues with Restrict labels are not allowed to be moved')
@@ -1398,7 +1424,7 @@
group_by_spec, # type: str
sort_spec, # type: str
use_cached_searches, # type: bool
- project=None # type: proto.Project
+ project=None # type: mrproto.Project
):
# type: (...) -> search.frontendsearchpipeline.FrontendSearchPipeline
"""Do an issue search w/ mc + passed in args to return a pipeline object.
@@ -1527,7 +1553,7 @@
self._AssertUserCanViewIssue(
issue, allow_viewing_deleted=allow_viewing_deleted)
except permissions.PermissionException as e:
- permission_err_agg.AddErrorMessage(e.message)
+ permission_err_agg.AddErrorMessage(str(e))
return issues_by_id
@@ -1600,6 +1626,25 @@
issue, allow_viewing_deleted=allow_viewing_deleted)
return issue
+ def ExtractMigratedIdFromLabels(self, labels):
+ """Returns the issue ID from a migration label if present."""
+ # Assume that there's only one migrated label.
+ # Or at least drop any labels besides the first one.
+ if labels is not None:
+ for label in labels:
+ lower_label = label.lower()
+ for prefix in settings.migrated_buganizer_issue_prefixes:
+ if lower_label.startswith(prefix):
+ return label.replace(prefix, '')
+ return None
+
+ def GetIssueMigratedID(self, project_name, local_id, labels=None):
+ """Return the redirect id for a specific issue."""
+ migrated_id = redirectissue.RedirectIssue.Get(project_name, local_id)
+ if migrated_id is not None:
+ return migrated_id
+ return self.ExtractMigratedIdFromLabels(labels)
+
def GetRelatedIssueRefs(self, issues):
"""Return a dict {iid: (project_name, local_id)} for all related issues."""
related_iids = set()
@@ -1609,7 +1654,6 @@
related_iids.update(issue.blocking_iids)
if issue.merged_into:
related_iids.add(issue.merged_into)
- logging.info('related_iids is %r', related_iids)
return self.services.issue.LookupIssueRefs(self.mc.cnxn, related_iids)
def GetIssueRefs(self, issue_ids):
@@ -1647,7 +1691,7 @@
def BulkUpdateIssueApprovalsV3(
self, delta_specifications, comment_content, send_email):
# type: (Sequence[Tuple[int, int, tracker_pb2.ApprovalDelta]]], str,
- # Boolean -> Sequence[proto.tracker_pb2.ApprovalValue]
+ # Boolean -> Sequence[mrproto.tracker_pb2.ApprovalValue]
"""Executes the ApprovalDeltas.
Args:
@@ -1691,10 +1735,10 @@
send_email=True,
kept_attachments=None,
update_perms=False):
- # type: (int, int, proto.tracker_pb2.ApprovalDelta, str, Boolean,
- # Optional[Sequence[proto.tracker_pb2.Attachment]], Optional[Boolean],
+ # type: (int, int, mrproto.tracker_pb2.ApprovalDelta, str, Boolean,
+ # Optional[Sequence[mrproto.tracker_pb2.Attachment]], Optional[Boolean],
# Optional[Sequence[int]], Optional[Boolean]) ->
- # (proto.tracker_pb2.ApprovalValue, proto.tracker_pb2.IssueComment)
+ # (mrproto.tracker_pb2.ApprovalValue, mrproto.tracker_pb2.IssueComment)
"""Update an issue's approval.
Raises:
@@ -1769,7 +1813,7 @@
def ConvertIssueApprovalsTemplate(
self, config, issue, template_name, comment_content, send_email=True):
- # type: (proto.tracker_pb2.ProjectIssueConfig, proto.tracker_pb2.Issue,
+ # type: (mrproto.tracker_pb2.ProjectIssueConfig, mrproto.tracker_pb2.Issue,
# str, str, Optional[Boolean] )
"""Convert an issue's existing approvals structure to match the one of
the given template.
@@ -1853,6 +1897,7 @@
# Reject attempts to merge an issue into an issue we cannot view and edit.
merged_into_issue = self.GetIssue(
delta.merged_into, use_cache=False, allow_viewing_deleted=True)
+ self._AssertPermInIssue(merged_into_issue, permissions.EDIT_ISSUE)
self._AssertPermInIssue(issue, permissions.EDIT_ISSUE)
# Reject attempts to merge an issue into itself.
if issue.issue_id == delta.merged_into:
@@ -1864,6 +1909,15 @@
comment_content) > tracker_constants.MAX_COMMENT_CHARS:
raise exceptions.InputException('Comment is too long')
+ # Reject attempts to modifying blocking issues we cannot edit.
+ all_block = (
+ delta.blocked_on_add + delta.blocking_add + delta.blocked_on_remove +
+ delta.blocking_remove)
+ for block_iid in all_block:
+ blocked_issue = self.GetIssue(
+ block_iid, use_cache=False, allow_viewing_deleted=True)
+ self._AssertPermInIssue(blocked_issue, permissions.EDIT_ISSUE)
+
# Reject attempts to block on issue on itself.
if (issue.issue_id in delta.blocked_on_add
or issue.issue_id in delta.blocking_add):
@@ -1877,7 +1931,13 @@
field_ids = [fv.field_id for fv in delta.field_vals_add]
field_ids.extend([fvr.field_id for fvr in delta.field_vals_remove])
field_ids.extend(delta.fields_clear)
+
labels = itertools.chain(delta.labels_add, delta.labels_remove)
+ labels_err_msg = field_helpers.ValidateLabels(
+ self.mc.cnxn, self.services, issue.project_id, delta.labels_add)
+ if labels_err_msg:
+ raise exceptions.InputException(labels_err_msg)
+
self._AssertUserCanEditFieldsAndEnumMaskedLabels(
project, config, field_ids, labels)
@@ -2062,6 +2122,7 @@
changes.issues_to_update_dict[issue.issue_id] = issue
issue.modified_timestamp = now_timestamp
+ issue.migration_modified_timestamp = now_timestamp
if (iid in changes.old_owners_by_iid and
old_owner != tracker_bizobj.GetOwnerId(issue)):
@@ -2142,8 +2203,8 @@
self.mc.cnxn, pid, attachment_bytes_used=new_bytes_used, commit=False)
# Reindex issues and commit all DB changes.
- issues_to_reindex = set(
- comments_by_iid.keys() + impacted_comments_by_iid.keys())
+ issues_to_reindex = (
+ set(comments_by_iid.keys()) | set(impacted_comments_by_iid.keys()))
if issues_to_reindex:
self.services.issue.EnqueueIssuesForIndexing(
self.mc.cnxn, issues_to_reindex, commit=False)
@@ -2168,9 +2229,9 @@
# Group issues for each unique delta by project because
# SendIssueBulkChangeNotification cannot handle cross-project
# notifications and hostports are specific to each project.
- issues_by_pid = collections.defaultdict(set)
+ issues_by_pid = collections.defaultdict(list)
for issue in issues:
- issues_by_pid[issue.project_id].add(issue)
+ issues_by_pid[issue.project_id].append(issue)
for project_issues in issues_by_pid.values():
# Send one email to involved users for the issue.
if len(project_issues) == 1:
@@ -3806,20 +3867,6 @@
self.services.features.UpdateHotlistItemsFields(
self.mc.cnxn, hotlist_id, new_notes=new_notes)
- def expungeUsersFromStars(self, user_ids):
- """Wipes any starred user or user's stars from all star services.
-
- This method will not commit the operation. This method will not
- make changes to in-memory data.
- """
-
- self.services.project_star.ExpungeStarsByUsers(self.mc.cnxn, user_ids)
- self.services.issue_star.ExpungeStarsByUsers(self.mc.cnxn, user_ids)
- self.services.hotlist_star.ExpungeStarsByUsers(self.mc.cnxn, user_ids)
- self.services.user_star.ExpungeStarsByUsers(self.mc.cnxn, user_ids)
- for user_id in user_ids:
- self.services.user_star.ExpungeStars(self.mc.cnxn, user_id, commit=False)
-
# Permissions
# ListFooPermission methods will return the list of permissions in addition to