Merge lp:~linaro-infrastructure/launchpad/workitems-widget into lp:launchpad
- workitems-widget
- Merge into devel
Status: | Merged | ||||||||
---|---|---|---|---|---|---|---|---|---|
Approved by: | Curtis Hovey | ||||||||
Approved revision: | no longer in the source branch. | ||||||||
Merged at revision: | 14942 | ||||||||
Proposed branch: | lp:~linaro-infrastructure/launchpad/workitems-widget | ||||||||
Merge into: | lp:launchpad | ||||||||
Diff against target: |
934 lines (+669/-6) 12 files modified
lib/lp/blueprints/adapters.py (+3/-1) lib/lp/blueprints/browser/configure.zcml (+6/-0) lib/lp/blueprints/browser/specification.py (+21/-1) lib/lp/blueprints/configure.zcml (+2/-1) lib/lp/blueprints/help/workitems-help.html (+50/-0) lib/lp/blueprints/interfaces/specification.py (+19/-0) lib/lp/blueprints/model/specification.py (+40/-2) lib/lp/blueprints/model/tests/test_specification.py (+117/-0) lib/lp/blueprints/templates/specification-index.pt (+26/-1) lib/lp/blueprints/tests/test_webservice.py (+6/-0) lib/lp/services/fields/__init__.py (+102/-0) lib/lp/services/fields/tests/test_fields.py (+277/-0) |
||||||||
To merge this branch: | bzr merge lp:~linaro-infrastructure/launchpad/workitems-widget | ||||||||
Related bugs: |
|
Reviewer | Review Type | Date Requested | Status |
---|---|---|---|
j.c.sackett (community) | Approve | ||
Richard Harding (community) | code* | Needs Fixing | |
Huw Wilkins | ui | Pending | |
Review via email: mp+94790@code.launchpad.net |
Commit message
[r=jcsackett,
Description of the change
Hi,
This branch adds a text area for editing Work Items, similar to the blueprint Whiteboard. The input will be parsed and verified to conform to the format assumed today by status.ubuntu.com and status.linaro.org. Additionally some validation is done to ensure that valid lp accounts are used as assignees as well as milestones that are valid targets for the specification.
The format of this text area is:
--
This is blocked on something: BLOCKED
This is already done: DONE
[bob] Something Bob should do: TODO
Work items for ubuntu-12.04:
This will be worked on later: TODO
--
Thanks,
Mattias
Guilherme Salgado (salgado) wrote : | # |
James Tunnicliffe (dooferlad) wrote : | # |
I think the whiteboard being first is nice, but as you say, it may cause people to miss the new box. How about a highlighted message to users about work items having moved that shows up above the whiteboard? It could be designed so that the user can hide the message and it sets a cookie so they don't see it again. The message can be deleted from the UI once sufficient time has passed that most users are using the two boxes in the new way.
Richard Harding (rharding) wrote : | # |
Thanks, this looks ok. I'd like to suggest a couple of tweaks, but they're mostly optional preference kinds of things.
I think it'd be better to move the compiled regex to module level constants so that it only gets compiled once when the app starts. They're not going to change after that.
Is there any reason you did regex matching for the parse method, but break the string apart manually in the parseLine method? It seems you'd be able to have two possible regexes (with and without [assignee]) to toss at it and you could grab out the title, status, assignee via the regex groups?
The nice thing about moving those outside like that and as regexes is that it's be easy to toss a quick run through of strings in tests to make for a very clear/quick view of potential issues matching and easily updated in the future as someone thinks of something that breaks it.
I'd also like to see the parseLine broken into parsing and validating. Again, it makes it easier to test the parser and the validation bits separately. Right now validation errors can come from parseLine, getAssignee, getMilestone.
Mattias Backman (mabac) wrote : | # |
> Thanks, this looks ok. I'd like to suggest a couple of tweaks, but they're
> mostly optional preference kinds of things.
>
> I think it'd be better to move the compiled regex to module level constants so
> that it only gets compiled once when the app starts. They're not going to
> change after that.
Absolutely, done.
>
> Is there any reason you did regex matching for the parse method, but break the
> string apart manually in the parseLine method? It seems you'd be able to have
The main reason, apart from snipping the code from work-items-tracker was to provide detailed error messages if parsing failed.
> two possible regexes (with and without [assignee]) to toss at it and you could
> grab out the title, status, assignee via the regex groups?
I tried that and still kept the ability to give detailed feedback. The code got cleaner but the re is pretty unreadable. I believe that there are enough tests to catch if someone ever messes it up though.
>
> The nice thing about moving those outside like that and as regexes is that
> it's be easy to toss a quick run through of strings in tests to make for a
> very clear/quick view of potential issues matching and easily updated in the
> future as someone thinks of something that breaks it.
I didn't change the tests to use the re directly, but I can do that too if you'd like that.
>
> I'd also like to see the parseLine broken into parsing and validating. Again,
> it makes it easier to test the parser and the validation bits separately.
> Right now validation errors can come from parseLine, getAssignee,
> getMilestone.
I split out the status validation to getStatus() so parseLine is now only re matching and checking the title, all parsing. I still raise LaunchpadValida
j.c.sackett (jcsackett) wrote : | # |
This looks pretty good. I have a few questions and nitpicks in the diff below.
> === modified file 'lib/lp/
> --- lib/lp/
> +++ lib/lp/
> @@ -179,6 +184,98 @@
> self.assertEqua
> self.assertEqua
>
> + def test_workitems_
> + spec = self.factory.
> + self.assertEqua
> +
> + def test_workitems_
> + work_item = self.factory.
> + self.assertEqua
> +
> + def test_workitems_
> + work_item = self.factory.
> + self.assertWork
> +
> + def test_workitems_
> + work_item1 = self.factory.
> + status=
> + work_item2 = self.factory.
> + specification=
> + status=
> + work_item3 = self.factory.
> + specification=
> + status=
> + work_item4 = self.factory.
> + specification=
> + status=
> + work_item5 = self.factory.
> + specification=
> + status=
> + work_items = [work_item1, work_item2, work_item3, work_item4, work_item5]
> + self.assertWork
This and other tests would read clearer if you first made a spec with
factory.
factory.
> + def test_workitems_
> + spec = self.factory.
> + milestone = self.factory.
> + login_person(
> + work_item = spec.newWorkItem(
> + title=u'
> + status=
> + milestone=
> + expected_wi_text = ("Work items for %s:\n"
> + "%s: TODO" % \
> + (milestone.name, work_item.title))
> + self.assertEqua
> +
> + def test_workitems_
> + spec = self.factory.
> + milestone = self.factory.
> + login_person(
> + work_item1 = spec.newWorkItem(
> + ...
Mattias Backman (mabac) wrote : | # |
> This looks pretty good. I have a few questions and nitpicks in the diff below.
>
>
> > === modified file 'lib/lp/
> > --- lib/lp/
> 04:24:19 +0000
> > +++ lib/lp/
> 13:48:22 +0000
> > @@ -179,6 +184,98 @@
> > self.assertEqua
> > self.assertEqua
> >
> > + def test_workitems_
> > + spec = self.factory.
> > + self.assertEqua
> > +
> > + def test_workitems_
> > + work_item = self.factory.
> > + self.assertEqua
> > +
> > + def test_workitems_
> > + work_item = self.factory.
> > + self.assertWork
> [work_item])
> > +
> > + def test_workitems_
> > + work_item1 = self.factory.
> > + status=
> > + work_item2 = self.factory.
> > + specification=
> > + status=
> > + work_item3 = self.factory.
> > + specification=
> > + status=
> > + work_item4 = self.factory.
> > + specification=
> > + status=
> > + work_item5 = self.factory.
> > + specification=
> > + status=
> > + work_items = [work_item1, work_item2, work_item3, work_item4,
> work_item5]
> > + self.assertWork
> work_items)
>
> This and other tests would read clearer if you first made a spec with
> factory.
> factory.
I've changed that, hope I didn't miss any tests that can be improved.
>
> > + def test_workitems_
> > + spec = self.factory.
> > + milestone = self.factory.
> > + login_person(
> > + work_item = spec.newWorkItem(
> > + title=u'
> > + status=
> > + milestone=
> > + expected_wi_text = ("Work items for %s:\n"
> > + "%s: TODO" % \
> > + (milestone.name, work_item.title))
> > + self.assertEqua
> > +
> > + def test_workitems_
> >...
j.c.sackett (jcsackett) wrote : | # |
Looks good, thanks.
Guilherme Salgado (salgado) wrote : | # |
Great to see this is ready. Just wanted to note that we should wait until the migration script has been QAed before we land this. That way we can turn the script on as soon as this is deployed.
Another option would be to guard the new UI with the same feature flag used in the script.
Guilherme Salgado (salgado) wrote : | # |
I've just noticed the widget we use for work-items-text doesn't display validation errors anywhere, so if the user enters unparseable/invalid data it will fail without giving the user any clue about what's wrong. This is a bug present in all text-editing widgets in Launchpad (e.g. the tags widget in a bug page will not tell you what's wrong if you enter a tag with upper-case letters), though, so it's something we'll need to fix in a separate branch.
Guilherme Salgado (salgado) wrote : | # |
There's a bug report for that already: bug 415365
Huw Wilkins (huwshimi) wrote : | # |
From my cursory look into how people use the whiteboard there is usually some kind of information before people list the work items (but not on all). With the work items field first this switches that pattern around.
There is also a pattern which usually goes, information about this item, then related information. In this case we have Blueprint information, details/discussion of the Blueprint (whiteboard) and lastly work items. I think the order of work items and whiteboard in a sense depends on what kind of information the whiteboard is being used for (not helpful, I know).
Lastly we have importance of information. As there can potentially be a lot of information for both the whiteboard and work items we may have to prioritise the information that is most important for the user to be able to see upfront.
It would seem like the order should be whiteboard then work items, but with some clarification on the importance of work items and the way whiteboards are used then we might discover which order is best.
As James says a message about the new field for work items may help if work items are last. I would suggest showing that message when the whiteboard is in edit mode.
Hope that helps somewhat. Sorry I can't be more concrete with my answer.
Mattias Backman (mabac) wrote : | # |
> From my cursory look into how people use the whiteboard there is usually some
> kind of information before people list the work items (but not on all). With
> the work items field first this switches that pattern around.
>
> There is also a pattern which usually goes, information about this item, then
> related information. In this case we have Blueprint information,
> details/discussion of the Blueprint (whiteboard) and lastly work items. I
> think the order of work items and whiteboard in a sense depends on what kind
> of information the whiteboard is being used for (not helpful, I know).
>
> Lastly we have importance of information. As there can potentially be a lot of
> information for both the whiteboard and work items we may have to prioritise
> the information that is most important for the user to be able to see upfront.
>
> It would seem like the order should be whiteboard then work items, but with
> some clarification on the importance of work items and the way whiteboards are
> used then we might discover which order is best.
>
> As James says a message about the new field for work items may help if work
> items are last. I would suggest showing that message when the whiteboard is in
> edit mode.
I have switched the order of the text fields so that the Whiteboard is on top. I also added an informational message which is hidden by default and is shown by some js when the editor is shown. When the editor is hidden, the message goes away.
>
> Hope that helps somewhat. Sorry I can't be more concrete with my answer.
Richard Harding (rharding) wrote : | # |
Mattias, if we're going to add support in the widget for informational messages I think it would need to get through a UI review of how to display and such. We'd want to fully implement it as the error message is done. Since this is the first case that needs this, I'd almost rather tack it on at the moment.
You can implement your message in just this location with the following diff:
=== modified file 'lib/lp/
--- lib/lp/
+++ lib/lp/
@@ -325,7 +325,7 @@
</div>
<script type="text/
- LPJS.use('lp.anim', 'lp.ui', function(Y) {
+ LPJS.use('lp.anim', 'lp.ui', 'node', 'widget' function(Y) {
Y.on('lp:context:implementation
var icon = Y.one('
@@ -366,6 +366,24 @@
});
+ // Watch for the whiteboard for edit mode so we can show/hide a
+ // message to the user to make sure not to put work items in there.
+ var whiteboard_node = Y.one('
+ var whiteboard = Y.Widget.
+ var notice_node = Y.Node.
+ notice_
+ notice_
+ notice_
+ whiteboard.
+ // If we're visible, show the message
+ if (ev.newVal) {
+ var par = whiteboard_
+ par.insertBefor
+ } else {
+ // Otherwise we need to remove the node
+ notice_
+ }
+ });
});
</script>
For the long term, please submit a bug for adding proper support for informational messages to the inlineedit widget. We can look at the right way to support it from there.
Mattias Backman (mabac) wrote : | # |
> Mattias, if we're going to add support in the widget for informational
> messages I think it would need to get through a UI review of how to display
> and such. We'd want to fully implement it as the error message is done. Since
> this is the first case that needs this, I'd almost rather tack it on at the
> moment.
>
> You can implement your message in just this location with the following diff:
Cool thanks! I used your code and just did a minor tweak which seems to work better when showing/hiding the editor multiple times.
>
>
> === modified file 'lib/lp/
> --- lib/lp/
> +0000
> +++ lib/lp/
> +0000
> @@ -325,7 +325,7 @@
> </div>
>
> <script type="text/
> - LPJS.use('lp.anim', 'lp.ui', function(Y) {
> + LPJS.use('lp.anim', 'lp.ui', 'node', 'widget' function(Y) {
>
> Y.on('lp:context:implementation
> var icon = Y.one('
> @@ -366,6 +366,24 @@
> window.
> });
>
> + // Watch for the whiteboard for edit mode so we can show/hide a
> + // message to the user to make sure not to put work items in there.
> + var whiteboard_node = Y.one('
> + var whiteboard = Y.Widget.
> + var notice_node = Y.Node.
> + notice_
> + notice_
> + notice_
> separate Work Items input field below.');
> + whiteboard.
> + // If we're visible, show the message
> + if (ev.newVal) {
> + var par = whiteboard_
> + par.insertBefor
> + } else {
> + // Otherwise we need to remove the node
> + notice_
> + }
> + });
> });
> </script>
>
>
> For the long term, please submit a bug for adding proper support for
> informational messages to the inlineedit widget. We can look at the right way
> to support it from there.
Sure, this is the bug: https:/
j.c.sackett (jcsackett) wrote : | # |
These changes look fine to me. Thanks for factoring all of these notes into your branch, Mattias.
Guilherme Salgado (salgado) wrote : | # |
Matsubara is now hammering this to see if he finds any bugs we haven't found yet. We also need to work out a plan to notify people about the BPs we won't be able to migrate and an announcement for the UI changes. Once that's done we can land this.
In the meantime I'm merging lp:~salgado/launchpad/workitems-widget-help-popup in here because here's where it belongs (and it has been approved already)
Mattias Backman (mabac) wrote : | # |
I have added a few revisions to fix a bug; we actually had changed the format a little by not inserting a milestone header for the default milestone (which is inherited from the spec).
14859 is approved elsewhere.
14861-14863 adds the default milestone header.
j.c.sackett (jcsackett) wrote : | # |
A cleaner diff of what the last few revisions contain (removing updates from devel and the popup branch) can be found at http://
j.c.sackett (jcsackett) wrote : | # |
Based on that diff the new revisions look fine.
Preview Diff
1 | === modified file 'lib/lp/blueprints/adapters.py' |
2 | --- lib/lp/blueprints/adapters.py 2010-07-30 12:56:27 +0000 |
3 | +++ lib/lp/blueprints/adapters.py 2012-03-08 16:42:47 +0000 |
4 | @@ -18,12 +18,14 @@ |
5 | summary=None, whiteboard=None, specurl=None, productseries=None, |
6 | distroseries=None, milestone=None, name=None, priority=None, |
7 | definition_status=None, target=None, bugs_linked=None, |
8 | - bugs_unlinked=None, approver=None, assignee=None, drafter=None): |
9 | + bugs_unlinked=None, approver=None, assignee=None, drafter=None, |
10 | + workitems_text=None): |
11 | self.specification = specification |
12 | self.user = user |
13 | self.title = title |
14 | self.summary = summary |
15 | self.whiteboard = whiteboard |
16 | + self.workitems_text = workitems_text |
17 | self.specurl = specurl |
18 | self.productseries = productseries |
19 | self.distroseries = distroseries |
20 | |
21 | === modified file 'lib/lp/blueprints/browser/configure.zcml' |
22 | --- lib/lp/blueprints/browser/configure.zcml 2012-02-17 04:09:06 +0000 |
23 | +++ lib/lp/blueprints/browser/configure.zcml 2012-03-08 16:42:47 +0000 |
24 | @@ -373,6 +373,12 @@ |
25 | permission="launchpad.AnyPerson" |
26 | template="../templates/specification-edit.pt"/> |
27 | <browser:page |
28 | + name="+workitems" |
29 | + for="lp.blueprints.interfaces.specification.ISpecification" |
30 | + class="lp.blueprints.browser.specification.SpecificationEditWorkItemsView" |
31 | + permission="launchpad.AnyPerson" |
32 | + template="../templates/specification-edit.pt"/> |
33 | + <browser:page |
34 | name="+people" |
35 | for="lp.blueprints.interfaces.specification.ISpecification" |
36 | class="lp.blueprints.browser.specification.SpecificationEditPeopleView" |
37 | |
38 | === modified file 'lib/lp/blueprints/browser/specification.py' |
39 | --- lib/lp/blueprints/browser/specification.py 2012-03-01 20:00:29 +0000 |
40 | +++ lib/lp/blueprints/browser/specification.py 2012-03-08 16:42:47 +0000 |
41 | @@ -21,6 +21,7 @@ |
42 | 'SpecificationEditStatusView', |
43 | 'SpecificationEditView', |
44 | 'SpecificationEditWhiteboardView', |
45 | + 'SpecificationEditWorkItemsView', |
46 | 'SpecificationGoalDecideView', |
47 | 'SpecificationGoalProposeView', |
48 | 'SpecificationLinkBranchView', |
49 | @@ -411,7 +412,7 @@ |
50 | |
51 | usedfor = ISpecification |
52 | links = ['edit', 'people', 'status', 'priority', |
53 | - 'whiteboard', 'proposegoal', |
54 | + 'whiteboard', 'proposegoal', 'workitems', |
55 | 'milestone', 'requestfeedback', 'givefeedback', 'subscription', |
56 | 'addsubscriber', |
57 | 'linkbug', 'unlinkbug', 'linkbranch', |
58 | @@ -521,6 +522,11 @@ |
59 | return Link('+whiteboard', text, icon='edit') |
60 | |
61 | @enabled_with_permission('launchpad.AnyPerson') |
62 | + def workitems(self): |
63 | + text = 'Edit work items' |
64 | + return Link('+workitems', text, icon='edit') |
65 | + |
66 | + @enabled_with_permission('launchpad.AnyPerson') |
67 | def linkbranch(self): |
68 | if self.context.linked_branches.count() > 0: |
69 | text = 'Link to another branch' |
70 | @@ -648,6 +654,14 @@ |
71 | hide_empty=False) |
72 | |
73 | @property |
74 | + def workitems_text_widget(self): |
75 | + """The Work Items text as a widget.""" |
76 | + return TextAreaEditorWidget( |
77 | + self.context, ISpecification['workitems_text'], title="Work Items", |
78 | + edit_view='+workitems', edit_title='Edit work items', |
79 | + hide_empty=False) |
80 | + |
81 | + @property |
82 | def direction_widget(self): |
83 | return BooleanChoiceWidget( |
84 | self.context, ISpecification['direction_approved'], |
85 | @@ -710,6 +724,12 @@ |
86 | custom_widget('whiteboard', TextAreaWidget, height=15) |
87 | |
88 | |
89 | +class SpecificationEditWorkItemsView(SpecificationEditView): |
90 | + label = 'Edit specification work items' |
91 | + field_names = ['workitems_text'] |
92 | + custom_widget('workitems_text', TextAreaWidget, height=15) |
93 | + |
94 | + |
95 | class SpecificationEditPeopleView(SpecificationEditView): |
96 | label = 'Change the people involved' |
97 | field_names = ['assignee', 'drafter', 'approver', 'whiteboard'] |
98 | |
99 | === modified file 'lib/lp/blueprints/configure.zcml' |
100 | --- lib/lp/blueprints/configure.zcml 2012-02-10 17:32:41 +0000 |
101 | +++ lib/lp/blueprints/configure.zcml 2012-03-08 16:42:47 +0000 |
102 | @@ -181,7 +181,8 @@ |
103 | <require |
104 | permission="launchpad.AnyPerson" |
105 | attributes="linkBug |
106 | - unlinkBug"/> |
107 | + unlinkBug |
108 | + setWorkItems"/> |
109 | </class> |
110 | |
111 | <class class="lp.blueprints.model.specificationbug.SpecificationBug"> |
112 | |
113 | === added file 'lib/lp/blueprints/help/workitems-help.html' |
114 | --- lib/lp/blueprints/help/workitems-help.html 1970-01-01 00:00:00 +0000 |
115 | +++ lib/lp/blueprints/help/workitems-help.html 2012-03-08 16:42:47 +0000 |
116 | @@ -0,0 +1,50 @@ |
117 | +<html> |
118 | + <head> |
119 | + <title>Blueprint work items</title> |
120 | + <link rel="stylesheet" type="text/css" |
121 | + href="/+icing/yui/cssreset/reset.css" /> |
122 | + <link rel="stylesheet" type="text/css" |
123 | + href="/+icing/yui/cssfonts/fonts.css" /> |
124 | + <link rel="stylesheet" type="text/css" |
125 | + href="/+icing/yui/cssbase/base.css" /> |
126 | + </head> |
127 | + <body> |
128 | + <h1>Using work items</h1> |
129 | + |
130 | + <p>Often, it can take a few separate steps to complete the work described |
131 | + in a blueprint. Launchpad lets you track these steps in the "Work items" |
132 | + text box.</p> |
133 | + |
134 | + <h2>Describing work items</h2> |
135 | + |
136 | + <p>It's easy to track the steps, or work items, necessary to complete the |
137 | + blueprint. Using the <em>Work items</em> text box, give a short |
138 | + description of each work item along with its status. For example:</p> |
139 | + |
140 | + <pre> |
141 | + Design the UI: DONE |
142 | + Test the UI: TODO |
143 | + Bootstrap the dev environment: POSTPONED |
144 | + </pre> |
145 | + |
146 | + <p>Each work item goes on its own line, followed by a colon and its |
147 | + status.</p> |
148 | + |
149 | + <h2>Work item statuses</h2> |
150 | + |
151 | + <p>Each work item can have one of four statuses:</p> |
152 | + |
153 | + <ul> |
154 | + <li>TODO</li> |
155 | + <li>INPROGRESS</li> |
156 | + <li>DONE</li> |
157 | + <li>POSTPONED</li> |
158 | + </ul> |
159 | + |
160 | + <h2>More about work items</h2> |
161 | + |
162 | + <p>There's <a href="https://help.launchpad.net/WorkItems" |
163 | + target="_blank">more about using work items</a> in the Launchpad help |
164 | + wiki.</p> |
165 | + </body> |
166 | +</html> |
167 | |
168 | === modified file 'lib/lp/blueprints/interfaces/specification.py' |
169 | --- lib/lp/blueprints/interfaces/specification.py 2012-02-29 13:27:51 +0000 |
170 | +++ lib/lp/blueprints/interfaces/specification.py 2012-03-08 16:42:47 +0000 |
171 | @@ -79,6 +79,7 @@ |
172 | PublicPersonChoice, |
173 | Summary, |
174 | Title, |
175 | + WorkItemsText, |
176 | ) |
177 | from lp.services.webapp import canonical_url |
178 | from lp.services.webapp.menu import structured |
179 | @@ -303,6 +304,13 @@ |
180 | "Any notes on the status of this spec you would like to " |
181 | "make. Your changes will override the current text.")), |
182 | as_of="devel") |
183 | + workitems_text = exported( |
184 | + WorkItemsText( |
185 | + title=_('Work Items'), required=False, readonly=True, |
186 | + description=_( |
187 | + "Work items for this specification input in a text format. " |
188 | + "Your changes will override the current work items.")), |
189 | + as_of="devel") |
190 | direction_approved = exported( |
191 | Bool(title=_('Basic direction approved?'), |
192 | required=True, default=False, |
193 | @@ -616,6 +624,16 @@ |
194 | |
195 | export_as_webservice_entry(as_of="beta") |
196 | |
197 | + @mutator_for(ISpecificationPublic['workitems_text']) |
198 | + @operation_parameters(new_work_items=WorkItemsText()) |
199 | + @export_write_operation() |
200 | + @operation_for_version('devel') |
201 | + def setWorkItems(new_work_items): |
202 | + """Set work items on this specification. |
203 | + |
204 | + :param new_work_items: Work items to set. |
205 | + """ |
206 | + |
207 | @operation_parameters( |
208 | bug=Reference(schema=Interface)) # Really IBug |
209 | @export_write_operation() |
210 | @@ -694,6 +712,7 @@ |
211 | title = Attribute("The spec title or None.") |
212 | summary = Attribute("The spec summary or None.") |
213 | whiteboard = Attribute("The spec whiteboard or None.") |
214 | + workitems_text = Attribute("The spec work items as text or None.") |
215 | specurl = Attribute("The URL to the spec home page (not in Launchpad).") |
216 | productseries = Attribute("The product series.") |
217 | distroseries = Attribute("The series to which this is targeted.") |
218 | |
219 | === modified file 'lib/lp/blueprints/model/specification.py' |
220 | --- lib/lp/blueprints/model/specification.py 2012-02-29 13:03:19 +0000 |
221 | +++ lib/lp/blueprints/model/specification.py 2012-03-08 16:42:47 +0000 |
222 | @@ -224,6 +224,40 @@ |
223 | self._subscriptions, key=lambda sub: person_sort_key(sub.person)) |
224 | |
225 | @property |
226 | + def workitems_text(self): |
227 | + """See ISpecification.""" |
228 | + workitems_lines = [] |
229 | + |
230 | + def get_header_text(milestone): |
231 | + if milestone is None: |
232 | + return "Work items:" |
233 | + else: |
234 | + return "Work items for %s:" % milestone.name |
235 | + |
236 | + if self.work_items.count() == 0: |
237 | + return '' |
238 | + milestone = self.work_items[0].milestone |
239 | + # Start by appending a header for the milestone of the first work |
240 | + # item. After this we're going to write a new header whenever we see a |
241 | + # work item with a different milestone. |
242 | + workitems_lines.append(get_header_text(milestone)) |
243 | + for work_item in self.work_items: |
244 | + if work_item.milestone != milestone: |
245 | + workitems_lines.append("") |
246 | + milestone = work_item.milestone |
247 | + workitems_lines.append(get_header_text(milestone)) |
248 | + assignee = work_item.assignee |
249 | + if assignee is not None: |
250 | + assignee_part = "[%s] " % assignee.name |
251 | + else: |
252 | + assignee_part = "" |
253 | + # work_items are ordered by sequence |
254 | + workitems_lines.append("%s%s: %s" % (assignee_part, |
255 | + work_item.title, |
256 | + work_item.status.name)) |
257 | + return "\n".join(workitems_lines) |
258 | + |
259 | + @property |
260 | def target(self): |
261 | """See ISpecification.""" |
262 | if self.product: |
263 | @@ -249,6 +283,10 @@ |
264 | SpecificationWorkItem, specification=self, |
265 | deleted=False).order_by("sequence") |
266 | |
267 | + def setWorkItems(self, new_work_items): |
268 | + field = ISpecification['workitems_text'].bind(self) |
269 | + self.updateWorkItems(field.parseAndValidate(new_work_items)) |
270 | + |
271 | def _deleteWorkItemsNotMatching(self, titles): |
272 | """Delete all work items whose title does not match the given ones. |
273 | |
274 | @@ -570,7 +608,7 @@ |
275 | "distroseries", "milestone")) |
276 | delta.recordNewAndOld(("name", "priority", "definition_status", |
277 | "target", "approver", "assignee", "drafter", |
278 | - "whiteboard")) |
279 | + "whiteboard", "workitems_text")) |
280 | delta.recordListAddedAndRemoved("bugs", |
281 | "bugs_linked", |
282 | "bugs_unlinked") |
283 | @@ -1035,7 +1073,7 @@ |
284 | |
285 | def new(self, name, title, specurl, summary, definition_status, |
286 | owner, approver=None, product=None, distribution=None, assignee=None, |
287 | - drafter=None, whiteboard=None, |
288 | + drafter=None, whiteboard=None, workitems_text=None, |
289 | priority=SpecificationPriority.UNDEFINED): |
290 | """See ISpecificationSet.""" |
291 | # Adapt the NewSpecificationDefinitionStatus item to a |
292 | |
293 | === modified file 'lib/lp/blueprints/model/tests/test_specification.py' |
294 | --- lib/lp/blueprints/model/tests/test_specification.py 2012-02-28 04:24:19 +0000 |
295 | +++ lib/lp/blueprints/model/tests/test_specification.py 2012-03-08 16:42:47 +0000 |
296 | @@ -17,6 +17,7 @@ |
297 | SpecificationWorkItemStatus, |
298 | ) |
299 | from lp.blueprints.model.specificationworkitem import SpecificationWorkItem |
300 | +from lp.registry.model.milestone import Milestone |
301 | from lp.services.webapp import canonical_url |
302 | from lp.testing import ( |
303 | ANONYMOUS, |
304 | @@ -152,6 +153,31 @@ |
305 | |
306 | layer = DatabaseFunctionalLayer |
307 | |
308 | + def setUp(self): |
309 | + super(TestSpecificationWorkItems, self).setUp() |
310 | + self.wi_header = self.factory.makeMilestone( |
311 | + name='none-milestone-as-header') |
312 | + |
313 | + def assertWorkItemsTextContains(self, spec, items): |
314 | + expected_lines = [] |
315 | + for item in items: |
316 | + if isinstance(item, SpecificationWorkItem): |
317 | + line = '' |
318 | + if item.assignee is not None: |
319 | + line = "[%s] " % item.assignee.name |
320 | + expected_lines.append(u"%s%s: %s" % (line, item.title, |
321 | + item.status.name)) |
322 | + else: |
323 | + self.assertIsInstance(item, Milestone) |
324 | + if expected_lines != []: |
325 | + expected_lines.append(u"") |
326 | + if item == self.wi_header: |
327 | + expected_lines.append(u"Work items:") |
328 | + else: |
329 | + expected_lines.append(u"Work items for %s:" % item.name) |
330 | + expected = "\n".join(expected_lines) |
331 | + self.assertEqual(expected, spec.workitems_text) |
332 | + |
333 | def test_anonymous_newworkitem_not_allowed(self): |
334 | spec = self.factory.makeSpecification() |
335 | login(ANONYMOUS) |
336 | @@ -179,6 +205,97 @@ |
337 | self.assertEqual(title, work_item.title) |
338 | self.assertEqual(milestone, work_item.milestone) |
339 | |
340 | + def test_workitems_text_no_workitems(self): |
341 | + spec = self.factory.makeSpecification() |
342 | + self.assertEqual('', spec.workitems_text) |
343 | + |
344 | + def test_workitems_text_deleted_workitem(self): |
345 | + work_item = self.factory.makeSpecificationWorkItem(deleted=True) |
346 | + self.assertEqual('', work_item.specification.workitems_text) |
347 | + |
348 | + def test_workitems_text_single_workitem(self): |
349 | + work_item = self.factory.makeSpecificationWorkItem() |
350 | + self.assertWorkItemsTextContains(work_item.specification, |
351 | + [self.wi_header, work_item]) |
352 | + |
353 | + def test_workitems_text_multi_workitems_all_statuses(self): |
354 | + spec = self.factory.makeSpecification() |
355 | + work_item1 = self.factory.makeSpecificationWorkItem(specification=spec, |
356 | + status=SpecificationWorkItemStatus.TODO) |
357 | + work_item2 = self.factory.makeSpecificationWorkItem(specification=spec, |
358 | + status=SpecificationWorkItemStatus.DONE) |
359 | + work_item3 = self.factory.makeSpecificationWorkItem(specification=spec, |
360 | + status=SpecificationWorkItemStatus.POSTPONED) |
361 | + work_item4 = self.factory.makeSpecificationWorkItem(specification=spec, |
362 | + status=SpecificationWorkItemStatus.INPROGRESS) |
363 | + work_item5 = self.factory.makeSpecificationWorkItem(specification=spec, |
364 | + status=SpecificationWorkItemStatus.BLOCKED) |
365 | + work_items = [self.wi_header, work_item1, work_item2, work_item3, |
366 | + work_item4, work_item5] |
367 | + self.assertWorkItemsTextContains(spec, work_items) |
368 | + |
369 | + def test_workitems_text_with_milestone(self): |
370 | + spec = self.factory.makeSpecification() |
371 | + milestone = self.factory.makeMilestone(product=spec.product) |
372 | + login_person(spec.owner) |
373 | + work_item = self.factory.makeSpecificationWorkItem(specification=spec, |
374 | + title=u'new-work-item', |
375 | + status=SpecificationWorkItemStatus.TODO, |
376 | + milestone=milestone) |
377 | + items = [milestone, work_item] |
378 | + self.assertWorkItemsTextContains(spec, items) |
379 | + |
380 | + def test_workitems_text_with_implicit_and_explicit_milestone(self): |
381 | + spec = self.factory.makeSpecification() |
382 | + milestone = self.factory.makeMilestone(product=spec.product) |
383 | + login_person(spec.owner) |
384 | + work_item1 = self.factory.makeSpecificationWorkItem(specification=spec, |
385 | + title=u'Work item with default milestone', |
386 | + status=SpecificationWorkItemStatus.TODO, |
387 | + milestone=None) |
388 | + work_item2 = self.factory.makeSpecificationWorkItem(specification=spec, |
389 | + title=u'Work item with set milestone', |
390 | + status=SpecificationWorkItemStatus.TODO, |
391 | + milestone=milestone) |
392 | + items = [self.wi_header, work_item1, milestone, work_item2] |
393 | + self.assertWorkItemsTextContains(spec, items) |
394 | + |
395 | + def test_workitems_text_with_implicit_and_explicit_milestone_reversed(self): |
396 | + spec = self.factory.makeSpecification() |
397 | + milestone = self.factory.makeMilestone(product=spec.product) |
398 | + login_person(spec.owner) |
399 | + work_item1 = self.factory.makeSpecificationWorkItem(specification=spec, |
400 | + title=u'Work item with set milestone', |
401 | + status=SpecificationWorkItemStatus.TODO, |
402 | + milestone=milestone) |
403 | + work_item2 = self.factory.makeSpecificationWorkItem(specification=spec, |
404 | + title=u'Work item with default milestone', |
405 | + status=SpecificationWorkItemStatus.TODO, |
406 | + milestone=None) |
407 | + items = [milestone, work_item1, self.wi_header, work_item2] |
408 | + self.assertWorkItemsTextContains(spec, items) |
409 | + |
410 | + def test_workitems_text_with_different_milestones(self): |
411 | + spec = self.factory.makeSpecification() |
412 | + milestone1 = self.factory.makeMilestone(product=spec.product) |
413 | + milestone2 = self.factory.makeMilestone(product=spec.product) |
414 | + login_person(spec.owner) |
415 | + work_item1 = self.factory.makeSpecificationWorkItem(specification=spec, |
416 | + title=u'Work item with first milestone', |
417 | + status=SpecificationWorkItemStatus.TODO, |
418 | + milestone=milestone1) |
419 | + work_item2 = self.factory.makeSpecificationWorkItem(specification=spec, |
420 | + title=u'Work item with second milestone', |
421 | + status=SpecificationWorkItemStatus.TODO, |
422 | + milestone=milestone2) |
423 | + items = [milestone1, work_item1, milestone2, work_item2] |
424 | + self.assertWorkItemsTextContains(spec, items) |
425 | + |
426 | + def test_workitems_text_with_assignee(self): |
427 | + assignee = self.factory.makePerson() |
428 | + work_item = self.factory.makeSpecificationWorkItem(assignee=assignee) |
429 | + self.assertWorkItemsTextContains(work_item.specification, [self.wi_header, work_item]) |
430 | + |
431 | def test_work_items_property(self): |
432 | spec = self.factory.makeSpecification() |
433 | wi1 = self.factory.makeSpecificationWorkItem( |
434 | |
435 | === modified file 'lib/lp/blueprints/templates/specification-index.pt' |
436 | --- lib/lp/blueprints/templates/specification-index.pt 2012-02-01 15:31:32 +0000 |
437 | +++ lib/lp/blueprints/templates/specification-index.pt 2012-03-08 16:42:47 +0000 |
438 | @@ -284,6 +284,12 @@ |
439 | </div> |
440 | |
441 | <div class="portlet"> |
442 | + <a href="/+help-blueprints/workitems-help.html" target="help" class="sprite maybe"> |
443 | + <span class="invisible-link">Tag help</span></a> |
444 | + <div class="wide" tal:content="structure view/workitems_text_widget" /> |
445 | + </div> |
446 | + |
447 | + <div class="portlet"> |
448 | <tal:deptree condition="view/has_dep_tree"> |
449 | <h2>Dependency tree</h2> |
450 | |
451 | @@ -320,7 +326,7 @@ |
452 | </div> |
453 | |
454 | <script type="text/javascript"> |
455 | - LPJS.use('lp.anim', 'lp.ui', function(Y) { |
456 | + LPJS.use('lp.anim', 'lp.ui', 'node', 'widget', function(Y) { |
457 | |
458 | Y.on('lp:context:implementation_status:changed', function(e) { |
459 | var icon = Y.one('#informational-icon'); |
460 | @@ -361,6 +367,25 @@ |
461 | window.document.title = title; |
462 | }); |
463 | |
464 | + // Watch for the whiteboard for edit mode so we can show/hide a |
465 | + // message to the user to make sure not to put work items in there. |
466 | + var whiteboard_node = Y.one('#edit-whiteboard'); |
467 | + var whiteboard = Y.Widget.getByNode(whiteboard_node); |
468 | + var notice_node = Y.Node.create('<p/>'); |
469 | + notice_node.set('id', 'wimessage'); |
470 | + notice_node.addClass('informational message'); |
471 | + notice_node.setContent('Please note that work items go in the separate Work Items input field below.'); |
472 | + whiteboard.editor.on('visibleChange', function (ev) { |
473 | + var par = whiteboard_node.get('parentNode'); |
474 | + // If we're visible, show the message |
475 | + if (ev.newVal) { |
476 | + par.insertBefore(notice_node, whiteboard_node); |
477 | + } else { |
478 | + // Otherwise we need to remove the node |
479 | + par.removeChild(notice_node) |
480 | + } |
481 | + }); |
482 | + |
483 | }); |
484 | </script> |
485 | |
486 | |
487 | === modified file 'lib/lp/blueprints/tests/test_webservice.py' |
488 | --- lib/lp/blueprints/tests/test_webservice.py 2012-01-01 02:58:52 +0000 |
489 | +++ lib/lp/blueprints/tests/test_webservice.py 2012-03-08 16:42:47 +0000 |
490 | @@ -143,6 +143,12 @@ |
491 | spec_webservice = self.getSpecOnWebservice(spec) |
492 | self.assertEqual(spec.whiteboard, spec_webservice.whiteboard) |
493 | |
494 | + def test_representation_contains_workitems(self): |
495 | + work_item = self.factory.makeSpecificationWorkItem() |
496 | + spec_webservice = self.getSpecOnWebservice(work_item.specification) |
497 | + self.assertEqual(work_item.specification.workitems_text, |
498 | + spec_webservice.workitems_text) |
499 | + |
500 | def test_representation_contains_milestone(self): |
501 | product = self.factory.makeProduct() |
502 | productseries = self.factory.makeProductSeries(product=product) |
503 | |
504 | === modified file 'lib/lp/services/fields/__init__.py' |
505 | --- lib/lp/services/fields/__init__.py 2012-02-16 00:38:53 +0000 |
506 | +++ lib/lp/services/fields/__init__.py 2012-03-08 16:42:47 +0000 |
507 | @@ -53,6 +53,7 @@ |
508 | 'URIField', |
509 | 'UniqueField', |
510 | 'Whiteboard', |
511 | + 'WorkItemsText', |
512 | 'is_public_person_or_closed_team', |
513 | 'is_public_person', |
514 | ] |
515 | @@ -102,12 +103,18 @@ |
516 | name_validator, |
517 | valid_name, |
518 | ) |
519 | +from lp.blueprints.enums import SpecificationWorkItemStatus |
520 | from lp.bugs.errors import InvalidDuplicateValue |
521 | from lp.registry.interfaces.pillar import IPillarNameSet |
522 | from lp.services.webapp.interfaces import ILaunchBag |
523 | |
524 | # Marker object to tell BaseImageUpload to keep the existing image. |
525 | KEEP_SAME_IMAGE = object() |
526 | +# Regexp for detecting milestone headers in work items text. |
527 | +MILESTONE_RE = re.compile('^work items(.*)\s*:\s*$', re.I) |
528 | +# Regexp for work items. |
529 | +WORKITEM_RE = re.compile( |
530 | + '^(\[(?P<assignee>.*)\])?\s*(?P<title>.*)\s*:\s*(?P<status>.*)\s*$', re.I) |
531 | |
532 | |
533 | # Field Interfaces |
534 | @@ -858,3 +865,98 @@ |
535 | else: |
536 | # The vocabulary prevents the revealing of private team names. |
537 | raise PrivateTeamNotAllowed(value) |
538 | + |
539 | + |
540 | +class WorkItemsText(Text): |
541 | + |
542 | + def parseLine(self, line): |
543 | + workitem_match = WORKITEM_RE.search(line) |
544 | + if workitem_match: |
545 | + assignee = workitem_match.group('assignee') |
546 | + title = workitem_match.group('title') |
547 | + status = workitem_match.group('status') |
548 | + else: |
549 | + raise LaunchpadValidationError( |
550 | + 'Invalid work item format: "%s"' % line) |
551 | + if title == '': |
552 | + raise LaunchpadValidationError( |
553 | + 'No work item title found on "%s"' % line) |
554 | + if title.startswith('['): |
555 | + raise LaunchpadValidationError( |
556 | + 'Missing closing "]" for assignee on "%s".' % line) |
557 | + |
558 | + return {'title': title, 'status': status.strip().upper(), |
559 | + 'assignee': assignee} |
560 | + |
561 | + def parse(self, text): |
562 | + sequence = 0 |
563 | + milestone = None |
564 | + work_items = [] |
565 | + for line in text.splitlines(): |
566 | + if line.strip() == '': |
567 | + continue |
568 | + milestone_match = MILESTONE_RE.search(line) |
569 | + if milestone_match: |
570 | + milestone_part = milestone_match.group(1).strip() |
571 | + if milestone_part == '': |
572 | + milestone = None |
573 | + else: |
574 | + milestone = milestone_part.split()[-1] |
575 | + else: |
576 | + new_work_item = self.parseLine(line) |
577 | + new_work_item['milestone'] = milestone |
578 | + new_work_item['sequence'] = sequence |
579 | + sequence += 1 |
580 | + work_items.append(new_work_item) |
581 | + return work_items |
582 | + |
583 | + def validate(self, value): |
584 | + self.parseAndValidate(value) |
585 | + |
586 | + def parseAndValidate(self, text): |
587 | + work_items = self.parse(text) |
588 | + for work_item in work_items: |
589 | + work_item['status'] = self.getStatus(work_item['status']) |
590 | + work_item['assignee'] = self.getAssignee(work_item['assignee']) |
591 | + work_item['milestone'] = self.getMilestone(work_item['milestone']) |
592 | + return work_items |
593 | + |
594 | + def getStatus(self, text): |
595 | + valid_statuses = SpecificationWorkItemStatus.items |
596 | + if text.lower() not in [item.name.lower() for item in valid_statuses]: |
597 | + raise LaunchpadValidationError('Unknown status: %s' % text) |
598 | + return valid_statuses[text.upper()] |
599 | + |
600 | + def getAssignee(self, assignee_name): |
601 | + if assignee_name is None: |
602 | + return None |
603 | + from lp.registry.interfaces.person import IPersonSet |
604 | + assignee = getUtility(IPersonSet).getByName(assignee_name) |
605 | + if assignee is None: |
606 | + raise LaunchpadValidationError("Unknown person name: %s" % assignee_name) |
607 | + return assignee |
608 | + |
609 | + def getMilestone(self, milestone_name): |
610 | + if milestone_name is None: |
611 | + return None |
612 | + |
613 | + target = self.context.target |
614 | + |
615 | + milestone = None |
616 | + from lp.registry.interfaces.distribution import IDistribution |
617 | + from lp.registry.interfaces.milestone import IMilestoneSet |
618 | + from lp.registry.interfaces.product import IProduct |
619 | + if IProduct.providedBy(target): |
620 | + milestone = getUtility(IMilestoneSet).getByNameAndProduct( |
621 | + milestone_name, target) |
622 | + elif IDistribution.providedBy(target): |
623 | + milestone = getUtility(IMilestoneSet).getByNameAndDistribution( |
624 | + milestone_name, target) |
625 | + else: |
626 | + raise AssertionError("Unexpected target type.") |
627 | + |
628 | + if milestone is None: |
629 | + raise LaunchpadValidationError("The milestone '%s' is not valid " |
630 | + "for the target '%s'." % \ |
631 | + (milestone_name, target.name)) |
632 | + return milestone |
633 | |
634 | === modified file 'lib/lp/services/fields/tests/test_fields.py' |
635 | --- lib/lp/services/fields/tests/test_fields.py 2012-01-01 02:58:52 +0000 |
636 | +++ lib/lp/services/fields/tests/test_fields.py 2012-03-08 16:42:47 +0000 |
637 | @@ -14,6 +14,7 @@ |
638 | from zope.schema.interfaces import TooShort |
639 | |
640 | from lp.app.validators import LaunchpadValidationError |
641 | +from lp.blueprints.enums import SpecificationWorkItemStatus |
642 | from lp.registry.interfaces.nameblacklist import INameBlacklistSet |
643 | from lp.registry.interfaces.person import ( |
644 | CLOSED_TEAM_POLICY, |
645 | @@ -26,6 +27,7 @@ |
646 | FormattableDate, |
647 | is_public_person_or_closed_team, |
648 | StrippableText, |
649 | + WorkItemsText, |
650 | ) |
651 | from lp.testing import ( |
652 | login_person, |
653 | @@ -108,6 +110,281 @@ |
654 | self.assertEqual(None, field.validate(u' a ')) |
655 | |
656 | |
657 | +class TestWorkItemsTextValidation(TestCaseWithFactory): |
658 | + |
659 | + layer = DatabaseFunctionalLayer |
660 | + |
661 | + def setUp(self): |
662 | + super(TestWorkItemsTextValidation, self).setUp() |
663 | + self.field = WorkItemsText(__name__='test') |
664 | + |
665 | + def test_parseandvalidate(self): |
666 | + status = SpecificationWorkItemStatus.TODO |
667 | + assignee = self.factory.makePerson() |
668 | + milestone = self.factory.makeMilestone() |
669 | + title = 'A work item' |
670 | + specification = self.factory.makeSpecification( |
671 | + product=milestone.product) |
672 | + field = self.field.bind(specification) |
673 | + work_items_text = ("Work items for %s:\n" |
674 | + "[%s]%s: %s" % (milestone.name, assignee.name, title, |
675 | + status.name)) |
676 | + work_item = field.parseAndValidate(work_items_text)[0] |
677 | + self.assertEqual({'assignee': assignee, |
678 | + 'milestone': milestone, |
679 | + 'sequence': 0, |
680 | + 'status': status, |
681 | + 'title': title}, work_item) |
682 | + |
683 | + def test_unknown_assignee_is_rejected(self): |
684 | + person_name = 'test-person' |
685 | + self.assertRaises( |
686 | + LaunchpadValidationError, self.field.getAssignee, person_name) |
687 | + |
688 | + def test_validate_valid_assignee(self): |
689 | + assignee = self.factory.makePerson() |
690 | + self.assertEqual(assignee, self.field.getAssignee(assignee.name)) |
691 | + |
692 | + def test_validate_unset_assignee(self): |
693 | + self.assertIs(None, self.field.getAssignee(None)) |
694 | + |
695 | + def test_validate_unset_milestone(self): |
696 | + self.assertIs(None, self.field.getMilestone(None)) |
697 | + |
698 | + def test_validate_unknown_milestone(self): |
699 | + specification = self.factory.makeSpecification() |
700 | + field = self.field.bind(specification) |
701 | + self.assertRaises( |
702 | + LaunchpadValidationError, field.getMilestone, 'does-not-exist') |
703 | + |
704 | + def test_validate_valid_product_milestone(self): |
705 | + milestone = self.factory.makeMilestone() |
706 | + specification = self.factory.makeSpecification( |
707 | + product=milestone.product) |
708 | + field = self.field.bind(specification) |
709 | + self.assertEqual(milestone, field.getMilestone(milestone.name)) |
710 | + |
711 | + def test_validate_valid_distro_milestone(self): |
712 | + distro = self.factory.makeDistribution() |
713 | + milestone = self.factory.makeMilestone(distribution=distro) |
714 | + specification = self.factory.makeSpecification( |
715 | + distribution=milestone.distribution) |
716 | + field = self.field.bind(specification) |
717 | + self.assertEqual(milestone, field.getMilestone(milestone.name)) |
718 | + |
719 | + def test_validate_invalid_milestone(self): |
720 | + milestone_name = 'test-milestone' |
721 | + self.factory.makeMilestone(name=milestone_name) |
722 | + # Milestone exists but is not a target for this spec. |
723 | + specification = self.factory.makeSpecification(product=None) |
724 | + field = self.field.bind(specification) |
725 | + self.assertRaises( |
726 | + LaunchpadValidationError, field.getMilestone, milestone_name) |
727 | + |
728 | + def test_validate_invalid_status(self): |
729 | + self.assertRaises( |
730 | + LaunchpadValidationError, self.field.getStatus, |
731 | + 'Invalid status: FOO') |
732 | + |
733 | + def test_validate_valid_statuses(self): |
734 | + statuses = [SpecificationWorkItemStatus.TODO, |
735 | + SpecificationWorkItemStatus.DONE, |
736 | + SpecificationWorkItemStatus.POSTPONED, |
737 | + SpecificationWorkItemStatus.INPROGRESS, |
738 | + SpecificationWorkItemStatus.BLOCKED] |
739 | + for status in statuses: |
740 | + validated_status = self.field.getStatus(status.name) |
741 | + self.assertEqual(validated_status, status) |
742 | + |
743 | + |
744 | +class TestWorkItemsText(TestCase): |
745 | + |
746 | + def setUp(self): |
747 | + super(TestWorkItemsText, self).setUp() |
748 | + self.field = WorkItemsText(__name__='test') |
749 | + |
750 | + def test_validate_raises_LaunchpadValidationError(self): |
751 | + self.assertRaises( |
752 | + LaunchpadValidationError, self.field.validate, |
753 | + 'This is not a valid work item.') |
754 | + |
755 | + def test_single_line_parsing(self): |
756 | + work_items_title = 'Test this work item' |
757 | + parsed = self.field.parseLine('%s: TODO' % (work_items_title)) |
758 | + self.assertEqual(parsed['title'], work_items_title) |
759 | + self.assertEqual(parsed['status'], 'TODO') |
760 | + |
761 | + def test_url_and_colon_in_title(self): |
762 | + work_items_title = 'Test this: which is a url: http://www.linaro.org/' |
763 | + parsed = self.field.parseLine('%s: TODO' % (work_items_title)) |
764 | + self.assertEqual(parsed['title'], work_items_title) |
765 | + |
766 | + def test_silly_caps_status_parsing(self): |
767 | + parsed_upper = self.field.parseLine('Test this work item: TODO ') |
768 | + self.assertEqual(parsed_upper['status'], 'TODO') |
769 | + parsed_lower = self.field.parseLine('Test this work item: todo') |
770 | + self.assertEqual(parsed_lower['status'], 'TODO') |
771 | + parsed_camel = self.field.parseLine('Test this work item: ToDo') |
772 | + self.assertEqual(parsed_camel['status'], 'TODO') |
773 | + |
774 | + def test_parseLine_without_status_fails(self): |
775 | + # We should require an explicit status to avoid the problem of work |
776 | + # items with a url but no status. |
777 | + self.assertRaises( |
778 | + LaunchpadValidationError, self.field.parseLine, |
779 | + 'Missing status') |
780 | + |
781 | + def test_parseLine_without_title_fails(self): |
782 | + self.assertRaises( |
783 | + LaunchpadValidationError, self.field.parseLine, |
784 | + ':TODO') |
785 | + |
786 | + def test_parseLine_without_title_with_assignee_fails(self): |
787 | + self.assertRaises( |
788 | + LaunchpadValidationError, self.field.parseLine, |
789 | + '[test-person] :TODO') |
790 | + |
791 | + def test_multi_line_parsing(self): |
792 | + title_1 = 'Work item 1' |
793 | + title_2 = 'Work item 2' |
794 | + work_items_text = "Work items:\n%s: TODO\n%s: POSTPONED" % (title_1, |
795 | + title_2) |
796 | + parsed = self.field.parse(work_items_text) |
797 | + self.assertEqual( |
798 | + parsed, [{'title': title_1, |
799 | + 'status': 'TODO', |
800 | + 'assignee': None, 'milestone': None, 'sequence': 0}, |
801 | + {'title': title_2, |
802 | + 'status': 'POSTPONED', |
803 | + 'assignee': None, 'milestone': None, 'sequence': 1}]) |
804 | + |
805 | + def test_multi_line_parsing_different_milestones(self): |
806 | + title_1 = 'Work item 1' |
807 | + title_2 = 'Work item 2' |
808 | + work_items_text = ("Work items:\n%s: TODO\nWork items for test-ms:\n" |
809 | + "%s: POSTPONED" % (title_1, title_2)) |
810 | + parsed = self.field.parse(work_items_text) |
811 | + self.assertEqual( |
812 | + parsed, [{'title': title_1, |
813 | + 'status': 'TODO', |
814 | + 'assignee': None, 'milestone': None, 'sequence': 0}, |
815 | + {'title': title_2, |
816 | + 'status': 'POSTPONED', |
817 | + 'assignee': None, 'milestone': 'test-ms', 'sequence': 1}]) |
818 | + |
819 | + def test_multi_line_parsing_different_milestones_reversed(self): |
820 | + title_1 = 'Work item 1' |
821 | + title_2 = 'Work item 2' |
822 | + work_items_text = ("Work items for test-ms:\n%s: TODO\nWork items:\n" |
823 | + "%s: POSTPONED" % (title_1, title_2)) |
824 | + parsed = self.field.parse(work_items_text) |
825 | + self.assertEqual( |
826 | + parsed, [{'title': title_1, |
827 | + 'status': 'TODO', |
828 | + 'assignee': None, 'milestone': 'test-ms', 'sequence': 0}, |
829 | + {'title': title_2, |
830 | + 'status': 'POSTPONED', |
831 | + 'assignee': None, 'milestone': None, 'sequence': 1}]) |
832 | + |
833 | + def test_parse_assignee(self): |
834 | + title = 'Work item 1' |
835 | + assignee = 'test-person' |
836 | + work_items_text = "[%s]%s: TODO" % (assignee, title) |
837 | + parsed = self.field.parseLine(work_items_text) |
838 | + self.assertEqual(parsed['assignee'], assignee) |
839 | + |
840 | + def test_parse_assignee_with_space(self): |
841 | + title = 'Work item 1' |
842 | + assignee = 'test-person' |
843 | + work_items_text = "[%s] %s: TODO" % (assignee, title) |
844 | + parsed = self.field.parseLine(work_items_text) |
845 | + self.assertEqual(parsed['assignee'], assignee) |
846 | + |
847 | + def test_parseLine_with_missing_closing_bracket_for_assignee(self): |
848 | + self.assertRaises( |
849 | + LaunchpadValidationError, self.field.parseLine, |
850 | + "[test-person A single work item: TODO") |
851 | + |
852 | + def test_parse_empty_lines_have_no_meaning(self): |
853 | + parsed = self.field.parse("\n\n\n\n\n\n\n\n") |
854 | + self.assertEqual(parsed, []) |
855 | + |
856 | + def test_parse_status(self): |
857 | + work_items_text = "A single work item: TODO" |
858 | + parsed = self.field.parse(work_items_text) |
859 | + self.assertEqual(parsed[0]['status'], 'TODO') |
860 | + |
861 | + def test_parse_milestone(self): |
862 | + milestone = '2012.02' |
863 | + title = "Work item for a milestone" |
864 | + work_items_text = "Work items for %s:\n%s: TODO" % (milestone, title) |
865 | + parsed = self.field.parse(work_items_text) |
866 | + self.assertEqual(parsed, [{'title': title, |
867 | + 'status': 'TODO', |
868 | + 'assignee': None, 'milestone': milestone, 'sequence': 0}]) |
869 | + |
870 | + def test_parse_multi_milestones(self): |
871 | + milestone_1 = '2012.02' |
872 | + milestone_2 = '2012.03' |
873 | + title_1 = "Work item for a milestone" |
874 | + title_2 = "Work item for a later milestone" |
875 | + work_items_text = ("Work items for %s:\n%s: POSTPONED\n\nWork items " |
876 | + "for %s:\n%s: TODO" % (milestone_1, title_1, |
877 | + milestone_2, title_2)) |
878 | + parsed = self.field.parse(work_items_text) |
879 | + self.assertEqual(parsed, |
880 | + [{'title': title_1, |
881 | + 'status': 'POSTPONED', |
882 | + 'assignee': None, 'milestone': milestone_1, |
883 | + 'sequence': 0}, |
884 | + {'title': title_2, |
885 | + 'status': 'TODO', |
886 | + 'assignee': None, 'milestone': milestone_2, |
887 | + 'sequence': 1}]) |
888 | + |
889 | + def test_parse_orphaned_work_items(self): |
890 | + # Work items not in a milestone block belong to the latest specified |
891 | + # milestone. |
892 | + milestone_1 = '2012.02' |
893 | + milestone_2 = '2012.03' |
894 | + title_1 = "Work item for a milestone" |
895 | + title_2 = "Work item for a later milestone" |
896 | + title_3 = "A work item preceeded by a blank line" |
897 | + work_items_text = ( |
898 | + "Work items for %s:\n%s: POSTPONED\n\nWork items for %s:\n%s: " |
899 | + "TODO\n\n%s: TODO" % (milestone_1, title_1, milestone_2, title_2, |
900 | + title_3)) |
901 | + parsed = self.field.parse(work_items_text) |
902 | + self.assertEqual(parsed, |
903 | + [{'title': title_1, |
904 | + 'status': 'POSTPONED', |
905 | + 'assignee': None, 'milestone': milestone_1, |
906 | + 'sequence': 0}, |
907 | + {'title': title_2, |
908 | + 'status': 'TODO', |
909 | + 'assignee': None, 'milestone': milestone_2, |
910 | + 'sequence': 1}, |
911 | + {'title': title_3, |
912 | + 'status': 'TODO', |
913 | + 'assignee': None, 'milestone': milestone_2, |
914 | + 'sequence': 2}]) |
915 | + |
916 | + def test_sequence_single_workitem(self): |
917 | + parsed = self.field.parse("A single work item: TODO") |
918 | + self.assertEqual(0, parsed[0]['sequence']) |
919 | + |
920 | + def test_only_workitems_get_sequence(self): |
921 | + parsed = self.field.parse("A single work item: TODO\n" |
922 | + "A second work item: TODO\n" |
923 | + "\n" |
924 | + "Work items for 2012.02:\n" |
925 | + "Work item for a milestone: TODO\n") |
926 | + self.assertEqual([(wi['title'], wi['sequence']) for wi in parsed], |
927 | + [("A single work item", 0), ("A second work item", 1), |
928 | + ("Work item for a milestone", 2)]) |
929 | + |
930 | + |
931 | + |
932 | class TestBlacklistableContentNameField(TestCaseWithFactory): |
933 | |
934 | layer = DatabaseFunctionalLayer |
I'm asking Huw to review this mostly to get feedback on the position of the new field on the page... should it be before the whiteboard or after it? Having it before makes it less likely for people to miss it and enter work-items on the whiteboard, but there may be reasons to have it after the whiteboard as well.