Merge lp:~cjwatson/launchpad/git-ref-ui into lp:launchpad

Proposed by Colin Watson on 2015-03-19
Status: Merged
Approved by: Colin Watson on 2015-03-24
Approved revision: no longer in the source branch.
Merged at revision: 17415
Proposed branch: lp:~cjwatson/launchpad/git-ref-ui
Merge into: lp:launchpad
Prerequisite: lp:~cjwatson/launchpad/git-ref-scanner-commits
Diff against target: 572 lines (+307/-24)
16 files modified
lib/lp/code/browser/configure.zcml (+19/-4)
lib/lp/code/browser/gitref.py (+9/-0)
lib/lp/code/browser/gitrepository.py (+28/-0)
lib/lp/code/browser/tests/test_gitrepository.py (+45/-1)
lib/lp/code/interfaces/gitref.py (+10/-0)
lib/lp/code/interfaces/gitrepository.py (+2/-0)
lib/lp/code/model/gitref.py (+4/-0)
lib/lp/code/model/gitrepository.py (+12/-9)
lib/lp/code/model/tests/test_gitjob.py (+1/-1)
lib/lp/code/model/tests/test_gitrepository.py (+1/-1)
lib/lp/code/templates/gitref-commits.pt (+27/-0)
lib/lp/code/templates/gitref-index.pt (+12/-0)
lib/lp/code/templates/gitref-listing.pt (+83/-0)
lib/lp/code/templates/gitref-macros.pt (+44/-0)
lib/lp/code/templates/gitrepository-index.pt (+8/-0)
lib/lp/services/doc/sprites.txt (+2/-8)
To merge this branch: bzr merge lp:~cjwatson/launchpad/git-ref-ui
Reviewer Review Type Date Requested Status
William Grant code 2015-03-19 Approve on 2015-03-24
Review via email: mp+253550@code.launchpad.net

Commit message

Flesh out GitRef:+index a little bit, and add a branch listing to GitRepository:+index.

Description of the change

Flesh out GitRef:+index a little bit, and add a branch listing to GitRepository:+index.

This is still pretty basic, but is functional. There are a couple of screenshots here:

  https://bugs.launchpad.net/launchpad/+bug/1032731/+attachment/4350129/+files/gitref-listing-grub.png
  https://bugs.launchpad.net/launchpad/+bug/1032731/+attachment/4350130/+files/gitref-index-grub.png

To post a comment you must log in.
William Grant (wgrant) wrote :

I'm not sure that the Git icon is useful at that size, and it's not used in listings any more, so perhaps we can do away with it.

