Code review comment for lp:~jcsackett/launchpad/unknown-blueprints-service-597738
- unknown-blueprints-service-597738
- Merge into devel
Revision history for this message
| j.c.sackett (jcsackett) wrote : | # |
| 1 | === modified file 'lib/lp/blueprints/browser/specificationtarget.py' |
| 2 | --- lib/lp/blueprints/browser/specificationtarget.py 2010-09-13 20:21:10 +0000 |
| 3 | +++ lib/lp/blueprints/browser/specificationtarget.py 2010-09-14 15:49:09 +0000 |
| 4 | @@ -26,6 +26,7 @@ |
| 5 | canonical_url, |
| 6 | LaunchpadView, |
| 7 | ) |
| 8 | +from canonical.launchpad.webapp.authorization import check_permission |
| 9 | from canonical.launchpad.webapp.batching import BatchNavigator |
| 10 | from canonical.launchpad.webapp.breadcrumb import Breadcrumb |
| 11 | from canonical.launchpad.webapp.menu import ( |
| 12 | @@ -219,6 +220,15 @@ |
| 13 | size=config.launchpad.default_batch_size) |
| 14 | |
| 15 | @property |
| 16 | + def can_configure_blueprints(self): |
| 17 | + """Can the user configure blueprints for the `ISpecificationTarget`.""" |
| 18 | + target = self.context |
| 19 | + if IProduct.providedBy(target) or IDistribution.providedBy(target): |
| 20 | + return check_permission('launchpad.Edit', self.context) |
| 21 | + else: |
| 22 | + return False |
| 23 | + |
| 24 | + @property |
| 25 | def label(self): |
| 26 | mapping = {'name': self.context.displayname} |
| 27 | if self.is_person: |
| 28 | |
| 29 | === modified file 'lib/lp/blueprints/browser/tests/test_specificationtarget.py' |
| 30 | --- lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-09-09 18:11:41 +0000 |
| 31 | +++ lib/lp/blueprints/browser/tests/test_specificationtarget.py 2010-09-14 15:47:58 +0000 |
| 32 | @@ -18,7 +18,10 @@ |
| 33 | login_person, |
| 34 | TestCaseWithFactory, |
| 35 | ) |
| 36 | -from lp.testing.views import create_view |
| 37 | +from lp.testing.views import ( |
| 38 | + create_view, |
| 39 | + create_initialized_view, |
| 40 | + ) |
| 41 | |
| 42 | |
| 43 | class TestRegisterABlueprintButtonView(TestCaseWithFactory): |
| 44 | @@ -149,6 +152,38 @@ |
| 45 | view.template.filename) |
| 46 | |
| 47 | |
| 48 | +class TestHasSpecificationsConfiguration(TestCaseWithFactory): |
| 49 | + |
| 50 | + layer = DatabaseFunctionalLayer |
| 51 | + |
| 52 | + def test_cannot_configure_blueprints_product_no_edit_permission(self): |
| 53 | + product = self.factory.makeProduct() |
| 54 | + view = create_initialized_view(product, '+specs') |
| 55 | + self.assertEqual(False, view.can_configure_blueprints) |
| 56 | + |
| 57 | + def test_can_configure_blueprints_product_with_edit_permission(self): |
| 58 | + product = self.factory.makeProduct() |
| 59 | + login_person(product.owner) |
| 60 | + view = create_initialized_view(product, '+specs') |
| 61 | + self.assertEqual(True, view.can_configure_blueprints) |
| 62 | + |
| 63 | + def test_cannot_configure_blueprints_distribution_no_edit_permission(self): |
| 64 | + distribution = self.factory.makeDistribution() |
| 65 | + view = create_initialized_view(distribution, '+specs') |
| 66 | + self.assertEqual(False, view.can_configure_blueprints) |
| 67 | + |
| 68 | + def test_can_configure_blueprints_distribution_with_edit_permission(self): |
| 69 | + distribution = self.factory.makeDistribution() |
| 70 | + login_person(distribution.owner) |
| 71 | + view = create_initialized_view(distribution, '+specs') |
| 72 | + self.assertEqual(True, view.can_configure_blueprints) |
| 73 | + |
| 74 | + def test_cannot_configure_blueprints_projectgroup_with_edit_permission(self): |
| 75 | + project_group = self.factory.makeProject() |
| 76 | + login_person(project_group.owner) |
| 77 | + view = create_initialized_view(project_group, '+specs') |
| 78 | + self.assertEqual(False, view.can_configure_blueprints) |
| 79 | + |
| 80 | def test_suite(): |
| 81 | suite = unittest.TestSuite() |
| 82 | suite.addTest(unittest.TestLoader().loadTestsFromName(__name__)) |
| 83 | |
| 84 | === modified file 'lib/lp/blueprints/templates/unknown-specs.pt' |
| 85 | --- lib/lp/blueprints/templates/unknown-specs.pt 2010-09-10 16:34:26 +0000 |
| 86 | +++ lib/lp/blueprints/templates/unknown-specs.pt 2010-09-14 16:02:38 +0000 |
| 87 | @@ -11,49 +11,47 @@ |
| 88 | |
| 89 | <div metal:fill-slot="main" |
| 90 | tal:define="specs view/specs; |
| 91 | - has_any_specs view/has_any_specifications"> |
| 92 | - |
| 93 | - <div tal:condition="view/context/blueprints_usage/enumvalue:EXTERNAL"> |
| 94 | - <strong><tal:project replace="view/context/displayname" /> does not use Launchpad |
| 95 | - for specification tracking.</strong> |
| 96 | - </div> |
| 97 | - |
| 98 | - <div tal:condition="view/context/blueprints_usage/enumvalue:NOT_APPLICABLE"> |
| 99 | - <strong><tal:project replace="view/context/displayname" /> does not track |
| 100 | - specifications.</strong> |
| 101 | - </div> |
| 102 | - |
| 103 | - <div tal:condition="view/context/blueprints_usage/enumvalue:UNKNOWN"> |
| 104 | - <strong> |
| 105 | - Launchpad does not know how |
| 106 | - <tal:project replace="view/context/displayname" /> uses specifications. |
| 107 | - </strong> |
| 108 | - </div> |
| 109 | - |
| 110 | - <div tal:condition="view/context/wikiurl"> |
| 111 | - <p><tal:project replace="view/context/displayname" /> has a wiki, which |
| 112 | - may be used for specifications.</p> |
| 113 | - |
| 114 | - <p> |
| 115 | + has_any_specs view/has_any_specifications; |
| 116 | + blueprints_usage view/context/blueprints_usage"> |
| 117 | + <div class="top-portlet"> |
| 118 | + <div id="specs-unknown"> |
| 119 | + <strong> |
| 120 | + <p tal:condition="blueprints_usage/enumvalue:EXTERNAL"> |
| 121 | + <tal:project replace="view/context/displayname" /> does not use Launchpad |
| 122 | + for planning or documentation. |
| 123 | + </p> |
| 124 | + <p tal:condition="blueprints_usage/enumvalue:NOT_APPLICABLE"> |
| 125 | + <tal:project replace="view/context/displayname" /> does not track |
| 126 | + use feature planning or documentation. |
| 127 | + </p> |
| 128 | + <p tal:condition="blueprints_usage/enumvalue:UNKNOWN"> |
| 129 | + Launchpad does not know how |
| 130 | + <tal:project replace="view/context/displayname" /> tracks feature |
| 131 | + planning or documentation. |
| 132 | + </p> |
| 133 | + </strong> |
| 134 | + |
| 135 | + <p id="wiki-fallback" |
| 136 | + tal:define="wiki view/context/wikiurl" |
| 137 | + tal:condition="wiki"> |
| 138 | + <tal:project replace="view/context/displayname" /> has a wiki, which |
| 139 | + may be used for feature plannning and documentation.<br /> |
| 140 | <a tal:attributes="href view/context/wikiurl"> |
| 141 | - <tal:project replace="view/context/displayname" /> wiki |
| 142 | + <tal:project replace="view/context/displayname" /> wiki |
| 143 | </a> |
| 144 | </p> |
| 145 | - |
| 146 | </div> |
| 147 | - <ul class="horizontal"> |
| 148 | - <li> |
| 149 | - <a class="info sprite" |
| 150 | - href="https://help.launchpad.net/BlueprintDocumentation"> |
| 151 | - Read more about tracking blueprints |
| 152 | - </a> |
| 153 | - </li> |
| 154 | - </ul> |
| 155 | - <ul class="horiztonal"> |
| 156 | - <li> |
| 157 | - <a tal:replace="structure context/menu:overview/configure_blueprints/fmt:link" /> |
| 158 | - </li> |
| 159 | - </ul> |
| 160 | + |
| 161 | + <p id="configure-support" |
| 162 | + tal:condition="view/can_configure_blueprints"> |
| 163 | + Launchpad Blueprints allow your project to track feature planning and |
| 164 | + documentation.<br /> |
| 165 | + <a class="info sprite" |
| 166 | + href="https://help.launchpad.net/BlueprintDocumentation"> |
| 167 | + Read more about tracking feature planning with blueprints</a><br /> |
| 168 | + <a tal:replace="structure context/menu:overview/configure_blueprints/fmt:link" /><br /> |
| 169 | + </p> |
| 170 | + </div> |
| 171 | </div> |
| 172 | </body> |
| 173 | </html> |
> Our goal is to communicate to the user where certain kinds of activities are
> performed--which site does the user need to visit to complete his goal.
> Launchpad blueprints is a branded service that provides specifications and
> documentation. The page needs to be talking about "specifications and
> documentation" because other sites, most often wikis, do not use the term
> blueprints.
Given this, I've updated the template to refer to the notion of documentation, and feature planning. In the instance where blueprints is set active, I've left the template alone, as the process of setting it up tells the user that Blueprints is a branded way that Launchpad does feature planning and documentation.
> We want to allow the users to provide a URL when they choose
> external (maybe repurpose the wiki field).
Based on our prior conversation I thought we were going to simply default to "There is a wiki for this project, which may be used for this purpose," with a link to the wiki if it exists, rather than introducing an extra db field to track an external specification tracking url.
I do think it may be worthwhile to circle back and add that field and get that information (for blueprints, answers, and translations) so we can provide a specific external place, but I think those changes are out of scope for this branch.
> The page is not really stating that
> Launchpad Blueprints allows the user to track specifications and
> documentation...I see no reason why I would turn the feature on since it does
> not have a sentence stating the basics of what it is for.
Agreed; I've updated what the page says to address this.
An incremental diff with these changes is attached.
I'll be addressing your other concerns re: specificationtarget next.