From 38f510105b1d88e4daa849ce058acad94d13df8b Mon Sep 17 00:00:00 2001
From: Oliver Gutierrez <ogutierrez@redhat.com>
Date: Jun 19 2016 11:30:35 +0000
Subject: [PATCH 1/2] Added commit comparison view


---

diff --git a/nosetests b/nosetests
index dbf9cf3..2f5271b 100755
--- a/nosetests
+++ b/nosetests
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/env python
 # EASY-INSTALL-ENTRY-SCRIPT: 'nose==0.10.4','console_scripts','nosetests'
 __requires__ = ['nose>=0.10.4', 'SQLAlchemy >= 0.7', 'jinja2 >= 2.4']
 import sys
diff --git a/pagure/templates/pull_request.html b/pagure/templates/pull_request.html
index af3837b..16ee4c4 100644
--- a/pagure/templates/pull_request.html
+++ b/pagure/templates/pull_request.html
@@ -107,6 +107,11 @@
       (<a href="{{ url_for('view_tree', username=username,
                repo=repo.name, identifier=commitid) }}"
       >tree</a>)</h2>
+
+    {% if origin == 'compare_commits' %}
+      <h5 class="text-muted">{{ commit1 }} .. {{ commit2 }}</h5>
+    {% endif %}
+
 {% endif %}
 
 {% if form and (repo_admin or remote_git) %}
diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py
index c668258..152b170 100644
--- a/pagure/ui/repo.py
+++ b/pagure/ui/repo.py
@@ -379,6 +379,57 @@ def view_commits(repo, branchname=None, username=None):
     )
 
 
+@APP.route('/<repo>/c/<commit1>..<commit2>/')
+@APP.route('/<repo>/c/<commit1>..<commit2>')
+@APP.route('/fork/<username>/<repo>/c/<commit1>..<commit2>/')
+@APP.route('/fork/<username>/<repo>/c/<commit1>..<commit2>')
+def compare_commits(repo, commit1, commit2, username=None):
+    """ Compares two commits for specified repo
+    """
+    repo = pagure.lib.get_project(SESSION, repo, user=username)
+
+    if not repo:
+        flask.abort(404, 'Project not found')
+
+    reponame = pagure.get_repo_path(repo)
+
+    repo_obj = pygit2.Repository(reponame)
+
+    if not repo_obj.is_empty and not repo_obj.head_is_unborn:
+        head = repo_obj.head.shorthand
+    else:
+        head = None
+
+    # Check commit1 and commit2 existence
+    commit1_obj = repo_obj.get(commit1)
+    commit2_obj = repo_obj.get(commit2)
+    if commit1_obj is None:
+        flask.abort(404, 'First commit does not exist')
+    if commit2_obj is None:
+        flask.abort(404, 'Second commit does not exist')
+
+    # Get commit1 and commit2 diff data
+    diff_commits = [commit1_obj, commit2_obj]
+    diff = repo_obj.diff(commit1, commit2)
+
+    return flask.render_template(
+        'pull_request.html',
+        select='logs',
+        origin='compare_commits',
+        repo=repo,
+        username=username,
+        head=head,
+        commit1=commit1,
+        commit2=commit2,
+        commit1_obj=commit1_obj,
+        commit2_obj=commit2_obj,
+        diff=diff,
+        diff_commits=diff_commits,
+        branches=sorted(repo_obj.listall_branches()),
+        repo_admin=is_repo_admin(repo),
+    )
+
+
 @APP.route('/<repo:repo>/blob/<path:identifier>/f/<path:filename>')
 @APP.route('/fork/<username>/<repo:repo>/blob/<path:identifier>/f/<path:filename>')
 def view_file(repo, identifier, filename, username=None):
diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py
index 3e49701..c2ffc34 100644
--- a/tests/test_pagure_flask_ui_repo.py
+++ b/tests/test_pagure_flask_ui_repo.py
@@ -1125,6 +1125,90 @@ class PagureFlaskRepotests(tests.Modeltests):
             'test project #3        </div>', output.data)
         self.assertIn('Forked from', output.data)
 
