From f6a6e0caa6de76a3dbfb8ffcbf4bc312c368d3ea Mon Sep 17 00:00:00 2001
From: Pierre-Yves Chibon <pingou@pingoured.fr>
Date: Dec 04 2018 10:01:54 +0000
Subject: Consolidate into a single place the logic to get stats for a given patch


We used to have this logic in three places:
- repo_pull_request.html
- _repo_renderdiff.html
- pagure.api.forks.py

This is now all in pagure.lib.git and exposed in the template via the
``patch_stats`` jinja filter.

Fixes https://pagure.io/pagure/issue/3868

Signed-off-by: Pierre-Yves Chibon <pingou@pingoured.fr>

---

diff --git a/pagure/api/fork.py b/pagure/api/fork.py
index d4466e8..2ad88bc 100644
--- a/pagure/api/fork.py
+++ b/pagure/api/fork.py
@@ -1591,44 +1591,10 @@ def api_pull_request_diffstats(repo, requestid, username=None, namespace=None):
     output = {}
     if diff:
         for patch in diff:
-            linesadded = patch.line_stats[1]
-            linesremoved = patch.line_stats[2]
-            if hasattr(patch, "new_file_path"):
-                # Older pygit2
-                status = patch.status
-                if patch.new_file_path != patch.old_file_path:
-                    status = "R"
-                output[patch.new_file_path] = {
-                    "status": patch.status,
-                    "old_path": patch.old_file_path,
-                    "lines_added": linesadded,
-                    "lines_removed": linesremoved,
-                }
-            elif hasattr(patch, "delta"):
-                # Newer pygit2
-                if (
-                    patch.delta.new_file.mode == 0
-                    and patch.delta.old_file.mode in [33188, 33261]
-                ):
-                    status = "D"
-                elif (
-                    patch.delta.new_file.mode in [33188, 33261]
-                    and patch.delta.old_file.mode == 0
-                ):
-                    status = "A"
-                elif patch.delta.new_file.mode in [
-                    33188,
-                    33261,
-                ] and patch.delta.old_file.mode in [33188, 33261]:
-                    status = "M"
-                if patch.delta.new_file.path != patch.delta.old_file.path:
-                    status = "R"
-                output[patch.delta.new_file.path] = {
-                    "status": status,
-                    "old_path": patch.delta.old_file.path,
-                    "lines_added": linesadded,
-                    "lines_removed": linesremoved,
-                }
+            stats = pagure.lib.git.get_stats_patch(patch)
+            new_path = stats["new_path"]
+            del (stats["new_path"])
+            output[new_path] = stats
     else:
         raise pagure.exceptions.APIError(400, error_code=APIERROR.ENOPRSTATS)
 
diff --git a/pagure/lib/git.py b/pagure/lib/git.py
index 9cf9a3c..d734925 100644
--- a/pagure/lib/git.py
+++ b/pagure/lib/git.py
@@ -2557,3 +2557,78 @@ def create_project_repos(project, region, templ, ignore_existing):
         raise
 
     set_up_project_hooks(project, region)
