diff --git a/rhodecode/apps/repository/views/repo_pull_requests.py b/rhodecode/apps/repository/views/repo_pull_requests.py --- a/rhodecode/apps/repository/views/repo_pull_requests.py +++ b/rhodecode/apps/repository/views/repo_pull_requests.py @@ -357,7 +357,7 @@ class RepoPullRequestsView(RepoAppView, 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, 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, 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, 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, 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, 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, '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 diff --git a/rhodecode/model/db.py b/rhodecode/model/db.py --- a/rhodecode/model/db.py +++ b/rhodecode/model/db.py @@ -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)) - - def older_than_version(self, version): + return str_json(self.outdated_at_version(version)) + + def older_than_version(self, version: int) -> bool: + """ + 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. """ - Checks if comment is made from previous version than given - """ + + # 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.") + + # Initialize current version to 0 or pull_request_version_id if it's available 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 cur_ver != version - + 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): diff --git a/rhodecode/templates/codeblocks/diffs.mako b/rhodecode/templates/codeblocks/diffs.mako --- a/rhodecode/templates/codeblocks/diffs.mako +++ b/rhodecode/templates/codeblocks/diffs.mako @@ -225,12 +225,12 @@ return '%s_%s_%i' % (h.md5_safe(commit+f ## file was renamed, or copied %if RENAMED_FILENODE in filediff.patch['stats']['ops']: <% - final_file_name = h.literal(u'{} {}'.format(filediff.target_file_path, filediff.source_file_path)) + final_file_name = h.literal('{} {}'.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'{} {}'.format(filediff.target_file_path, filediff.source_file_path)) + final_file_name = h.literal('{} {}'.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+f >