+    def test_compare_commits(self):
+        """ Test the compare_commits endpoint. """
+        output = self.app.get('/foo/bar')
+        # No project registered in the DB
+        self.assertEqual(output.status_code, 404)
+
+        tests.create_projects(self.session)
+
+        output = self.app.get('/test/bar')
+        # No git repo associated
+        self.assertEqual(output.status_code, 404)
+
+        tests.create_projects_git(tests.HERE, bare=True)
+
+        output = self.app.get('/test/bar')
+        self.assertEqual(output.status_code, 404)
+
+        # Add a README to the git repo - First commit
+        tests.add_readme_git_repo(os.path.join(tests.HERE, 'test.git'))
+        repo = pygit2.Repository(os.path.join(tests.HERE, 'test.git'))
+        c1 = repo.revparse_single('HEAD')
+
+        # Add some content to the git repo
+        tests.add_content_git_repo(os.path.join(tests.HERE, 'test.git'))
+
+        repo = pygit2.Repository(os.path.join(tests.HERE, 'test.git'))
+        c2 = repo.revparse_single('HEAD')
+
+        # View commits comparison
+        output = self.app.get('/test/c/%s..%s' % (c1.oid.hex, c2.oid.hex))
+        self.assertEqual(output.status_code, 200)
+        self.assertIn(
+            '<h5 class="text-muted">%s .. %s</h5>' % (c1.oid.hex, c2.oid.hex),
+            output.data)
+        self.assertIn(
+            '<span style="color: #800080; font-weight: bold">@@ -0,0 +1,3 @@</span>',
+            output.data)
+        self.assertIn('<span style="color: #00A000">+ foo</span>', output.data)
+        self.assertIn('<span style="color: #00A000">+  bar</span>', output.data)
+        self.assertIn('<span style="color: #00A000">+ baz </span>', output.data)
+
+        # View inverse commits comparison
+        output = self.app.get(
+            '/test/c/%s..%s' % (c2.oid.hex, c1.oid.hex))
+        self.assertIn(
+            '<h5 class="text-muted">%s .. %s</h5>' % (c2.oid.hex, c1.oid.hex),
+            output.data)
+        self.assertIn(
+            '<span style="color: #800080; font-weight: bold">@@ -1,3 +0,0 @@</span>',
+            output.data)
+        self.assertIn('<span style="color: #a40000">- foo</span>', output.data)
+        self.assertIn('<span style="color: #a40000">-  bar</span>', output.data)
+        self.assertIn('<span style="color: #a40000">- baz </span>', output.data)
+
+        user = tests.FakeUser()
+
+        # Set user logged in
+        with tests.user_set(pagure.APP, user):
+            # View commits comparison
+            output = self.app.get('/test/c/%s..%s' % (c1.oid.hex, c2.oid.hex))
+            self.assertEqual(output.status_code, 200)
+            self.assertIn(
+                '<h5 class="text-muted">%s .. %s</h5>' % (c1.oid.hex, c2.oid.hex),
+                output.data)
+            self.assertIn(
+                '<span style="color: #800080; font-weight: bold">@@ -0,0 +1,3 @@</span>',
+                output.data)
+            self.assertIn('<span style="color: #00A000">+ foo</span>', output.data)
+            self.assertIn('<span style="color: #00A000">+  bar</span>', output.data)
+            self.assertIn('<span style="color: #00A000">+ baz </span>', output.data)
+
+            # View inverse commits comparison
+            output = self.app.get(
+                '/test/c/%s..%s' % (c2.oid.hex, c1.oid.hex))
+            self.assertIn(
+                '<h5 class="text-muted">%s .. %s</h5>' % (c2.oid.hex, c1.oid.hex),
+                output.data)
+            self.assertIn(
+                '<span style="color: #800080; font-weight: bold">@@ -1,3 +0,0 @@</span>',
+                output.data)
+            self.assertIn('<span style="color: #a40000">- foo</span>', output.data)
+            self.assertIn('<span style="color: #a40000">-  bar</span>', output.data)
+            self.assertIn('<span style="color: #a40000">- baz </span>', output.data)
+
     def test_view_file(self):
         """ Test the view_file endpoint. """
         output = self.app.get('/foo/blob/foo/f/sources')