+
+
+def get_stats_patch(patch):
+    """ Returns some statistics about a given patch.
+
+    These stats include:
+        status: if the file was added (A), deleted (D), modified (M) or
+            renamed (R)
+        old_path: the path to the old file
+        new_path: the path to the new file
+        lines_added: the number of lines added in this patch
+        lines_removed: the number of lines removed in this patch
+
+    All these information are returned in a dict.
+
+    Args:
+        patch (pygit2.Patch): the patch object to get stats on
+    Returns: a dict with the stats described above
+    Raises (pagure.exceptions.PagureException): if for some reason (likely
+        a change in pygit2's API) this function does not manage to gather
+        all the stats it should
+
+    """
+
+    output = {
+        "lines_added": patch.line_stats[1],
+        "lines_removed": patch.line_stats[2],
+        "new_path": None,
+        "old_path": None,
+        "status": None,
+        "new_id": None,
+        "old_id": None,
+    }
+    if hasattr(patch, "new_file_path"):
+        # Older pygit2
+        status = patch.status
+        if patch.new_file_path != patch.old_file_path:
+            status = "R"
+        output["status"] = status
+        output["new_path"] = patch.new_file_path
+        output["old_path"] = patch.old_file_path
+        output["new_id"] = str(patch.new_id)
+        output["old_id"] = str(patch.old_id)
+    elif hasattr(patch, "delta"):
+        # Newer pygit2
+        if patch.delta.new_file.mode == 0 and patch.delta.old_file.mode in [
+            33188,
+            33261,
+        ]:
+            status = "D"
+        elif (
+            patch.delta.new_file.mode in [33188, 33261]
+            and patch.delta.old_file.mode == 0
+        ):
+            status = "A"
+        elif patch.delta.new_file.mode in [
+            33188,
+            33261,
+        ] and patch.delta.old_file.mode in [33188, 33261]:
+            status = "M"
+        if patch.delta.new_file.path != patch.delta.old_file.path:
+            status = "R"
+
+        output["status"] = status
+        output["new_path"] = patch.delta.new_file.path
+        output["new_id"] = str(patch.delta.new_file.id)
+        output["old_path"] = patch.delta.old_file.path
+        output["old_id"] = str(patch.delta.old_file.id)
+
+    if None in output.values():  # pragma: no-cover
+        raise pagure.exceptions.PagureException(
+            "Unable to properly retrieve the stats for this patch"
+        )
+
+    return output
diff --git a/pagure/templates/_repo_renderdiff.html b/pagure/templates/_repo_renderdiff.html
index fbf5e45..e89fe7f 100644
--- a/pagure/templates/_repo_renderdiff.html
+++ b/pagure/templates/_repo_renderdiff.html
@@ -1,268 +1,166 @@
 {% macro repo_renderdiff(diff, diff_commits, pull_request, repo, username, namespace) -%}
 
-{% if diff %}
-      {% for patch in diff %}
-        {% if patch |hasattr('new_id') %}
-          {% set patch_new_id = patch.new_id %}
-        {% elif patch |hasattr('delta') %}
-          {% set patch_new_id = patch.delta.new_file.id %}
-        {% else %}
-          {% set patch_new_id = patch.new_oid %}
-        {% endif %}
-
-        {% if patch |hasattr('old_id') %}
-          {% set patch_old_id = patch.old_id %}
-        {% elif patch |hasattr('delta') %}
-          {% set patch_old_id = patch.delta.old_file.id %}
-        {% else %}
-          {% set patch_old_id = patch.old_oid %}
-        {% endif %}
-
-        {% if patch | hasattr('new_file_path') %}
-          {% set patch_new_file_path = patch.new_file_path -%}
-          {% if patch.new_file_path != patch.old_file_path %}
-            {% set patch_old_file_path = patch.old_file_path %}
-          {%- endif -%}
-        {%- elif patch | hasattr('delta') -%}
-          {% set patch_new_file_path = patch.delta.new_file.path -%}
-          {%- if patch.delta.new_file.path != patch.delta.old_file.path -%}
-            {% set patch_old_file_path = patch.delta.old_file.path %}
-          {%- endif -%}
-        {%- endif -%}
-
+{% macro lineschanged(linesadded, linesremoved) -%}
+  <div class="btn-group">
+    {%if linesadded != 0 %}
+    <span class="btn btn-success btn-sm font-weight-bold disabled opacity-100">+{{linesadded}}</span>
+    {%endif%}
+    {%if linesremoved != 0%}
+    <span class="btn btn-danger btn-sm font-weight-bold disabled opacity-100">-{{linesremoved}}</span>
+    {%endif%}
+  </div>
+{%endmacro%}
+
+{% macro viewfilelink(filepath, patch_new_id) %}
+  {% if pull_request and not pull_request.remote %}
+      <a class="font-weight-bold ml-2" href="{{
+        url_for(
+            'ui_ns.view_file',
+            repo=pull_request.project_from.name,
+            username=pull_request.project_from.user.username
+                if pull_request.project_from.is_fork else None,
+            namespace=pull_request.project_from.namespace,
+            identifier=patch_new_id,
+            filename=filepath) }}"
+        title="View file as of {{ patch_new_id|short }}">{{
+          filepath | unicode }}</a>
+  {% elif not pull_request %}
+      <a class="font-weight-bold ml-2" href="{{
+        url_for(
+            'ui_ns.view_file',
+            repo=repo.name,
+            username=username,
+            namespace=repo.namespace,
+            identifier=patch_new_id,
+            filename=filepath) }}"
+        title="View file as of {{ patch_new_id|short }}">{{
+          filepath | unicode }}</a>
+  {% elif pull_request and pull_request.remote %}
+    {{ filepath | unicode }}
+  {% endif %}
+{% endmacro %}
+
+{% macro viewfilelinkbutton(filepath, patch_new_id, disabled=False) %}
+  {% if pull_request and not pull_request.remote %}
+    <a class="btn btn-outline-primary {{'disabled' if disabled}} btn-sm ml-2" href="{{
+      url_for(
+          'ui_ns.view_file',
+          repo=pull_request.project_from.name,
+          username=pull_request.project_from.user.username
+              if pull_request.project_from.is_fork else None,
+          namespace=pull_request.project_from.namespace,
+          identifier=patch_new_id,
+          filename=filepath) }}"
+      title="View file as of {{ patch_new_id|short }}">
+      <i class="fa fa-file-code-o fa-fw"></i>
+    </a>
+  {% elif not pull_request %}
+      <a class="btn btn-outline-primary {{'disabled' if disabled}} btn-sm ml-2" href="{{
+        url_for(
+            'ui_ns.view_file',
+            repo=repo.name,
+            username=username,
+            namespace=repo.namespace,
+            identifier=patch_new_id,
+            filename=filepath) }}"
+        title="View file as of {{ patch_new_id|short }}">
+        <i class="fa fa-file-code-o fa-fw"></i>
+      </a>
+  {% elif pull_request and pull_request.remote %}
+  {% endif %}
+{% endmacro %}
+
+{% macro changedlabel(thelabel, thecolor)%}
+  <div class="btn btn-outline-{{thecolor}} disabled opacity-100 border-0 font-weight-bold">
+    {{thelabel}}
+  </div>
+{% endmacro %}
+
+{% macro diffcollapsebtn(loop)%}
+  <a href="javascript:void(0)" class="btn btn-sm btn-outline-primary diffhighlightcollapse ml-2" data-toggle="collapse" data-target="#diffhighlight_{{loop.index}}">
+    <i class="fa fa-fw fa-caret-up"></i>
+  </a>
+{% endmacro %}
 
