fix(pull-requests): fixes for rendering comments

This commit is contained in:
RhodeCode Admin 2023-11-28 16:31:41 +01:00
parent f5a73506ea
commit 3cb072893c
3 changed files with 44 additions and 24 deletions

View file

@ -357,7 +357,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
pr_closed = pull_request_latest.is_closed()
if pr_closed and (version or from_version):
# not allow to browse versions for closed PR
# not allow browsing versions for closed PR
raise HTTPFound(h.route_path(
'pullrequest_show', repo_name=self.db_repo_name,
pull_request_id=pull_request_id))
@ -394,7 +394,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
compare = at_version != prev_at_version
# pull_requests repo_name we opened it against
# ie. target_repo must match
# i.e., target_repo must match
if self.db_repo_name != pull_request_at_ver.target_repo.repo_name:
log.warning('Mismatch between the current repo: %s, and target %s',
self.db_repo_name, pull_request_at_ver.target_repo.repo_name)
@ -470,6 +470,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
c.pull_request_set_reviewers_data_json = collections.OrderedDict({'reviewers': []})
c.pull_request_set_observers_data_json = collections.OrderedDict({'observers': []})
# reviewers
for review_obj, member, reasons, mandatory, status in pull_request_at_ver.reviewers_statuses():
member_reviewer = h.reviewer_as_json(
member, reasons=reasons, mandatory=mandatory,
@ -485,6 +486,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
c.pull_request_set_reviewers_data_json = ext_json.str_json(c.pull_request_set_reviewers_data_json)
# observers
for observer_obj, member in pull_request_at_ver.observers():
member_observer = h.reviewer_as_json(
member, reasons=[], mandatory=False,
@ -620,7 +622,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
target_commit,
target_ref_id,
target_scm,
maybe_unreachable=maybe_unreachable)
maybe_unreachable=maybe_unreachable)
# register our commit range
for comm in commit_cache.values():
@ -763,7 +765,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
try:
commit = commits_source_repo.get_commit(raw_id)
except CommitDoesNotExistError:
# in case we fail extracting still use "dummy" commit
# in case we fail getting the commit, still use a dummy commit
# for display in commit diff
commit = h.AttributeDict(
{'raw_id': raw_id,
@ -1722,6 +1724,7 @@ class RepoPullRequestsView(RepoAppView, DataGridAppView):
'f_path': self.request.POST.get('f_path'),
'line': self.request.POST.get('line'),
}
data = self._pull_request_comments_create(pull_request, [comment_data])
return data

View file

@ -65,7 +65,7 @@ from rhodecode.lib.jsonalchemy import (
from rhodecode.lib.hash_utils import sha1
from rhodecode.lib import ext_json
from rhodecode.lib import enc_utils
from rhodecode.lib.ext_json import json
from rhodecode.lib.ext_json import json, str_json
from rhodecode.lib.caching_query import FromCache
from rhodecode.lib.exceptions import (
ArtifactMetadataDuplicate, ArtifactMetadataBadValueType)
@ -3845,7 +3845,7 @@ class ChangesetComment(Base, BaseModel):
@property
def outdated_js(self):
return json.dumps(self.display_state == self.COMMENT_OUTDATED)
return str_json(self.display_state == self.COMMENT_OUTDATED)
@property
def immutable(self):
@ -3855,6 +3855,15 @@ class ChangesetComment(Base, BaseModel):
"""
Checks if comment is outdated for given pull request version
"""
# If version is None, return False as the current version cannot be less than None
if version is None:
return False
# Ensure that the version is an integer to prevent TypeError on comparison
if not isinstance(version, int):
raise ValueError("The provided version must be an integer.")
def version_check():
return self.pull_request_version_id and self.pull_request_version_id != version
@ -3868,26 +3877,35 @@ class ChangesetComment(Base, BaseModel):
"""
Checks if comment is outdated for given pull request version
"""
return json.dumps(self.outdated_at_version(version))
return str_json(self.outdated_at_version(version))
def older_than_version(self, version):
def older_than_version(self, version: int) -> bool:
"""
Checks if comment is made from previous version than given
Checks if comment is made from a previous version than given.
Assumes self.pull_request_version.pull_request_version_id is an integer if not None.
"""
cur_ver = 0
if self.pull_request_version:
cur_ver = self.pull_request_version.pull_request_version_id or cur_ver
# If version is None, return False as the current version cannot be less than None
if version is None:
return cur_ver != version
return False
# Ensure that the version is an integer to prevent TypeError on comparison
if not isinstance(version, int):
raise ValueError("The provided version must be an integer.")
# Initialize current version to 0 or pull_request_version_id if it's available
cur_ver = 0
if self.pull_request_version and self.pull_request_version.pull_request_version_id is not None:
cur_ver = self.pull_request_version.pull_request_version_id
# Return True if the current version is less than the given version
return cur_ver < version
def older_than_version_js(self, version):
"""
Checks if comment is made from previous version than given
"""
return json.dumps(self.older_than_version(version))
return str_json(self.older_than_version(version))
@property
def commit_id(self):
@ -4217,7 +4235,7 @@ class _PullRequestBase(BaseModel):
@property
def reviewer_data_json(self):
return json.dumps(self.reviewer_data)
return str_json(self.reviewer_data)
@property
def last_merge_metadata_parsed(self):

View file

@ -225,12 +225,12 @@ return '%s_%s_%i' % (h.md5_safe(commit+filename), type, line)
## file was renamed, or copied
%if RENAMED_FILENODE in filediff.patch['stats']['ops']:
<%
final_file_name = h.literal(u'{} <i class="icon-angle-left"></i> <del>{}</del>'.format(filediff.target_file_path, filediff.source_file_path))
final_file_name = h.literal('{} <i class="icon-angle-left"></i> <del>{}</del>'.format(filediff.target_file_path, filediff.source_file_path))
final_path = filediff.target_file_path
%>
%elif COPIED_FILENODE in filediff.patch['stats']['ops']:
<%
final_file_name = h.literal(u'{} <i class="icon-angle-left"></i> {}'.format(filediff.target_file_path, filediff.source_file_path))
final_file_name = h.literal('{} <i class="icon-angle-left"></i> {}'.format(filediff.target_file_path, filediff.source_file_path))
final_path = filediff.target_file_path
%>
%endif
@ -272,7 +272,7 @@ return '%s_%s_%i' % (h.md5_safe(commit+filename), type, line)
>
<label for="filediff-collapse-${id(filediff)}" class="filediff-heading">
<%
file_comments = (get_inline_comments(inline_comments, filediff.patch['filename']) or {}).values()
file_comments = list((get_inline_comments(inline_comments, filediff.patch['filename']) or {}).values())
total_file_comments = [_c for _c in h.itertools.chain.from_iterable(file_comments) if not (_c.outdated or _c.draft)]
%>
<div class="filediff-collapse-indicator icon-"></div>
@ -302,7 +302,6 @@ return '%s_%s_%i' % (h.md5_safe(commit+filename), type, line)
<span class="pull-right icon-clipboard clipboard-action" data-clipboard-text="${permalink}" title="Copy permalink"></span>
</div>
</details-menu>
</details>
@ -667,8 +666,8 @@ return '%s_%s_%i' % (h.md5_safe(commit+filename), type, line)
<%!
def get_inline_comments(comments, filename):
if hasattr(filename, 'unicode_path'):
filename = filename.unicode_path
if hasattr(filename, 'str_path'):
filename = filename.str_path
if not isinstance(filename, str):
return None
@ -679,8 +678,8 @@ def get_inline_comments(comments, filename):
return None
def get_comments_for(diff_type, comments, filename, line_version, line_number):
if hasattr(filename, 'unicode_path'):
filename = filename.unicode_path
if hasattr(filename, 'str_path'):
filename = filename.str_path
if not isinstance(filename, str):
return None
@ -689,7 +688,7 @@ def get_comments_for(diff_type, comments, filename, line_version, line_number):
if file_comments is None:
return None
line_key = '{}{}'.format(line_version, line_number) ## e.g o37, n12
line_key = f'{line_version}{line_number}' ## e.g o37, n12
if line_key in file_comments:
data = file_comments.pop(line_key)
return data