From 9e99d9cd93393107a967ddc5fe1a6ceb0eeaf7d7 Mon Sep 17 00:00:00 2001
From: Oliver Gutierrez <ogutierrez@redhat.com>
Date: Jun 19 2016 11:30:42 +0000
Subject: [PATCH 2/2] Added full list of commits to commit comparison view


---

diff --git a/pagure/templates/pull_request.html b/pagure/templates/pull_request.html
index 16ee4c4..e8ca6a0 100644
--- a/pagure/templates/pull_request.html
+++ b/pagure/templates/pull_request.html
@@ -6,6 +6,8 @@
     PR#{{ requestid }}: {{ pull_request.title | noJS(ignore="img")}} - {{ repo.name }}
   {% elif form and (repo_admin or remote_git) %}
     Create new Pull Request for {{ branch_to }} - {{ repo.name }}
+  {%- elif origin == 'compare_commits' -%}
+    Diff from {{ commit1 }} to {{ commit2 }} - {{ repo.name }}
   {%- else -%}
     Diff from {{ branch_from }} to {{ branch_to }} - {{ repo.name }}
   {%- endif -%}
diff --git a/pagure/ui/repo.py b/pagure/ui/repo.py
index 152b170..eadb3b4 100644
--- a/pagure/ui/repo.py
+++ b/pagure/ui/repo.py
@@ -406,12 +406,34 @@ def compare_commits(repo, commit1, commit2, username=None):
     if commit1_obj is None:
         flask.abort(404, 'First commit does not exist')
     if commit2_obj is None:
-        flask.abort(404, 'Second commit does not exist')
+        flask.abort(404, 'Last commit does not exist')
 
-    # Get commit1 and commit2 diff data
-    diff_commits = [commit1_obj, commit2_obj]
+    # Get commits diff data
     diff = repo_obj.diff(commit1, commit2)
 