review: Approve (code)
Colin Watson (cjwatson) :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'lib/lp/code/browser/configure.zcml'
2--- lib/lp/code/browser/configure.zcml 2015-03-13 14:15:24 +0000
3+++ lib/lp/code/browser/configure.zcml 2015-03-24 15:16:03 +0000
4@@ -806,12 +806,27 @@
5 path_expression="string:+ref/${path}"
6 attribute_to_parent="repository"
7 rootsite="code"/>
8- <browser:page
9+ <browser:pages
10 for="lp.code.interfaces.gitref.IGitRef"
11 class="lp.code.browser.gitref.GitRefView"
12- permission="launchpad.View"
13- name="+index"
14- template="../templates/gitref-index.pt"/>
15+ permission="launchpad.View">
16+ <browser:page
17+ name="+index"
18+ template="../templates/gitref-index.pt"/>
19+ <browser:page
20+ name="++ref-commits"
21+ template="../templates/gitref-commits.pt"/>
22+ </browser:pages>
23+ <browser:page
24+ for="lp.code.interfaces.gitref.IGitRef"
25+ name="+macros"
26+ permission="zope.Public"
27+ template="../templates/gitref-macros.pt"/>
28+ <browser:page
29+ for="lp.code.interfaces.gitref.IGitRefBatchNavigator"
30+ permission="zope.Public"
31+ name="+ref-listing"
32+ template="../templates/gitref-listing.pt"/>
33
34 <browser:menus
35 classes="ProductBranchesMenu"
36
37=== modified file 'lib/lp/code/browser/gitref.py'
38--- lib/lp/code/browser/gitref.py 2015-03-13 14:15:24 +0000
39+++ lib/lp/code/browser/gitref.py 2015-03-24 15:16:03 +0000
40@@ -17,3 +17,12 @@
41 @property
42 def label(self):
43 return self.context.display_name
44+
45+ @property
46+ def tip_commit_info(self):
47+ return {
48+ "sha1": self.context.commit_sha1,
49+ "author": self.context.author,
50+ "author_date": self.context.author_date,
51+ "commit_message": self.context.commit_message,
52+ }
53
54=== modified file 'lib/lp/code/browser/gitrepository.py'
55--- lib/lp/code/browser/gitrepository.py 2015-03-13 14:15:24 +0000
56+++ lib/lp/code/browser/gitrepository.py 2015-03-24 15:16:03 +0000
57@@ -18,6 +18,7 @@
58
59 from lp.app.browser.informationtype import InformationTypePortletMixin
60 from lp.app.errors import NotFoundError
61+from lp.code.interfaces.gitref import IGitRefBatchNavigator
62 from lp.code.interfaces.gitrepository import IGitRepository
63 from lp.services.config import config
64 from lp.services.webapp import (
65@@ -31,6 +32,7 @@
66 check_permission,
67 precache_permission_for_objects,
68 )
69+from lp.services.webapp.batching import TableBatchNavigator
70 from lp.services.webapp.breadcrumb import NameBreadcrumb
71 from lp.services.webapp.interfaces import ICanonicalUrlData
72
73@@ -90,6 +92,28 @@
74 return Link(url, text, icon="info")
75
76
77+class GitRefBatchNavigator(TableBatchNavigator):
78+ """Batch up the branch listings."""
79+ implements(IGitRefBatchNavigator)
80+
81+ def __init__(self, view, context):
82+ super(GitRefBatchNavigator, self).__init__(
83+ context.branches, view.request,
84+ size=config.launchpad.branchlisting_batch_size)
85+ self.view = view
86+ self.column_count = 3
87+
88+ @property
89+ def table_class(self):
90+ # XXX: MichaelHudson 2007-10-18 bug=153894: This means there are two
91+ # ways of sorting a one-page branch listing, which is confusing and
92+ # incoherent.
93+ if self.has_multiple_pages:
94+ return "listing"
95+ else:
96+ return "listing sortable"
97+
98+
99 class GitRepositoryView(InformationTypePortletMixin, LaunchpadView):
100
101 @property
102@@ -128,3 +152,7 @@
103 def user_can_push(self):
104 """Whether the user can push to this branch."""
105 return check_permission("launchpad.Edit", self.context)
106+
107+ def branches(self):
108+ """All branches in this repository, sorted for display."""
109+ return GitRefBatchNavigator(self, self.context)
110
111=== modified file 'lib/lp/code/browser/tests/test_gitrepository.py'
112--- lib/lp/code/browser/tests/test_gitrepository.py 2015-03-13 14:15:24 +0000
113+++ lib/lp/code/browser/tests/test_gitrepository.py 2015-03-24 15:16:03 +0000
114@@ -5,15 +5,21 @@
115
116 __metaclass__ = type
117
118+from datetime import datetime
119+
120 from BeautifulSoup import BeautifulSoup
121 from bzrlib import urlutils
122 from fixtures import FakeLogger
123+import pytz
124+from testtools.matchers import Equals
125 from zope.component import getUtility
126 from zope.publisher.interfaces import NotFound
127+from zope.security.proxy import removeSecurityProxy
128
129 from lp.app.enums import InformationType
130 from lp.app.interfaces.services import IService
131 from lp.code.interfaces.gitrepository import GIT_FEATURE_FLAG
132+from lp.code.interfaces.revision import IRevisionSet
133 from lp.registry.interfaces.person import PersonVisibility
134 from lp.services.config import config
135 from lp.services.features.testing import FeatureFixture
136@@ -24,9 +30,11 @@
137 login_person,
138 logout,
139 person_logged_in,
140+ record_two_runs,
141 TestCaseWithFactory,
142 )
143 from lp.testing.layers import DatabaseFunctionalLayer
144+from lp.testing.matchers import HasQueryCount
145 from lp.testing.pages import (
146 setupBrowser,
147 setupBrowserForUser,
148@@ -148,7 +156,7 @@
149
150
151 class TestGitRepositoryViewPrivateArtifacts(BrowserTestCase):
152- """ Tests that Git repositories with private team artifacts can be viewed.
153+ """Tests that Git repositories with private team artifacts can be viewed.
154
155 A repository may be associated with a private team as follows:
156 - the owner is a private team
157@@ -198,3 +206,39 @@
158 url = canonical_url(repository, rootsite="code")
159 browser = self._getBrowser()
160 self.assertRaises(NotFound, browser.open, url)
161+
162+
163+class TestGitRepositoryBranches(BrowserTestCase):
164+ """Test the listing of branches in a Git repository."""
165+
166+ layer = DatabaseFunctionalLayer
167+
168+ def setUp(self):
169+ super(TestGitRepositoryBranches, self).setUp()
170+ self.useFixture(FeatureFixture({GIT_FEATURE_FLAG: u"on"}))
171+
172+ def makeRevisionAuthor(self, person=None):
173+ if person is None:
174+ person = self.factory.makePerson()
175+ email = removeSecurityProxy(person).preferredemail.email
176+ return getUtility(IRevisionSet).acquireRevisionAuthors([email])[email]
177+
178+ def test_query_count(self):
179+ # The number of queries is constant in the number of refs.
180+ person = self.factory.makePerson()
181+ repository = self.factory.makeGitRepository(owner=person)
182+ now = datetime.now(pytz.UTC)
183+
184+ def create_ref():
185+ with person_logged_in(person):
186+ [ref] = self.factory.makeGitRefs(repository=repository)
187+ naked_ref = removeSecurityProxy(ref)
188+ naked_ref.author = self.makeRevisionAuthor()
189+ naked_ref.author_date = now
190+ naked_ref.committer = self.makeRevisionAuthor()
191+ naked_ref.committer_date = now
192+ naked_ref.commit_message = u"something"
193+
194+ recorder1, recorder2 = record_two_runs(
195+ lambda: self.getMainText(repository, "+index"), create_ref, 10)
196+ self.assertThat(recorder2, HasQueryCount(Equals(recorder1.count)))
197
198=== modified file 'lib/lp/code/interfaces/gitref.py'
199--- lib/lp/code/interfaces/gitref.py 2015-03-19 11:15:48 +0000
200+++ lib/lp/code/interfaces/gitref.py 2015-03-24 15:16:03 +0000
201@@ -7,6 +7,7 @@
202
203 __all__ = [
204 'IGitRef',
205+ 'IGitRefBatchNavigator',
206 ]
207
208 from zope.interface import (
209@@ -22,6 +23,7 @@
210
211 from lp import _
212 from lp.code.enums import GitObjectType
213+from lp.services.webapp.interfaces import ITableBatchNavigator
214
215
216 class IGitRef(Interface):
217@@ -65,3 +67,11 @@
218 display_name = TextLine(
219 title=_("Display name"), required=True, readonly=True,
220 description=_("Display name of the reference."))
221+
222+ commit_message_first_line = TextLine(
223+ title=_("The first line of the commit message."),
224+ required=True, readonly=True)
225+
226+
227+class IGitRefBatchNavigator(ITableBatchNavigator):
228+ pass
229
230=== modified file 'lib/lp/code/interfaces/gitrepository.py'
231--- lib/lp/code/interfaces/gitrepository.py 2015-03-20 14:17:28 +0000
232+++ lib/lp/code/interfaces/gitrepository.py 2015-03-24 15:16:03 +0000
233@@ -188,6 +188,8 @@
234
235 refs = Attribute("The references present in this repository.")
236
237+ branches = Attribute("The branch references present in this repository.")
238+
239 def getRefByPath(path):
240 """Look up a single reference in this repository by path.
241
242
243=== modified file 'lib/lp/code/model/gitref.py'
244--- lib/lp/code/model/gitref.py 2015-03-19 11:15:48 +0000
245+++ lib/lp/code/model/gitref.py 2015-03-24 15:16:03 +0000
246@@ -53,3 +53,7 @@
247 @property
248 def display_name(self):
249 return self.path.split("/", 2)[-1]
250+
251+ @property
252+ def commit_message_first_line(self):
253+ return self.commit_message.split("\n", 1)[0]
254
255=== modified file 'lib/lp/code/model/gitrepository.py'
256--- lib/lp/code/model/gitrepository.py 2015-03-20 14:17:28 +0000
257+++ lib/lp/code/model/gitrepository.py 2015-03-24 15:16:03 +0000
258@@ -109,10 +109,7 @@
259 Values,
260 )
261 from lp.services.features import getFeatureFlag
262-from lp.services.propertycache import (
263- cachedproperty,
264- get_property_cache,
265- )
266+from lp.services.propertycache import cachedproperty
267 from lp.services.webapp.authorization import available_with_permission
268
269
270@@ -311,11 +308,19 @@
271 reconcile_access_for_artifact(
272 self, self.information_type, pillars, wanted_links)
273
274- @cachedproperty
275+ @property
276 def refs(self):
277 """See `IGitRepository`."""
278- return list(Store.of(self).find(
279- GitRef, GitRef.repository_id == self.id).order_by(GitRef.path))
280+ return Store.of(self).find(
281+ GitRef, GitRef.repository_id == self.id).order_by(GitRef.path)
282+
283+ @property
284+ def branches(self):
285+ """See `IGitRepository`."""
286+ return Store.of(self).find(
287+ GitRef,
288+ GitRef.repository_id == self.id,
289+ GitRef.path.startswith(u"refs/heads/")).order_by(GitRef.path)
290
291 def getRefByPath(self, path):
292 return Store.of(self).find(
293@@ -416,7 +421,6 @@
294 else:
295 created = []
296
297- del get_property_cache(self).refs
298 if get_objects:
299 return bulk.load(GitRef, updated + created)
300
301@@ -425,7 +429,6 @@
302 Store.of(self).find(
303 GitRef,
304 GitRef.repository == self, GitRef.path.is_in(paths)).remove()
305- del get_property_cache(self).refs
306
307 def planRefChanges(self, hosting_client, hosting_path, logger=None):
308 """See `IGitRepository`."""
309
310=== modified file 'lib/lp/code/model/tests/test_gitjob.py'
311--- lib/lp/code/model/tests/test_gitjob.py 2015-03-20 11:51:05 +0000
312+++ lib/lp/code/model/tests/test_gitjob.py 2015-03-24 15:16:03 +0000
313@@ -154,7 +154,7 @@
314 with self.expectedLog(expected_message):
315 with dbuser("branchscanner"):
316 job.run()
317- self.assertEqual([], repository.refs)
318+ self.assertEqual([], list(repository.refs))
319
320
321 # XXX cjwatson 2015-03-12: We should test that the job works via Celery too,
322
323=== modified file 'lib/lp/code/model/tests/test_gitrepository.py'
324--- lib/lp/code/model/tests/test_gitrepository.py 2015-03-20 14:17:28 +0000
325+++ lib/lp/code/model/tests/test_gitrepository.py 2015-03-24 15:16:03 +0000
326@@ -482,7 +482,7 @@
327
328 def test_create(self):
329 repository = self.factory.makeGitRepository()
330- self.assertEqual([], repository.refs)
331+ self.assertEqual([], list(repository.refs))
332 paths = (u"refs/heads/master", u"refs/tags/1.0")
333 self.factory.makeGitRefs(repository=repository, paths=paths)
334 self.assertRefsMatch(repository.refs, repository, paths)
335
336=== added file 'lib/lp/code/templates/gitref-commits.pt'
337--- lib/lp/code/templates/gitref-commits.pt 1970-01-01 00:00:00 +0000
338+++ lib/lp/code/templates/gitref-commits.pt 2015-03-24 15:16:03 +0000
339@@ -0,0 +1,27 @@
340+<div
341+ xmlns:tal="http://xml.zope.org/namespaces/tal"
342+ xmlns:metal="http://xml.zope.org/namespaces/metal"
343+ xmlns:i18n="http://xml.zope.org/namespaces/i18n">
344+
345+ <p tal:condition="not: context/committer_date">
346+ <metal:no-commit-message use-macro="context/@@+macros/no-commit-message" />
347+ </p>
348+
349+ <tal:has_commit condition="context/committer_date">
350+ <style type="text/css">
351+ .subordinate {
352+ margin-left: 1em;
353+ }
354+ </style>
355+ <dl class="revision">
356+ <tal:comment condition="nothing">
357+ XXX cjwatson 2015-03-19: This should display more commits once we
358+ have them. For now, we only have the tip commit.
359+ </tal:comment>
360+ <tal:tip define="commit_info view/tip_commit_info">
361+ <metal:commit-text use-macro="context/@@+macros/commit-text" />
362+ </tal:tip>
363+ </dl>
364+ </tal:has_commit>
365+
366+</div>
367
368=== modified file 'lib/lp/code/templates/gitref-index.pt'
369--- lib/lp/code/templates/gitref-index.pt 2015-03-13 14:15:24 +0000
370+++ lib/lp/code/templates/gitref-index.pt 2015-03-24 15:16:03 +0000
371@@ -9,6 +9,11 @@
372
373 <body>
374
375+<tal:registering metal:fill-slot="registering">
376+ Last commit made on
377+ <tal:committer-date replace="structure context/committer_date/fmt:date" />
378+</tal:registering>
379+
380 <div metal:fill-slot="main">
381
382 <div class="yui-g">
383@@ -28,6 +33,13 @@
384 </div>
385 </div>
386
387+ <div class="yui-g">
388+ <div class="portlet" id="recent-commits">
389+ <h2>Recent commits</h2>
390+ <tal:commits replace="structure context/@@++ref-commits" />
391+ </div>
392+ </div>
393+
394 </div>
395
396 </body>
397
398=== added file 'lib/lp/code/templates/gitref-listing.pt'
399--- lib/lp/code/templates/gitref-listing.pt 1970-01-01 00:00:00 +0000
400+++ lib/lp/code/templates/gitref-listing.pt 2015-03-24 15:16:03 +0000
401@@ -0,0 +1,83 @@
402+<div
403+ xmlns:tal="http://xml.zope.org/namespaces/tal"
404+ xmlns:metal="http://xml.zope.org/namespaces/metal"
405+ xmlns:i18n="http://xml.zope.org/namespaces/i18n">
406+
407+ <script type="text/javascript">
408+function show_git_commit(id) {
409+ var div = document.getElementById('git-ref-' + id);
410+ if (div) {
411+ div.style.display = "block";
412+ }
413+}
414+function hide_git_commit(id) {
415+ var div = document.getElementById('git-ref-' + id);
416+ if (div) {
417+ div.style.display = "none";
418+ }
419+}
420+ </script>
421+
422+ <tal:needs-batch condition="context/has_multiple_pages">
423+ <div id="branch-batch-links">
424+ <tal:navigation replace="structure context/@@+navigation-links-upper" />
425+ </div>
426+ </tal:needs-batch>
427+
428+ <table tal:attributes="class context/table_class" id="branchtable">
429+ <thead>
430+ <tr>
431+ <th>Name</th>
432+ <th>Last Modified</th>
433+ <th>Last Commit</th>
434+ </tr>
435+ </thead>
436+ <tbody>
437+ <tr tal:repeat="ref context/currentBatch">
438+ <td>
439+ <a tal:attributes="href ref/fmt:url"
440+ tal:content="structure ref/display_name/fmt:break-long-words" />
441+ </td>
442+ <tal:no_commit condition="not: ref/committer_date">
443+ <td colspan="2">
444+ <metal:no-commit-message use-macro="ref/@@+macros/no-commit-message" />
445+ </td>
446+ </tal:no_commit>
447+ <tal:has_commit condition="ref/committer_date">
448+ <td>
449+ <span class="sortkey"
450+ tal:content="ref/committer_date/fmt:datetime" />
451+ <span tal:attributes="title ref/committer_date/fmt:datetime"
452+ tal:content="ref/committer_date/fmt:approximatedate" />
453+ </td>
454+ <td tal:attributes="onmouseover string:show_git_commit(${repeat/ref/index});
455+ onmouseout string:hide_git_commit(${repeat/ref/index});">
456+ <div class="lastCommit">
457+ <tal:revision-log replace="ref/commit_message_first_line/fmt:shorten/80" />
458+ </div>
459+ <div class="popupTitle"
460+ tal:attributes="id string:git-ref-${repeat/ref/index};
461+ onmouseover string:hide_commit(${repeat/ref/index});">
462+ <p>
463+ <strong>Author:</strong>
464+ <tal:author replace="structure ref/author/fmt:link" />
465+ <br />
466+ <strong>Author Date:</strong>
467+ <tal:author replace="structure ref/author_date/fmt:datetime" />
468+ </p>
469+ <tal:commit-msg replace="structure ref/commit_message/fmt:text-to-html" />
470+ </div>
471+ </td>
472+ </tal:has_commit>
473+ </tr>
474+ <tr tal:condition="not: context/batch/total">
475+ <td colspan="3">
476+ This repository has no branches.
477+ </td>
478+ </tr>
479+ </tbody>
480+ </table>
481+
482+ <tal:navigation replace="structure context/@@+navigation-links-lower" />
483+
484+</div>
485
486=== added file 'lib/lp/code/templates/gitref-macros.pt'
487--- lib/lp/code/templates/gitref-macros.pt 1970-01-01 00:00:00 +0000
488+++ lib/lp/code/templates/gitref-macros.pt 2015-03-24 15:16:03 +0000
489@@ -0,0 +1,44 @@
490+<tal:root
491+ xmlns:tal="http://xml.zope.org/namespaces/tal"
492+ xmlns:metal="http://xml.zope.org/namespaces/metal"
493+ omit-tag="">
494+
495+<metal:commit-text define-macro="commit-text">
496+
497+ <tal:comment condition="nothing">
498+ This macro requires the following defined variable:
499+ commit_info - a dict of the commit information (sha1, author,
500+ author_date, commit_message) to be displayed.
501+
502+ It is expected that this macro is called from within a definition list
503+ (<dl></dl>).
504+ </tal:comment>
505+ <dt class="commit-details"
506+ tal:define="
507+ sha1 python:commit_info['sha1'];
508+ author python:commit_info['author'];
509+ author_date python:commit_info['author_date']">
510+ <tal:sha1 replace="sha1/fmt:shorten/10" />
511+ by
512+ <tal:known-person condition="author/person">
513+ <tal:person-link replace="structure author/person/fmt:link" />
514+ </tal:known-person>
515+ <tal:unknown-person condition="not: author/person">
516+ <strong tal:content="author/name/fmt:obfuscate-email" />
517+ </tal:unknown-person>
518+ <span tal:attributes="title author_date/fmt:datetime"
519+ tal:content="author_date/fmt:displaydate" />
520+ </dt>
521+ <dd class="subordinate commit-message"
522+ tal:define="commit_message python:commit_info['commit_message']">
523+ <tal:commit-message
524+ replace="structure commit_message/fmt:obfuscate-email/fmt:text-to-html" />
525+ </dd>
526+
527+</metal:commit-text>
528+
529+<metal:no-commit-message define-macro="no-commit-message">
530+ This branch has not been scanned yet.
531+</metal:no-commit-message>
532+
533+</tal:root>
534
535=== modified file 'lib/lp/code/templates/gitrepository-index.pt'
536--- lib/lp/code/templates/gitrepository-index.pt 2015-03-17 16:05:54 +0000
537+++ lib/lp/code/templates/gitrepository-index.pt 2015-03-24 15:16:03 +0000
538@@ -51,6 +51,14 @@
539 </div>
540 </div>
541
542+ <div class="yui-g">
543+ <div id="repository-branches" class="portlet"
544+ tal:define="branches view/branches">
545+ <h2>Branches</h2>
546+ <tal:repository-branches replace="structure branches/@@+ref-listing" />
547+ </div>
548+ </div>
549+
550 </div>
551
552 </body>
553
554=== modified file 'lib/lp/services/doc/sprites.txt'
555--- lib/lp/services/doc/sprites.txt 2012-06-02 03:19:33 +0000
556+++ lib/lp/services/doc/sprites.txt 2015-03-24 15:16:03 +0000
557@@ -6,14 +6,8 @@
558 view a page. An individual sprite is displayed as a background image
559 on an element by setting the background position.
560
561-A new image can be added to the combined file with the command::
562-
563- make sprite_image
564-
565-The resulting icon-sprites.png and icon-sprites.positioning files must
566-be committed to the repository. Any changes to the CSS template will be
567-automatically picked up when Launchpad is restarted. If you don't want
568-to restart launchpad.dev, you can run::
569+A new image can be added to the combined image file and CSS template with
570+the command::
571
572 make css_combine
573