Merge branch 'main' into avm99963-monorail
Merged commit 34d8229ae2b51fb1a15bd208e6fe6185c94f6266
GitOrigin-RevId: 7ee0917f93a577e475f8e09526dd144d245593f4
diff --git a/tracker/tracker_helpers.py b/tracker/tracker_helpers.py
index cd9acfa..f339b95 100644
--- a/tracker/tracker_helpers.py
+++ b/tracker/tracker_helpers.py
@@ -1,7 +1,6 @@
-# Copyright 2016 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 2022 The Chromium Authors
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
"""Helper functions and classes used by the Monorail Issue Tracker pages.
@@ -38,7 +37,7 @@
from framework import template_helpers
from framework import urls
from project import project_helpers
-from proto import tracker_pb2
+from mrproto import tracker_pb2
from services import client_config_svc
from tracker import field_helpers
from tracker import tracker_bizobj
@@ -153,16 +152,14 @@
status = post_data.get('status', '')
template_name = urllib.parse.unquote_plus(post_data.get('template_name', ''))
component_str = post_data.get('components', '')
- # TODO: switch when convert /p to flask
- # label_strs = post_data.getlist('label')
- label_strs = post_data.getall('label')
+ label_strs = post_data.getlist('label')
if is_description:
tmpl_txt = post_data.get('tmpl_txt', '')
comment = MarkupDescriptionOnInput(comment, tmpl_txt)
comp_paths, comp_paths_remove = _ClassifyPlusMinusItems(
- re.split('[,;\s]+', component_str))
+ re.split(r'[,;\s]+', component_str))
parsed_components = ParsedComponents(
component_str, comp_paths, comp_paths_remove)
labels, labels_remove = _ClassifyPlusMinusItems(label_strs)
@@ -234,7 +231,7 @@
def _ParseHotlists(post_data):
entered_str = post_data.get('hotlists', '').strip()
hotlist_refs = []
- for ref_str in re.split('[,;\s]+', entered_str):
+ for ref_str in re.split(r'[,;\s]+', entered_str):
if not ref_str:
continue
if ':' in ref_str:
@@ -259,9 +256,7 @@
phase_field_val_strs_remove = collections.defaultdict(dict)
for key in post_data.keys():
if key.startswith(_CUSTOM_FIELD_NAME_PREFIX):
- # TODO: switch when convert /p to flask
- # val_strs = [v for v in post_data.getlist(key) if v]
- val_strs = [v for v in post_data.getall(key) if v]
+ val_strs = [v for v in post_data.getlist(key) if v]
if val_strs:
try:
field_id = int(key[len(_CUSTOM_FIELD_NAME_PREFIX):])
@@ -315,9 +310,10 @@
item.filename = item.filename[item.filename.rindex('\\') + 1:]
if not item.filename:
continue # Skip any FILE fields that were not filled in.
- attachments.append((
- item.filename, item.value,
- filecontent.GuessContentTypeFromFilename(item.filename)))
+ attachments.append(
+ (
+ item.filename, item.read(),
+ filecontent.GuessContentTypeFromFilename(item.filename)))
return attachments
@@ -331,9 +327,7 @@
Returns:
a list of attachment ids for kept attachments
"""
- # TODO: switch when convert /p to flask
- # kept_attachments = post_data.getlist('keep-attachment')
- kept_attachments = post_data.getall('keep-attachment')
+ kept_attachments = post_data.getlist('keep-attachment')
return [int(aid) for aid in kept_attachments]
@@ -359,7 +353,7 @@
owner_email = post_data.get('owner', '').strip().lower()
cc_usernames, cc_usernames_remove = _ClassifyPlusMinusItems(
- re.split('[,;\s]+', cc_username_str))
+ re.split(r'[,;\s]+', cc_username_str))
# Figure out the email addresses to lookup and do the lookup.
emails_to_lookup = cc_usernames + cc_usernames_remove
@@ -401,7 +395,7 @@
federated_ref_strings = []
issue_ref = None
- for ref_str in re.split('[,;\s]+', entered_str):
+ for ref_str in re.split(r'[,;\s]+', entered_str):
# Handle federated references.
if federated.IsShortlinkValid(ref_str):
federated_ref_strings.append(ref_str)
@@ -940,7 +934,7 @@
def ParsePostDataUsers(cnxn, pd_users_str, user_service):
"""Parse all the usernames from a users string found in a post data."""
- emails, _remove = _ClassifyPlusMinusItems(re.split('[,;\s]+', pd_users_str))
+ emails, _remove = _ClassifyPlusMinusItems(re.split(r'[,;\s]+', pd_users_str))
users_ids_by_email = user_service.LookupUserIDs(cnxn, emails, autocreate=True)
user_ids = [users_ids_by_email[username] for username in emails if username]
return user_ids, pd_users_str
@@ -1023,25 +1017,19 @@
cnxn, services, config, merge_into_iid, new_starrers, True)
-def IsMergeAllowed(merge_into_issue, mr, services):
- """Check to see if user has permission to merge with specified issue."""
- merge_into_project = services.project.GetProjectByName(
- mr.cnxn, merge_into_issue.project_name)
- merge_into_config = services.config.GetProjectConfig(
- mr.cnxn, merge_into_project.project_id)
- merge_granted_perms = tracker_bizobj.GetGrantedPerms(
- merge_into_issue, mr.auth.effective_ids, merge_into_config)
+def CanEditProjectIssue(mr, project, issue, granted_perms):
+ """Check if user permissions in another project allow editing.
- merge_view_allowed = mr.perms.CanUsePerm(
- permissions.VIEW, mr.auth.effective_ids,
- merge_into_project, permissions.GetRestrictions(merge_into_issue),
- granted_perms=merge_granted_perms)
- merge_edit_allowed = mr.perms.CanUsePerm(
- permissions.EDIT_ISSUE, mr.auth.effective_ids,
- merge_into_project, permissions.GetRestrictions(merge_into_issue),
- granted_perms=merge_granted_perms)
+ Wraps CanEditIssue with a call to get user permissions in given project.
- return merge_view_allowed and merge_edit_allowed
+ We deviate from using CanUsePerm because that method does not calculate
+ Project state as part of the permissions. This seems to have deviated in
+ 2018. CanEditIssue uses Project state to authorize user actions.
+ """
+ project_perms = permissions.GetPermissions(
+ mr.auth.user_pb, mr.auth.effective_ids, project)
+ return permissions.CanEditIssue(
+ mr.auth.effective_ids, project_perms, project, issue, granted_perms)
def GetVisibleMembers(mr, project, services):
@@ -1243,8 +1231,8 @@
def _GetEnumFieldValuesAndDocstrings(field_def, config):
- # type: (proto.tracker_pb2.LabelDef, proto.tracker_pb2.ProjectIssueConfig) ->
- # Sequence[tuple(string, string)]
+ # type: (mrproto.tracker_pb2.LabelDef,
+ # mrproto.tracker_pb2.ProjectIssueConfig) -> Sequence[tuple(string, string)]
"""Get sequence of value, docstring tuples for an enum field"""
label_defs = config.well_known_labels
lower_field_name = field_def.field_name.lower()
@@ -1354,8 +1342,8 @@
def UpdateClosedTimestamp(config, issue, old_effective_status):
- # type: (proto.tracker_pb2.ProjectIssueConfig, proto.tracker_pb2.Issue, str)
- # -> None
+ # type: (mrproto.tracker_pb2.ProjectIssueConfig,
+ # mrproto.tracker_pb2.Issue, str) -> None
"""Sets or unsets the closed_timestamp based based on status changes.
If the status is changing from open to closed, the closed_timestamp is set to
@@ -1496,7 +1484,7 @@
new_bytes_by_pid = {}
with exceptions.ErrorAggregator(exceptions.OverAttachmentQuota) as err_agg:
- for pid, count in issue_count_by_pid.items():
+ for pid, count in sorted(issue_count_by_pid.items()):
project = projects_by_id[pid]
try:
new_bytes_used = ComputeNewQuotaBytesUsed(
@@ -1597,6 +1585,10 @@
err_agg.AddErrorMessage('{}: Summary required.', issue_ref)
if delta.status == '':
err_agg.AddErrorMessage('{}: Status is required.', issue_ref)
+ labels_err_msgs = field_helpers.ValidateLabels(
+ cnxn, services, issue.project_id, delta.labels_add)
+ if labels_err_msgs:
+ err_agg.AddErrorMessage('{}: {}', issue_ref, labels_err_msgs)
# Do not pass in issue for validation, as issue is pre-update, and would
# result in being unable to edit issues in invalid states.
fvs_err_msgs = field_helpers.ValidateCustomFields(
@@ -1659,6 +1651,11 @@
all_users.extend(field_users)
AssertUsersExist(cnxn, services, all_users, err_agg)
+ label_validity_error = field_helpers.ValidateLabels(
+ cnxn, services, issue.project_id, issue.labels)
+ if label_validity_error:
+ err_agg.AddErrorMessage(label_validity_error)
+
field_validity_errors = field_helpers.ValidateCustomFields(
cnxn, services, issue.field_values, config, project, issue=issue)
if field_validity_errors:
@@ -1695,7 +1692,10 @@
if issue.owner_id:
new_cc_ids.add(issue.owner_id)
- return [cc_id for cc_id in new_cc_ids if cc_id not in merge_into_issue.cc_ids]
+ return [
+ cc_id for cc_id in sorted(new_cc_ids)
+ if cc_id not in merge_into_issue.cc_ids
+ ]
def _EnforceNonMergeStatusDeltas(cnxn, issue_delta_pairs, services):
@@ -1745,9 +1745,11 @@
def ComputeAllImpactedIIDs(self):
# type: () -> Collection[int]
"""Computes the unique set of all impacted issue ids."""
- return set(self.blocking_add.keys() + self.blocking_remove.keys() +
- self.blocked_on_add.keys() + self.blocked_on_remove.keys() +
- self.merged_from_add.keys() + self.merged_from_remove.keys())
+ return (
+ set(self.blocking_add.keys()) | set(self.blocking_remove.keys())
+ | set(self.blocked_on_add.keys()) | set(self.blocked_on_remove.keys())
+ | set(self.merged_from_add.keys())
+ | set(self.merged_from_remove.keys()))
def TrackImpactedIssues(self, issue, delta):
# type: (Issue, IssueDelta) -> None