fix(pull-requests): fixes for rendering comments

rhodecode/apps/repository/views/repo_pull_requests.py

@@ -357,7 +357,7 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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 b' 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

rhodecode/model/db.py

@@ -65,7 +65,7 b' 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 b' 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 b' 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 b' 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 b' 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):

rhodecode/templates/codeblocks/diffs.mako

@@ -225,12 +225,12 b" 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'{} <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 b" return '%s_%s_%i' % (h.md5_safe(commit+f"
         >
         <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 b" return '%s_%s_%i' % (h.md5_safe(commit+f"
                         <span class="pull-right icon-clipboard clipboard-action" data-clipboard-text="${permalink}" title="Copy permalink"></span>
                    </div>
 
-
                   </details-menu>
                 </details>
 
@@ -667,8 +666,8 b" return '%s_%s_%i' % (h.md5_safe(commit+f"
 <%!
 
 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 b' def get_inline_comments(comments, filena'
     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 b' def get_comments_for(diff_type, comments'
     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