+    # Get commits list
+    diff_commits = []
+    order = pygit2.GIT_SORT_TIME
+    first_commit = commit1
+    last_commit = commit2
+
+    commits = map(lambda x: x.oid.hex[:len(first_commit)],
+                  repo_obj.walk(last_commit, pygit2.GIT_SORT_TIME))
+
+    if first_commit not in commits:
+        first_commit = commit2
+        last_commit = commit1
+
+    for commit in repo_obj.walk(last_commit, order):
+        diff_commits.append(commit)
+
+        if commit.oid.hex == first_commit \
+                or commit.oid.hex.startswith(first_commit):
+            break
+
+    if first_commit == commit2:
+        diff_commits.reverse()
+
     return flask.render_template(
         'pull_request.html',
         select='logs',
@@ -421,8 +443,6 @@ def compare_commits(repo, commit1, commit2, username=None):
         head=head,
         commit1=commit1,
         commit2=commit2,
-        commit1_obj=commit1_obj,
-        commit2_obj=commit2_obj,
         diff=diff,
         diff_commits=diff_commits,
         branches=sorted(repo_obj.listall_branches()),
diff --git a/tests/test_pagure_flask_ui_repo.py b/tests/test_pagure_flask_ui_repo.py
index c2ffc34..d674d59 100644
--- a/tests/test_pagure_flask_ui_repo.py
+++ b/tests/test_pagure_flask_ui_repo.py
@@ -1127,6 +1127,111 @@ class PagureFlaskRepotests(tests.Modeltests):
 
     def test_compare_commits(self):
         """ Test the compare_commits endpoint. """
+
+        # First two commits comparison
+        def compare_first_two(c1, c2):
+            # View commits comparison
+            output = self.app.get('/test/c/%s..%s' % (c2.oid.hex, c1.oid.hex))
+            self.assertEqual(output.status_code, 200)
+            self.assertIn(
+                '<title>Diff from %s to %s - test - Pagure</title>'
+                % (c2.oid.hex, c1.oid.hex),
+                output.data)
+            self.assertIn(
+                '<h5 class="text-muted">%s .. %s</h5>'
+                % (c2.oid.hex, c1.oid.hex),
+                output.data)
+            self.assertIn(
+                '<span class="hidden-sm-down">Commits&nbsp;</span>\n      ' +
+                '<span ' +
+                'class="label label-default label-pill hidden-sm-down">' +
+                '\n        2\n      </span>',
+                output.data)
+            self.assertIn(
+                '<span style="color: #800080; font-weight: bold">' +
+                '@@ -1,2 +1,1 @@</span>',
+                output.data)
+            self.assertIn(
+                '<span style="color: #a40000">- Row 0</span>', output.data)
+            # View inverse commits comparison
+            output = self.app.get('/test/c/%s..%s' % (c1.oid.hex, c2.oid.hex))
+            self.assertEqual(output.status_code, 200)
+            self.assertIn(
+                '<title>Diff from %s to %s - test - Pagure</title>' %
+                (c1.oid.hex, c2.oid.hex),
+                output.data)
+            self.assertIn(
+                '<span class="hidden-sm-down">Commits&nbsp;</span>\n      ' +
+                '<span ' +
+                'class="label label-default label-pill hidden-sm-down">' +
+                '\n        2\n      </span>',
+                output.data)
+            self.assertIn(
+                '<h5 class="text-muted">%s .. %s</h5>' %
+                (c1.oid.hex, c2.oid.hex),
+                output.data)
+            self.assertIn(
+                '<span style="color: #800080; font-weight: bold">' +
+                '@@ -1,1 +1,2 @@</span>',
+                output.data)
+            self.assertIn(
+                '<span style="color: #00A000">+ Row 0</span>',
+                output.data)
+
+        def compare_all(c1, c3):
+            # View commits comparison
+            output = self.app.get('/test/c/%s..%s' % (c1.oid.hex, c3.oid.hex))
+            self.assertEqual(output.status_code, 200)
+            self.assertIn(
+                '<title>Diff from %s to %s - test - Pagure</title>' %
+                (c1.oid.hex, c3.oid.hex), output.data)
+            self.assertIn(
+                '<h5 class="text-muted">%s .. %s</h5>' %
+                (c1.oid.hex, c3.oid.hex),
+                output.data)
+            self.assertIn(
+                '<span style="color: #800080; font-weight: bold">' +
+                '@@ -1,1 +1,3 @@</span>',
+                output.data)
+            self.assertIn(
+                '<span style="color: #00A000">+ Row 0</span>', output.data)
+            self.assertEqual(
+                output.data.count(
+                    '<span style="color: #00A000">+ Row 0</span>'), 2)
+            self.assertIn(
+                '<span class="hidden-sm-down">Commits&nbsp;</span>\n      ' +
+                '<span ' +
+                'class="label label-default label-pill hidden-sm-down">' +
+                '\n        3\n      </span>',
+                output.data)
+            # View inverse commits comparison
+            output = self.app.get(
+                '/test/c/%s..%s' % (c3.oid.hex, c1.oid.hex))
+            self.assertEqual(output.status_code, 200)
+            self.assertIn(
+                '<title>Diff from %s to %s - test - Pagure</title>' %
+                (c3.oid.hex, c1.oid.hex), output.data)
+            self.assertIn(
+                '<h5 class="text-muted">%s .. %s</h5>' %
+                (c3.oid.hex, c1.oid.hex),
+                output.data)
+            self.assertIn(
+                '<span style="color: #800080; font-weight: bold">' +
+                '@@ -1,3 +1,1 @@</span>',
+                output.data)
+            self.assertIn(
+                '<span style="color: #a40000">- Row 0</span>',
+                output.data)
+            self.assertEqual(
+                output.data.count(
+                    '<span style="color: #a40000">- Row 0</span>'), 2)
+            self.assertIn(
+                '<span class="hidden-sm-down">Commits&nbsp;</span>\n      ' +
+                '<span ' +
+                'class="label label-default label-pill hidden-sm-down">' +
+                '\n        3\n      </span>',
+                output.data)
+
         output = self.app.get('/foo/bar')
         # No project registered in the DB
         self.assertEqual(output.status_code, 404)
@@ -1142,72 +1247,31 @@ class PagureFlaskRepotests(tests.Modeltests):
         output = self.app.get('/test/bar')
         self.assertEqual(output.status_code, 404)
 
-        # Add a README to the git repo - First commit
-        tests.add_readme_git_repo(os.path.join(tests.HERE, 'test.git'))
         repo = pygit2.Repository(os.path.join(tests.HERE, 'test.git'))
-        c1 = repo.revparse_single('HEAD')
 
-        # Add some content to the git repo
-        tests.add_content_git_repo(os.path.join(tests.HERE, 'test.git'))
+        # Add one commit to git repo
+        tests.add_commit_git_repo(
+            os.path.join(tests.HERE, 'test.git'), ncommits=1)
+        c1 = repo.revparse_single('HEAD')
 
-        repo = pygit2.Repository(os.path.join(tests.HERE, 'test.git'))
+        # Add another commit to git repo
+        tests.add_commit_git_repo(
+            os.path.join(tests.HERE, 'test.git'), ncommits=1)
         c2 = repo.revparse_single('HEAD')
 
-        # View commits comparison
-        output = self.app.get('/test/c/%s..%s' % (c1.oid.hex, c2.oid.hex))
-        self.assertEqual(output.status_code, 200)
-        self.assertIn(
-            '<h5 class="text-muted">%s .. %s</h5>' % (c1.oid.hex, c2.oid.hex),
-            output.data)
-        self.assertIn(
-            '<span style="color: #800080; font-weight: bold">@@ -0,0 +1,3 @@</span>',
-            output.data)
-        self.assertIn('<span style="color: #00A000">+ foo</span>', output.data)
-        self.assertIn('<span style="color: #00A000">+  bar</span>', output.data)
-        self.assertIn('<span style="color: #00A000">+ baz </span>', output.data)
+        # Add one more commit to git repo
+        tests.add_commit_git_repo(
+            os.path.join(tests.HERE, 'test.git'), ncommits=1)
+        c3 = repo.revparse_single('HEAD')
 
-        # View inverse commits comparison
-        output = self.app.get(
-            '/test/c/%s..%s' % (c2.oid.hex, c1.oid.hex))
-        self.assertIn(
-            '<h5 class="text-muted">%s .. %s</h5>' % (c2.oid.hex, c1.oid.hex),
-            output.data)
-        self.assertIn(
-            '<span style="color: #800080; font-weight: bold">@@ -1,3 +0,0 @@</span>',
-            output.data)
-        self.assertIn('<span style="color: #a40000">- foo</span>', output.data)
-        self.assertIn('<span style="color: #a40000">-  bar</span>', output.data)
-        self.assertIn('<span style="color: #a40000">- baz </span>', output.data)
+        compare_first_two(c1, c2)
+        compare_all(c1, c3)
 
         user = tests.FakeUser()
-
         # Set user logged in
         with tests.user_set(pagure.APP, user):
-            # View commits comparison
-            output = self.app.get('/test/c/%s..%s' % (c1.oid.hex, c2.oid.hex))
-            self.assertEqual(output.status_code, 200)
-            self.assertIn(
-                '<h5 class="text-muted">%s .. %s</h5>' % (c1.oid.hex, c2.oid.hex),
-                output.data)
-            self.assertIn(
-                '<span style="color: #800080; font-weight: bold">@@ -0,0 +1,3 @@</span>',
-                output.data)
-            self.assertIn('<span style="color: #00A000">+ foo</span>', output.data)
-            self.assertIn('<span style="color: #00A000">+  bar</span>', output.data)
-            self.assertIn('<span style="color: #00A000">+ baz </span>', output.data)
-
-            # View inverse commits comparison
-            output = self.app.get(
-                '/test/c/%s..%s' % (c2.oid.hex, c1.oid.hex))
-            self.assertIn(
-                '<h5 class="text-muted">%s .. %s</h5>' % (c2.oid.hex, c1.oid.hex),
-                output.data)
-            self.assertIn(
-                '<span style="color: #800080; font-weight: bold">@@ -1,3 +0,0 @@</span>',
-                output.data)
-            self.assertIn('<span style="color: #a40000">- foo</span>', output.data)
-            self.assertIn('<span style="color: #a40000">-  bar</span>', output.data)
-            self.assertIn('<span style="color: #a40000">- baz </span>', output.data)
+            compare_first_two(c1, c2)
+            compare_all(c1, c3)
 
     def test_view_file(self):
         """ Test the view_file endpoint. """