Launchpad itself

Merge lp:~abentley/launchpad/restricted-diffs into lp:launchpad

  1. restricted-diffs
  2. Merge into devel
Proposed by Aaron Bentley
Status: Merged
Approved by: Tim Penhey
Approved revision: not available
Merged at revision: not available
Proposed branch: lp:~abentley/launchpad/restricted-diffs
Merge into: lp:launchpad
Diff against target: 109 lines (+36/-6)
4 files modified
lib/lp/code/browser/configure.zcml (+6/-0)
lib/lp/code/browser/diff.py (+12/-4)
lib/lp/code/browser/tests/test_tales.py (+12/-1)
lib/lp/testing/factory.py (+6/-1)
To merge this branch: bzr merge lp:~abentley/launchpad/restricted-diffs
Reviewer Review Type Date Requested Status
Tim Penhey (community) Approve
Review via email: mp+19999@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Aaron Bentley (abentley) wrote :

= Summary =
Fix bug #525857: "attempt to get canonical_url of a review diff"

== Proposed fix ==
Implement a TAL formatter for IDiff

== Pre-implementation notes ==
None

== Implementation details ==
Made PreviewDiffFormatterAPI a subclass of DiffFormatterAPI, and used "template
method" to get the URL.

As a drive-by, added makeStaticDiff to the LaunchpadObjectFactory.

== Tests ==
-t bin/test test_tales -t TestDiffFormatter

== Demo and Q/A ==
Go to an old merge proposal, like
https://code.edge.launchpad.net/~adeuring/launchpad/hwdb-dmi-data-parser/+merge/8881.
This should display correctly, not oops.

= Launchpad lint =

Checking for conflicts. and issues in doctests and templates.
Running jslint, xmllint, pyflakes, and pylint.
Using normal rules.

Linting changed files:
  lib/lp/code/browser/configure.zcml
  lib/lp/code/browser/diff.py
  lib/lp/testing/factory.py
  lib/lp/code/browser/tests/test_tales.py

== Pylint notices ==

lib/lp/testing/factory.py
    1627: [F0401, LaunchpadObjectFactory.makeRecipe] Unable to import 'bzrlib.plugins.builder.recipe' (No module named builder)

lib/lp/code/browser/tests/test_tales.py
    70: [C0324, TestPreviewDiffFormatter.test_creation_method] Comma not followed by a space
    self.assertEqual({'filename': (3,2)}, preview.diffstat)
    ^^

Revision history for this message
Tim Penhey (thumper) wrote :

Looks fine.

review: Approve

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 2010-02-19 16:33:29 +0000
3+++ lib/lp/code/browser/configure.zcml 2010-02-23 20:16:22 +0000
4@@ -1061,5 +1061,11 @@
5 factory="lp.code.browser.diff.PreviewDiffFormatterAPI"
6 name="fmt"
7 />
8+ <adapter
9+ for="lp.code.interfaces.diff.IDiff"
10+ provides="zope.traversing.interfaces.IPathAdapter"
11+ factory="lp.code.browser.diff.DiffFormatterAPI"
12+ name="fmt"
13+ />
14
15 </configure>
16
17=== modified file 'lib/lp/code/browser/diff.py'
18--- lib/lp/code/browser/diff.py 2010-02-19 16:31:54 +0000
19+++ lib/lp/code/browser/diff.py 2010-02-23 20:16:22 +0000
20@@ -23,8 +23,10 @@
21 usedfor = IPreviewDiff
22
23
24-class PreviewDiffFormatterAPI(ObjectFormatterAPI):
25- """Formatter for preview diffs."""
26+class DiffFormatterAPI(ObjectFormatterAPI):
27+
28+ def _get_url(self, librarian_alias):
29+ return librarian_alias.getURL()
30
31 def url(self, view_name=None, rootsite=None):
32 """Use the url of the librarian file containing the diff.
33@@ -32,8 +34,7 @@
34 librarian_alias = self._context.diff_text
35 if librarian_alias is None:
36 return None
37- else:
38- return canonical_url(self._context) + '/+files/preview.diff'
39+ return self._get_url(librarian_alias)
40
41 def link(self, view_name):
42 """The link to the diff should show the line count.
43@@ -85,3 +86,10 @@
44 '<a href="%(url)s" class="diff-link">'
45 '%(line_count)s%(count_text)s%(file_text)s%(conflict_text)s'
46 '</a>' % args)
47+
48+
49+class PreviewDiffFormatterAPI(DiffFormatterAPI):
50+ """Formatter for preview diffs."""
51+
52+ def _get_url(self, library_):
53+ return canonical_url(self._context) + '/+files/preview.diff'
54
55=== modified file 'lib/lp/code/browser/tests/test_tales.py'
56--- lib/lp/code/browser/tests/test_tales.py 2010-02-18 17:23:49 +0000
57+++ lib/lp/code/browser/tests/test_tales.py 2010-02-23 20:16:22 +0000
58@@ -67,7 +67,7 @@
59 self.assertEqual(False, preview.stale)
60 self.assertEqual(True, self._createStalePreviewDiff().stale)
61 self.assertEqual(u'conflicts', preview.conflicts)
62- self.assertEqual({'filename': (3,2)}, preview.diffstat)
63+ self.assertEqual({'filename': (3, 2)}, preview.diffstat)
64
65 def test_fmt_no_diff(self):
66 # If there is no diff, there is no link.
67@@ -159,6 +159,17 @@
68 test_tales('preview/fmt:link', preview=preview))
69
70
71+class TestDiffFormatter(TestCaseWithFactory):
72+ """Test the DiffFormatterAPI class."""
73+
74+ layer = LaunchpadFunctionalLayer
75+
76+ def test_url(self):
77+ diff = self.factory.makeDiff()
78+ self.assertEqual(
79+ diff.diff_text.getURL(), test_tales('diff/fmt:url', diff=diff))
80+
81+
82 def test_suite():
83 return unittest.TestLoader().loadTestsFromName(__name__)
84
85
86=== modified file 'lib/lp/testing/factory.py'
87--- lib/lp/testing/factory.py 2010-02-22 20:12:10 +0000
88+++ lib/lp/testing/factory.py 2010-02-23 20:16:22 +0000
89@@ -101,7 +101,7 @@
90 from lp.code.interfaces.codeimportmachine import ICodeImportMachineSet
91 from lp.code.interfaces.codeimportresult import ICodeImportResultSet
92 from lp.code.interfaces.revision import IRevisionSet
93-from lp.code.model.diff import Diff, PreviewDiff
94+from lp.code.model.diff import Diff, PreviewDiff, StaticDiff
95 from lp.registry.model.distributionsourcepackage import (
96 DistributionSourcePackage)
97 from lp.registry.model.milestone import Milestone
98@@ -980,6 +980,11 @@
99 preview_diff.target_revision_id = self.getUniqueUnicode()
100 return preview_diff
101
102+ def makeStaticDiff(self):
103+ return StaticDiff.acquireFromText(
104+ self.getUniqueUnicode(), self.getUniqueUnicode(),
105+ self.getUniqueString())
106+
107 def makeRevision(self, author=None, revision_date=None, parent_ids=None,
108 rev_id=None, log_body=None, date_created=None):
109 """Create a single `Revision`."""