+{% if diff %}
+  {% for patch in diff %}
+    {% set patchstats = (patch | patch_stats) %}
     <section class="commit_diff">
       <div class="card mb-3" id="_{{loop.index}}">
         <div class="card-header">
-            <div class="d-flex align-items-center">
-
-              {% set linesadded = patch.line_stats[1] %}
-              {% set linesremoved = patch.line_stats[2] %}
-
-              {% macro lineschanged(added, removed) -%}
-                <div class="btn-group">
-                  {%if added%}
-                  <span class="btn btn-success btn-sm font-weight-bold disabled opacity-100">+{{linesadded}}</span>
-                  {%endif%}
-                  {%if removed%}
-                  <span class="btn btn-danger btn-sm font-weight-bold disabled opacity-100">-{{linesremoved}}</span>
-                  {%endif%}
-                </div>
-              {%endmacro%}
-
-              {% macro viewfilelink(filepath, identifier=False)%}
-                {% if pull_request and not pull_request.remote %}
-                    {% if not identifier %}
-                      {% set identifier = pull_request.branch_from %}
-                    {% endif %}
-                    <a class="font-weight-bold ml-2" href="{{
-                      url_for(
-                          'ui_ns.view_file',
-                          repo=pull_request.project_from.name,
-                          username=pull_request.project_from.user.username
-                              if pull_request.project_from.is_fork else None,
-                          namespace=pull_request.project_from.namespace,
-                          identifier=identifier,
-                          filename=filepath) }}"
-                      title="View file as of {{ patch_new_id|short }}">{{
-                      filepath | unicode }}</a>
-                {% elif not pull_request %}
-                    <a class="font-weight-bold ml-2" href="{{
-                      url_for(
-                          'ui_ns.view_file',
-                          repo=repo.name,
-                          username=username,
-                          namespace=repo.namespace,
-                          identifier=patch_new_id,
-                          filename=filepath) }}"
-                      title="View file as of {{ patch_new_id|short }}">{{
-                      filepath | unicode }}</a>
-                {% elif pull_request and pull_request.remote %}
-                  {{ filepath | unicode }}
-                {% endif %}
-              {% endmacro %}
-
-              {% macro viewfilelinkbutton(filepath, disabled=False, identifier=False) %}
-              {% if pull_request and not pull_request.remote %}
-                    {% if not identifier %}
-                      {% set identifier = pull_request.branch_from %}
-                    {% endif %}
-                    <a class="btn btn-outline-primary {{'disabled' if disabled}} btn-sm ml-2" href="{{
-                      url_for(
-                          'ui_ns.view_file',
-                          repo=pull_request.project_from.name,
-                          username=pull_request.project_from.user.username
-                              if pull_request.project_from.is_fork else None,
-                          namespace=pull_request.project_from.namespace,
-                          identifier=identifier,
-                          filename=filepath) }}"
-                      title="View file as of {{ patch_new_id|short }}">
-                      <i class="fa fa-file-code-o fa-fw"></i>
-                    </a>
-                {% elif not pull_request %}
-                    <a class="btn btn-outline-primary {{'disabled' if disabled}} btn-sm ml-2" href="{{
-                      url_for(
-                          'ui_ns.view_file',
-                          repo=repo.name,
-                          username=username,
-                          namespace=repo.namespace,
-                          identifier=patch_new_id,
-                          filename=filepath) }}"
-                      title="View file as of {{ patch_new_id|short }}">
-                      <i class="fa fa-file-code-o fa-fw"></i>
-                    </a>
-                {% elif pull_request and pull_request.remote %}
-                {% endif %}
-              {% endmacro %}
-
-              {% macro changedlabel(thelabel, thecolor)%}
-                <div class="btn btn-outline-{{thecolor}} disabled opacity-100 border-0 font-weight-bold">
-                  {{thelabel}}
-                </div>
-              {% endmacro %}
-
-              {% macro diffcollapsebtn()%}
-                <a href="javascript:void(0)" class="btn btn-sm btn-outline-primary diffhighlightcollapse ml-2" data-toggle="collapse" data-target="#diffhighlight_{{loop.index}}">
-                  <i class="fa fa-fw fa-caret-up"></i>
-                </a>
-              {% endmacro %}
-
-              {% if patch | hasattr('new_file_path') %}
-                {%- if patch.new_file_path == patch.old_file_path -%}
-                  {%- if patch.status == 'D' -%}
-                    {% set patchtype = "removed"%}
-                    <div>
-                        {{ viewfilelink(patch.new_file_path) }}
-                    </div>
-                    <div class="d-flex align-items-center ml-auto">
-                      {{ changedlabel("file removed", "danger")}}
-                      {{ lineschanged(False, True) }}
-                      {{ viewfilelinkbutton(patch.new_file_path, disabled=True) }}
-                      {{ diffcollapsebtn() }}
-                    </div>
-                  {%-elif patch.status == 'A' -%}
-                    {% set patchtype = "added"%}
-                    <div>
-                        {{ viewfilelink(patch.new_file_path) }}
-                    </div>
-                    <div class="d-flex align-items-center ml-auto">
-                      {{ changedlabel("file added", "success")}}
-                      {{ lineschanged(True, False) }}
-                      {{ viewfilelinkbutton(patch.new_file_path) }}
-                      {{ diffcollapsebtn() }}
-                    </div>
-                  {%-elif patch.status == 'M' -%}
-                    {% set patchtype = "changed"%}
-                    <div>
-                        {{ viewfilelink(patch.new_file_path) }}
-                    </div>
-                    <div class="d-flex align-items-center ml-auto">
-                      {{ changedlabel("file modified", "secondary")}}
-                      {{ lineschanged(True, True) }}
-                      {{ viewfilelinkbutton(patch.new_file_path) }}
-                      {{ diffcollapsebtn() }}
-                    </div>
-                  {%-endif-%}
-                {%- else -%}
-                  {% set patchtype = "moved"%}
-                  <div>
-                      {{ viewfilelink(patch.new_file_path) }}<strike>{{patch.old_file_path}}</strike>
-                  </div>
-                  <div class="d-flex align-items-center ml-auto">
-                    {{ changedlabel("file renamed", "info")}}
-                    {% if linesadded != 0 and linesremoved != 0%}
-                      {{ lineschanged(True, True) }}
-                    {% endif %}
-                    {{ viewfilelinkbutton(patch.new_file_path) }}
-                    {{ diffcollapsebtn() }}
-                  </div>
-                {%- endif -%}
-              {%- elif patch | hasattr('delta') -%}
-                {%- if patch.delta.new_file.path == patch.delta.old_file.path -%}
-                  {%- if patch.delta.new_file.mode == 0
-                      and patch.delta.old_file.mode in [33188, 33261] -%}
-                    {% set patchtype = "removed"%}
-                    <div>
-                        {{ viewfilelink(patch.delta.new_file.path) }}
-                    </div>
-                    <div class="d-flex align-items-center ml-auto">
-                      {{ changedlabel("file removed", "danger")}}
-                      {{ lineschanged(False, True) }}
-                      {{ viewfilelinkbutton(patch.delta.new_file.path, disabled=True) }}
-                      {{ diffcollapsebtn() }}
-                    </div>
-                  {%-elif patch.delta.new_file.mode in [33188, 33261]
-                      and patch.delta.old_file.mode == 0 -%}
-                    {% set patchtype = "added"%}
-                    <div>
-                        {{ viewfilelink(patch.delta.new_file.path) }}
-                    </div>
-                    <div class="d-flex align-items-center ml-auto">
-                      {{ changedlabel("file added", "success")}}
-                      {{ lineschanged(True, False) }}
-                      {{ viewfilelinkbutton(patch.delta.new_file.path) }}
-                      {{ diffcollapsebtn() }}
-                    </div>
-                  {%-elif patch.delta.new_file.mode in [33188, 33261]
-                      and patch.delta.old_file.mode in [33188, 33261] -%}
-                    {% set patchtype = "changed"%}
-                    <div>
-                        {{ viewfilelink(patch.delta.new_file.path) }}
-                    </div>
-                    <div class="d-flex align-items-center ml-auto">
-                      {{ changedlabel("file modified", "secondary")}}
-                      {{ lineschanged(True, True) }}
-                      {{ viewfilelinkbutton(patch.delta.new_file.path) }}
-                      {{ diffcollapsebtn() }}
-                    </div>
-                  {%-endif-%}
-
-                {%- else -%}
-                  {% set patchtype = "moved"%}
-                  <div>
-                      {{ viewfilelink(patch.delta.new_file.path) }}<strike>{{patch.delta.old_file.path}}</strike>
-                  </div>
-                  <div class="d-flex align-items-center ml-auto">
-                    {{ changedlabel("file renamed", "info")}}
-                    {% if linesadded != 0 and linesremoved != 0%}
-                      {{ lineschanged(True, True) }}
-                    {% endif %}
-                    {{ viewfilelinkbutton(patch.delta.new_file.path) }}
-                    {{ diffcollapsebtn() }}
-                  </div>
-                {%- endif -%}
-              {%- endif -%}
+          <div class="d-flex align-items-center">
+            {%- if patchstats["status"] == 'D' -%}
+              <div>
+                  {{ viewfilelink(patchstats["new_path"], patchstats["new_id"]) }}
+              </div>
+              <div class="d-flex align-items-center ml-auto">
+                {{ changedlabel("file removed", "danger")}}
+                {{ lineschanged(patchstats["lines_added"], patchstats["lines_removed"]) }}
+                {{ viewfilelinkbutton(patchstats["new_path"], patchstats["new_id"], disabled=True) }}
+                {{ diffcollapsebtn(loop) }}
+              </div>
+            {%-elif patchstats["status"] == 'A' -%}
+              <div>
+                  {{ viewfilelink(patchstats["new_path"], patchstats["new_id"]) }}
+              </div>
+              <div class="d-flex align-items-center ml-auto">
+                {{ changedlabel("file added", "success")}}
+                {{ lineschanged(patchstats["lines_added"], patchstats["lines_removed"]) }}
+                {{ viewfilelinkbutton(patchstats["new_path"], patchstats["new_id"]) }}
+                {{ diffcollapsebtn(loop) }}
+              </div>
+            {%-elif patchstats["status"] == 'M' -%}
+              <div>
+                  {{ viewfilelink(patchstats["new_path"], patchstats["new_id"]) }}
+              </div>
+              <div class="d-flex align-items-center ml-auto">
+                {{ changedlabel("file modified", "secondary")}}
+                {{ lineschanged(patchstats["lines_added"], patchstats["lines_removed"]) }}
+                {{ viewfilelinkbutton(patchstats["new_path"], patchstats["new_id"]) }}
+                {{ diffcollapsebtn(loop) }}
+              </div>
+            {%- else -%}
+              <div>
+                {{ viewfilelink(patchstats["new_path"], patchstats["new_id"]) }}<strike>{{patchstats["old_path"]}}</strike>
+              </div>
+              <div class="d-flex align-items-center ml-auto">
+                {{ changedlabel("file renamed", "info")}}
+                {{ lineschanged(patchstats["lines_added"], patchstats["lines_removed"]) }}
+                {{ viewfilelinkbutton(patchstats["new_path"], patchstats["new_id"]) }}
+                {{ diffcollapsebtn(loop) }}
+              </div>
+            {%- endif -%}
           </div>
         </div>
