Merge lp:~jcsackett/launchpad/unknown-blueprints-service-597738 into lp:launchpad
| Status: | Merged |
|---|---|
| Approved by: | Curtis Hovey on 2010-09-15 |
| Approved revision: | no longer in the source branch. |
| Merged at revision: | 11571 |
| Proposed branch: | lp:~jcsackett/launchpad/unknown-blueprints-service-597738 |
| Merge into: | lp:launchpad |
| Prerequisite: | lp:~jcsackett/launchpad/deprecate-remaining-official-bools |
| Diff against target: |
914 lines (+464/-59) 13 files modified
lib/lp/blueprints/browser/configure.zcml (+4/-8) lib/lp/blueprints/browser/specificationtarget.py (+82/-7) lib/lp/blueprints/browser/tests/test_specificationtarget.py (+127/-3) lib/lp/blueprints/stories/blueprints/xx-creation.txt (+71/-29) lib/lp/blueprints/stories/blueprints/xx-productseries.txt (+12/-0) lib/lp/blueprints/stories/standalone/xx-batching.txt (+19/-5) lib/lp/blueprints/stories/standalone/xx-index.txt (+14/-0) lib/lp/blueprints/stories/standalone/xx-overview.txt (+11/-0) lib/lp/blueprints/stories/standalone/xx-views.txt (+21/-4) lib/lp/blueprints/templates/unknown-specs.pt (+93/-0) lib/lp/registry/browser/distribution.py (+7/-0) lib/lp/registry/browser/product.py (+1/-1) lib/lp/registry/browser/tests/pillar-views.txt (+2/-2) |
| To merge this branch: | bzr merge lp:~jcsackett/launchpad/unknown-blueprints-service-597738 |
| Related bugs: |
| Reviewer | Review Type | Date Requested | Status |
|---|---|---|---|
| Curtis Hovey (community) | ui | Approve on 2010-09-15 | |
| Guilherme Salgado (community) | ui* | 2010-09-09 | Approve on 2010-09-15 |
| Brad Crittenden (community) | code | Approve on 2010-09-10 | |
| Registry Administrators | ui | 2010-09-09 | Pending |
|
Review via email:
|
|||
Commit Message
Updates the blueprints service page to use the blueprints_usage enum and display relevant data for each setting.
Description of the Change
= Summary =
Updates the blueprints service to present relevant data for the UNKNOWN, EXTERNAL, and NOT_APPLICABLE settings of the blueprints_usage enum.
== Proposed fix ==
Modify the template and/or view to detect the usage status and present the correct template/info.
== Pre-implementation notes ==
Spoke with Curtis Hovey (sinzui) and Brad Crittenden (bac).
== Implementation details ==
The HasSpecificatio
The new template presents slightly different information based on the enum case, and provides some basic controls if relevant (e.g. configure blueprints if the user has edit permissions).
== Tests ==
bin/test -vvcm lp.blueprints.
== Demo and Q/A ==
In Launchpad.dev, signed in as admin, go to http://
Linting changed files:
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
lib/lp/
Lint output corrupted by merged dependent branch.
| j.c.sackett (jcsackett) wrote : | # |
| Brad Crittenden (bac) wrote : | # |
Hi Jon,
Thanks for working on this branch. I was unaware of the added
complexity of the whole template/
I like the work you've done here modulo the on-going discussion with
Gary about a better approach to the template swapping. I'm marking
the MP 'needs information' so I can track the change you come up with,
but the branch is very close.
You mentioned lint being confused by the dependency on another
branch. Even so, please ensure these are not for real:
./lib/lp/
251: E301 expected 1 blank line, found 0
254: E301 expected 1 blank line, found 0
./lib/lp/
38: Line has trailing whitespace.
./lib/lp/
582: source exceeds 78 characters.
584: source exceeds 78 characters.
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -15,6 +15,7 @@
>
> from operator import itemgetter
>
> +from z3c.ptcompat import ViewPageTemplat
> from zope.component import queryMultiAdapter
>
> from canonical.config import config
> @@ -32,6 +33,8 @@
> Link,
> )
> from canonical.
> +from lp.app.enums import service_
> +from lp.app.
> from lp.blueprints.
> SpecificationFi
> SpecificationSort,
> @@ -133,6 +136,50 @@
> is_sprint = False
> has_drivers = False
>
> + # Templates for the various conditions of blueprints:
> + # * On Launchpad
> + # * External
> + # * Disabled
> + # * Unknown
> + default_template = ViewPageTemplat
> + '../templates/
> + not_launchpad_
> + '../templates/
> +
> + @property
> + def template(self):
> + # If the template has been defined in the zcml, use that, as this
> + # isn't the base service page for blueprints.
> + if hasattr(self, 'index'):
As we discussed on IRC, you should use 'safe_hasattr' from
canonical.
see if there is a more obvious technique for this. Sadly I cannot
offer any other solution.
> + return super(HasSpecif
> +
> + # Sprints and Persons don't have a usage enum for blueprints, so we
> + # have to fallback to the default.
> + if (ISprint.
> + or IPerson.
> + return self.default_
> +
> + # ProjectGroups are a special case, as their products may be a
> + # combination of usage settings. Use the default, since it handles
> + # fetching anything that's set for ProjectGroups, and it's not correct
> + # to say anything about the ProjectGroup's usage.
> + if IProjectGroup.
> + return self.default_
> +
> + # If specific...
| Curtis Hovey (sinzui) wrote : | # |
On Fri, 2010-09-10 at 15:59 +0000, Brad Crittenden wrote:
> > + @property
> > + def template(self):
> > + # If the template has been defined in the zcml, use that,
> as this
> > + # isn't the base service page for blueprints.
> > + if hasattr(self, 'index'):
>
> As we discussed on IRC, you should use 'safe_hasattr' from
> canonical.
> see if there is a more obvious technique for this. Sadly I cannot
> offer any other solution.
We can abandon safe_hasattr when lpnet is on lucid. This issue was fixed
several years ago in pythong 2.6
| Brad Crittenden (bac) wrote : | # |
Good changes Jon. Please alphabetize the imports where you added safe_hasattr.
| Guilherme Salgado (salgado) wrote : | # |
The UI changes look good to me, although it's confusing that the new template (lib/lp/
| Curtis Hovey (sinzui) wrote : | # |
Gentleman, I think the reviewers and developers need to consider why we are making this change, and what we are really changing.
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. We want to allow the users to provide a URL when they choose external (maybe repurpose the wiki field). 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. Specification itself may be too technical. This app is essentially about planning and documenting features.
My other concern is about what has specifications--a SpecificationTa
jcsackett, Salgado, take a look at the branch again from the perspective of kinds of users who are interested in feature planning.
| j.c.sackett (jcsackett) wrote : | # |
I just discovered I missed comments within the diff; sorry for not replying on Friday.
> If the enum is wrong, how will the user get here? The tab will be
> disabled, so we assume they have a direct URL?
I'm not sure the tab will be disabled--if no blueprints existed, and the enum said "EXTERNAL" or something, you could still get to this view so the "${Pillar} does not use blueprints" could be displayed. In this instance if it's set to EXTERNAL but people have been using the blueprints, we continue to display the blueprints information when you get here.
The involvement menu link is disabled, but the tab along the top of the page is still there, and of course nothing stops someone from going to blueprints.
Now, I'm open to debate on whether we should just turn off the feature regardless of whether or not blueprints are already being used--I erred on the side of not allowing someone to just delete a bunch of work, but that might not be the decision we want to make here.
>> self.verify_
>
> You didn't introduce it, but please fix the typo of s/verify_
Uhm, not sure what you mean--looks like it already is verify_involvement?
>> def test_adaptable_
>> @@ -87,6 +92,63 @@
>> '<div id="involvement" class="portlet involvement">' in view())
>>
>>
>> +class TestHasSpecific
>> + """Tests the selection of templates based on blueprints usage."""
>> +
>> + layer = DatabaseFunctio
>> +
>> + def setUp(self):
>> + super(TestHasSp
>> + self.user = self.factory.
>> + self.product = self.factory.
>> + self.naked_product = removeSecurityP
>> + login_person(
>
> While I'm not a prude, I must ask why all of the nakedness here?
> Couldn't you just make self.user be the owner of the product and
> proceed fully clothed?
I wanted to test what a non-owner user has happen here; that may be the same thing as self.user, but it may not--I've been bitten by using owners/admins in the past.
> Why use 'specifications' just the once? The page says 'blueprints'
> six times and it is unclear that specifications and blueprints are the
> same from the context.
>
> Should we be more frank:
>
> <project> tracks specifications somewhere besides Launchpad, but we
> don't know where. You should check with the project maintainers for
> the location.
>
> I'm just throwing that out for thought. I'm not sure I even like the idea.
I'm playing with this right now related to the UI review; neither blueprints nor specifications are totally right for the copy, per Curtis's comments.
But consider your comments thrown into the mix.
> Thanks for the additional drive-by fixes.
I do what I can. :-P
| Curtis Hovey (sinzui) wrote : | # |
On Mon, 2010-09-13 at 19:54 +0000, j.c.sackett wrote:
>
> The involvement menu link is disabled, but the tab along the top of
> the page is still there, and of course nothing stops someone from
> going to blueprints.
>
> Now, I'm open to debate on whether we should just turn off the feature
> regardless of whether or not blueprints are already being used--I
> erred on the side of not allowing someone to just delete a bunch of
> work, but that might not be the decision we want to make here.
The complaint from project owners is that users created blueprints on
Launchpad instead of the upstream wiki. The project owner can enable
blueprints if he chooses to. Blueprints should be off until enabled.
I went to firefox in lp.dev and saw blueprints on. As the owner of the
project I decided to turn blueprints off, so I choses the configure
link, and was surprised that Blueprints was off. More over, Saving the
state of unknown did not disable blueprints. The state of the app does
not work like answers. Remember, we are working on other applications to
ensure all apps have the same kind of behaviours.
--
__Curtis C. Hovey_________
http://
| j.c.sackett (jcsackett) wrote : | # |
> The complaint from project owners is that users created blueprints on
> Launchpad instead of the upstream wiki. The project owner can enable
> blueprints if he chooses to. Blueprints should be off until enabled.
>
> I went to firefox in lp.dev and saw blueprints on. As the owner of the
> project I decided to turn blueprints off, so I choses the configure
> link, and was surprised that Blueprints was off. More over, Saving the
> state of unknown did not disable blueprints. The state of the app does
> not work like answers. Remember, we are working on other applications to
> ensure all apps have the same kind of behaviours.
I can buy this argument, totally. I've removed the conditional that prioritizes the existence of blueprints over the status of the enum.
| j.c.sackett (jcsackett) wrote : | # |
> 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.
| j.c.sackett (jcsackett) wrote : | # |
> My other concern is about what has specifications--a SpecificationTa
> reviewed the bugs branch a few days ago and saw only IProduct was supports
> well.
Valid; I've done some digging and have some explanations below.
> A BugTarget may be a product, projectgroup, distribution, and
> distributionsou
> everyone needs to visit an example of each to ensure the message is correct
> and the actions that can be takes are presented. eg, a project group's use of
> an app is implicit in the settings of its products.
So, I think products are now handled right.
Project groups are tricky--right now I have it defaulting to the old style, so that any blueprints made for any project will be shown. Given how the earlier question of whether or not to continue showing blueprints if has_any_
Distributions were mostly handled right; the issue with say, Kubuntu was the configure_
DSPs aren't valid for blueprints; it's always NOT_APPLICABLE and the links are all disabled. You can't even get to it by going to blueprints.
Attached is a diff of this round of changes.
| Curtis Hovey (sinzui) wrote : | # |
On Tue, 2010-09-14 at 19:16 +0000, j.c.sackett wrote:
> Project groups are tricky--right now I have it defaulting to the old
> style, so that any blueprints made for any project will be shown.
> Given how the earlier question of whether or not to continue showing
> blueprints if has_any_
> appropriate to only turn this on if at least one product has LAUNCHPAD
> as its usage setting.
I do not this this rule is right. PillarView that renders the
Involvement Portlet and the "Register a Blueprint" link only shows the
link on a project group if a sub project has enabled it. The involvement
portlet is controlled by the pillar owners and its links are enabled to
attack contributors to the applications the pillar uses.
We do not want the Involvement portlet to contradict the app page, which
is the reason we need to disable apps for unknown, not applicable, and
external (most of the time). Answers uses the involvement portlet to
decide the state to ensure it never contradicts. I expect all the apps
do do something similar so that we have a DRY implementation.
involvement = getMultiAdapter(
if service_
# enable the listing.
--
__Curtis C. Hovey_________
http://
| j.c.sackett (jcsackett) wrote : | # |
I like the multiAdapter strategy you showed and have put it in place of the for loop.
| Guilherme Salgado (salgado) wrote : | # |
Hi Jon,
This looks good, and I only have a couple suggestions.
s/use// @ "[...] does not track use feature planning or documentation."
And for bonus points you could change the 'Configure blueprints' text according to the state the context is in. For example, in the UNKNOWN state, it could be changed to "Tell us how <project> tracks feature planning and documentation". Similarly, for the EXTERNAL (and maybe NOT_APPLICABLE as well?) state, it could be "Use Launchpad to track feature planning and documentation", although that's a bit of a lie as there are other things you can do by following that link (but then it should be a worthy tradeoff if our goal is to get people to use blueprints).
| Curtis Hovey (sinzui) wrote : | # |
I think this is good to land, with the fixing of some link test. I suspect there are some other issues we will see when this feature is used with real data.
Thanks Salgado for seeing the problem in the enums. I see a similar issue with this link test: 'Enable tracking of specifications and meetings'. meetings, AKA sprints do not belong to projects or teams. They are always available because they are a cross-project/

Sorry about the bad update. Just in case of divergences between the prereq and this post update from devel, here's the diff:
=== modified file 'lib/lp/ blueprints/ browser/ configure. zcml' blueprints/ browser/ configure. zcml 2010-07-27 17:17:59 +0000 blueprints/ browser/ configure. zcml 2010-09-08 20:11:11 +0000
name= "+mdz-specs. csv"
attribute ="mdzCsv" />
<browser: page "../templates/ hasspecificatio ns-specs. pt"/>
<browser: page
name= "+portlet- latestspecs"
template= "../templates/ specificationta rget-portlet- latestspecs. pt"/>
class= "lp.blueprints. browser. specificationta rget.HasSpecifi cationsView"
permissio n="zope. Public" >
< browser: page "../templates/ hasspecificatio ns-specs. pt"/>
</browser: pages>
<browser: page
for= "lp.blueprints. interfaces. specificationta rget.ISpecifica tionGoal"
class= "lp.blueprints. browser. specificationta rget.HasSpecifi cationsView"
permissio n="zope. Public" >
< browser: page "../templates/ hasspecificatio ns-specs. pt"/>
< browser: page
name= "+portlet- latestspecs"
template= "../templates/ specificationta rget-portlet- latestspecs. pt"/>
facet= "specifications "
permission= "zope.Public" >
<browser: page "../templates/ hasspecificatio ns-specs. pt"/>
<browser: page
name= "+portlet- latestspecs"
template= "../templates/ specificationta rget-portlet- latestspecs. pt"/>
--- lib/lp/
+++ lib/lp/
@@ -53,8 +53,7 @@
- name="+specs"
- template=
+ name="+specs"/>
@@ -500,8 +499,7 @@
- name="+specs"
- template=
+ name="+specs"/>
@@ -532,8 +530,7 @@
- name="+specs"
- template=
+ name="+specs"/>
@@ -583,8 +580,7 @@
- name="+specs"
- template=
+ name="+specs"/>
=== modified file 'lib/lp/ blueprints/ browser/ specificationta rget.py' blueprints/ browser/ specificationta rget.py 2010-08-24 10:45:57 +0000 blueprints/ browser/ specificationta rget.py 2010-09-09 18:06:03 +0000
--- lib/lp/
+++ lib/lp/
@@ -15,6 +15,7 @@
from operator import itemgetter
+from z3c.ptcompat import ViewPageTemplat eFile
from zope.component import queryMultiAdapter
from canonical.config import config lazr.utils import smartquote uses_launchpad interfaces. launchpad import IServiceUsage interfaces. specification import ( ionFilter, ionSort,
@@ -32,6 +33,8 @@
Link,
)
from canonical.
+from lp.app.enums import service_
+from lp.app.
from lp.blueprints.
Specificat
Specificat
@@ -133,6 +136,50 @@
is_sprint = False
has_drivers = False
+ # Templates for the various conditions of blueprints: eFile( hasspecificatio ns-specs. pt') template = ViewPageTemplat eFile( unknown- specs.pt' )
+ # * On Launchpad
+ # * External
+ # * Disabled
+ # * Unknown
+ default_template = ViewPageTemplat
+ '../templates/
+ not_launchpad_
+ '../templates/
+
+ @property
+ def template(self):
+ # If the template has been defined in the zcml, use that, as t...