-      {% if patchtype == "moved" and linesadded == 0 and linesremoved == 0%}
+      {% if patchstats["status"] == "R" and patchstats["lines_added"] == 0 and patchstats["lines_removed"] == 0%}
         <div class="card-body collapse show" id="diffhighlight_{{loop.index}}">
           <div class="text-muted text-center">file was moved with no change to the file</div>
         </div>
-      {% elif patchtype == "added" and linesadded == 0 %}
+      {% elif patchstats["status"] == "A" and patchstats["lines_added"] == 0 %}
         <div class="card-body collapse show" id="diffhighlight_{{loop.index}}">
           <div class="text-muted text-xs-center">empty file added</div>
         </div>
       {% else %}
-        {% if patchtype == "added" and linesadded > 1000 %}
+        {% if patchstats["status"] == "A" and patchstats["lines_added"] > 1000 %}
           <div class="card-body collapse show" id="diffhighlight_{{loop.index}}">
             <div class="text-muted text-center">
                 The added file is too large to be shown here, see it at:
-                {{ viewfilelink(patch_new_file_path) }}
+                {{ viewfilelink(patchstats["new_path"], patchstats["new_id"]) }}
             </div>
           </div>
-        {% elif patchtype == "removed" and linesadded > 1000 %}
+        {% elif patchstats["status"] == "D" and patchstats["lines_added"] > 1000 %}
           <div class="card-body collapse show" id="diffhighlight_{{loop.index}}">
             <div class="text-muted text-center">
                 The removed file is too large to be shown here, see it at:
-                {{ viewfilelink(patch_new_file_path, patch_old_id) }}
+                {{ viewfilelink(patchstats["new_path"], patchstats["old_id"]) }}
             </div>
           </div>
         {% else %}
           <div class="card-body p-0 collapse show" id="diffhighlight_{{loop.index}}">
           {% autoescape false %}
               {{ patch | patch_to_diff | format_loc(
-                      filename=patch_new_file_path,
-                      commit=patch_new_id,
+                      filename=patchstats["new_path"],
+                      commit=patchstats["new_id"],
                       prequest=pull_request,
                       index=loop.index,
                       isprdiff=True,
@@ -275,7 +173,6 @@
       </div>
       </section>
       {% endfor %}
-      {% endif %}
+  {% endif %}
 
 {%- endmacro %}
-
diff --git a/pagure/templates/repo_pull_request.html b/pagure/templates/repo_pull_request.html
index ac0afcb..bca3ccc 100644
--- a/pagure/templates/repo_pull_request.html
+++ b/pagure/templates/repo_pull_request.html
@@ -684,36 +684,16 @@
     {%- endmacro %}
     <div class="list-group list-group-flush">
     {% for patch in diff %}
-          {% set linesadded = patch.line_stats[1] %}
-          {% set linesremoved = patch.line_stats[2] %}
-          {% if patch | hasattr('new_file_path') %}
-            {%- if patch.new_file_path == patch.old_file_path -%}
-              {%- if patch.status == 'D' -%}
-                {{ changesdeletedfile(patch.new_file_path, linesadded, linesremoved, loop.index) }}
-              {%-elif patch.status == 'A' -%}
-                {{ changesaddedfile(patch.new_file_path, linesadded, linesremoved, loop.index) }}
-              {%-elif patch.status == 'M' -%}
-                {{ changeschangedfile(patch.new_file_path, linesadded, linesremoved, loop.index) }}
-              {%-endif-%}
-            {%- else -%}
-              {{changesrenamedfile(patch.old_file_path, patch.new_file_path, linesadded, linesremoved, loop.index)}}
-            {%- endif -%}
-          {%- elif patch | hasattr('delta') -%}
-            {%- if patch.delta.new_file.path == patch.delta.old_file.path -%}
-              {%- if patch.delta.new_file.mode == 0
-                  and patch.delta.old_file.mode in [33188, 33261] -%}
-                {{ changesdeletedfile(patch.delta.new_file.path, linesadded, linesremoved, loop.index) }}
-              {%-elif patch.delta.new_file.mode in [33188, 33261]
-                   and patch.delta.old_file.mode == 0 -%}
-                {{ changesaddedfile(patch.delta.new_file.path, linesadded, linesremoved, loop.index) }}
-              {%-elif patch.delta.new_file.mode in [33188, 33261]
-                   and patch.delta.old_file.mode in [33188, 33261] -%}
-                {{ changeschangedfile(patch.delta.new_file.path, linesadded, linesremoved, loop.index) }}
-              {%-endif-%}
-            {%- else -%}
-              {{changesrenamedfile(patch.delta.old_file.path, patch.delta.new_file.path, linesadded, linesremoved, loop.index)}}
-            {%- endif -%}
-          {%- endif -%}
+      {% set patchstats = (patch | patch_stats) %}
+      {%- if patchstats["status"] == 'D' -%}
+        {{ changesdeletedfile(patchstats["new_path"], patchstats["lines_added"], patchstats["lines_removed"], loop.index) }}
+      {%-elif patchstats["status"] == 'A' -%}
+        {{ changesaddedfile(patchstats["new_path"], patchstats["lines_added"], patchstats["lines_removed"], loop.index) }}
+      {%-elif patchstats["status"] == 'M' -%}
+        {{ changeschangedfile(patchstats["new_path"], patchstats["lines_added"], patchstats["lines_removed"], loop.index) }}
+      {%- else -%}
+        {{changesrenamedfile(patchstats["old_path"], patchstats["new_path"], patchstats["lines_added"], patchstats["lines_removed"], loop.index) }}
+      {%-endif-%}
     {% endfor %}
       </div>
     </div>
diff --git a/pagure/ui/filters.py b/pagure/ui/filters.py
index e860f55..f7cb6c1 100644
--- a/pagure/ui/filters.py
+++ b/pagure/ui/filters.py
@@ -16,6 +16,7 @@
 from __future__ import unicode_literals
 
 import textwrap
+import logging
 
 import arrow
 import flask
@@ -33,6 +34,7 @@ from pagure.ui import UI_NS
 from pagure.utils import authenticated, is_repo_committer, is_true
 
 
+_log = logging.getLogger(__name__)
 # Jinja filters
 
 
@@ -103,12 +105,16 @@ def format_loc(
 
     output = ['<div class="highlight">', '<table class="code_table">']
 
+    commit_hash = commit
+    if hasattr(commit_hash, "hex"):
+        commit_hash = commit_hash.hex
+
     comments = {}
     if prequest and not isinstance(prequest, flask.wrappers.Request):
         for com in prequest.comments:
             if (
                 commit
-                and com.commit_id == commit.hex
+                and com.commit_id == commit_hash
                 and com.filename == filename
             ):
                 if com.line in comments:
@@ -801,3 +807,14 @@ def get_git_url_ssh(complement=""):
         except (KeyError, IndexError):
             pass
     return git_url_ssh + complement
+
+
+@UI_NS.app_template_filter("patch_stats")
+def get_patch_stats(patch):
+    """ Return a dict of stats about the provided patch."""
+    try:
+        output = pagure.lib.git.get_stats_patch(patch)
+    except pagure.exceptions.PagureException:
+        _log.exception("Failed to get stats on a patch")
+        output = {}
+    return output
diff --git a/tests/test_pagure_flask_api_fork.py b/tests/test_pagure_flask_api_fork.py
index d68236c..59cc74b 100644
--- a/tests/test_pagure_flask_api_fork.py
+++ b/tests/test_pagure_flask_api_fork.py
@@ -2781,6 +2781,8 @@ class PagureFlaskApiForkPRDiffStatstests(tests.Modeltests):
                 'sources': {
                     'lines_added': 10,
                     'lines_removed': 0,
+                    "new_id": "540916fbd3d825d14cc0c0b2397606fda69379ce",
+                    "old_id": "265f133a7c94ede4cb183dd808219c5bf9e08f87",
                     'old_path': 'sources',
                     'status': 'M'
                 }
@@ -2810,12 +2812,16 @@ class PagureFlaskApiForkPRDiffStatstests(tests.Modeltests):
                   "README.md": {
                     "lines_added": 5,
                     "lines_removed": 0,
+                    "new_id": "bd913ea153650b94f33f53e5164c36a28b761bf4",
+                    "old_id": "0000000000000000000000000000000000000000",
                     "old_path": "README.md",
                     "status": "A"
                   },
                   "sources": {
                     "lines_added": 5,
                     "lines_removed": 0,
+                    "new_id": "540916fbd3d825d14cc0c0b2397606fda69379ce",
+                    "old_id": "293500070b9dfc6ab66e31383f8f7fccf6a95fe2",
                     "old_path": "sources",
                     "status": "M"
                   }
@@ -2824,12 +2830,16 @@ class PagureFlaskApiForkPRDiffStatstests(tests.Modeltests):
                   "README.md": {
                     "lines_added": 5,
                     "lines_removed": 0,
+                    "new_id": "bd913ea153650b94f33f53e5164c36a28b761bf4",
+                    "old_id": "0000000000000000000000000000000000000000",
                     "old_path": "README.md",
                     "status": "A"
                   },
                   "sources": {
                     "lines_added": 10,
                     "lines_removed": 0,
+                    "new_id": "540916fbd3d825d14cc0c0b2397606fda69379ce",
+                    "old_id": "265f133a7c94ede4cb183dd808219c5bf9e08f87",
                     "old_path": "sources",
                     "status": "M"
                   }
@@ -2865,12 +2875,16 @@ class PagureFlaskApiForkPRDiffStatstests(tests.Modeltests):
               "README.md": {
                 "lines_added": 5,
                 "lines_removed": 0,
+                "new_id": "bd913ea153650b94f33f53e5164c36a28b761bf4",
+                "old_id": "0000000000000000000000000000000000000000",
                 "old_path": "README.md",
                 "status": "A"
               },
               "sources": {
                 "lines_added": 0,
                 "lines_removed": 5,
+                "new_id": "0000000000000000000000000000000000000000",
+                "old_id": "265f133a7c94ede4cb183dd808219c5bf9e08f87",
                 "old_path": "sources",
                 "status": "D"
               }
diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py
index 7533572..6f5e935 100644
--- a/tests/test_pagure_flask_ui_repo.py
+++ b/tests/test_pagure_flask_ui_repo.py
@@ -2138,7 +2138,7 @@ class PagureFlaskRepotests(tests.Modeltests):
             )
             self.assertIn(
                 '<div class="btn btn-outline-success disabled opacity-100 border-0 font-weight-bold">\n'
-                '                  file added\n', output_text)
+                '    file added\n', output_text)
 
             # View inverse commits comparison
             output = self.app.get(
@@ -2164,7 +2164,7 @@ class PagureFlaskRepotests(tests.Modeltests):
             )
             self.assertIn(
                 '<div class="btn btn-outline-danger disabled opacity-100 border-0 font-weight-bold">\n'
-                '                  file removed\n', output_text)
+                '    file removed\n', output_text)
 
         output = self.app.get('/foo/bar')
         # No project registered in the DB
diff --git a/tests/test_pagure_flask_ui_slash_branch_name.py b/tests/test_pagure_flask_ui_slash_branch_name.py
index f3b7774..ae3675c 100644
--- a/tests/test_pagure_flask_ui_slash_branch_name.py
+++ b/tests/test_pagure_flask_ui_slash_branch_name.py
@@ -345,7 +345,7 @@ class PagureFlaskSlashInBranchtests(tests.SimplePagureTest):
             '+1</span>\n', output_text)
         self.assertIn(
             '<div class="btn btn-outline-success disabled opacity-100 border-0 font-weight-bold">\n'
-            '                  file added\n', output_text)
+            '    file added\n', output_text)
 
         user = tests.FakeUser()
         with tests.user_set(self.app.application, user):
@@ -359,7 +359,7 @@ class PagureFlaskSlashInBranchtests(tests.SimplePagureTest):
                 '+1</span>\n', output_text)
             self.assertIn(
                 '<div class="btn btn-outline-success disabled opacity-100 border-0 font-weight-bold">\n'
-                '                  file added\n', output_text)
+                '    file added\n', output_text)
 
 
 if __name__ == '__main__':