Merge lp:~linaro-infrastructure/launchpad/workitems-widget into lp:launchpad

Proposed by Mattias Backman
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
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,rharding][bug=940450] New UI for editing Blueprint work items

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

To post a comment you must log in.
Revision history for this message
Guilherme Salgado (salgado) wrote :

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.

Revision history for this message
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.

Revision history for this message
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.

review: Approve (code*)
Revision history for this message
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 LaunchpadValidationError from the parsing code, so please let me know if something else is expected.

Revision history for this message
j.c.sackett (jcsackett) wrote :
Download full text (7.5 KiB)

This looks pretty good. I have a few questions and nitpicks in the diff below.

> === modified file 'lib/lp/blueprints/model/tests/test_specification.py'
> --- lib/lp/blueprints/model/tests/test_specification.py 2012-02-28 04:24:19 +0000
> +++ lib/lp/blueprints/model/tests/test_specification.py 2012-03-01 13:48:22 +0000
> @@ -179,6 +184,98 @@
> self.assertEqual(title, work_item.title)
> self.assertEqual(milestone, work_item.milestone)
>
> + def test_workitems_text_no_workitems(self):
> + spec = self.factory.makeSpecification()
> + self.assertEqual('', spec.workitems_text)
> +
> + def test_workitems_text_deleted_workitem(self):
> + work_item = self.factory.makeSpecificationWorkItem(deleted=True)
> + self.assertEqual('', work_item.specification.workitems_text)
> +
> + def test_workitems_text_single_workitem(self):
> + work_item = self.factory.makeSpecificationWorkItem()
> + self.assertWorkItemsTextContains(work_item.specification, [work_item])
> +
> + def test_workitems_text_multi_workitems_all_statuses(self):
> + work_item1 = self.factory.makeSpecificationWorkItem(
> + status=SpecificationWorkItemStatus.TODO)
> + work_item2 = self.factory.makeSpecificationWorkItem(
> + specification=work_item1.specification,
> + status=SpecificationWorkItemStatus.DONE)
> + work_item3 = self.factory.makeSpecificationWorkItem(
> + specification=work_item1.specification,
> + status=SpecificationWorkItemStatus.POSTPONED)
> + work_item4 = self.factory.makeSpecificationWorkItem(
> + specification=work_item1.specification,
> + status=SpecificationWorkItemStatus.INPROGRESS)
> + work_item5 = self.factory.makeSpecificationWorkItem(
> + specification=work_item1.specification,
> + status=SpecificationWorkItemStatus.BLOCKED)
> + work_items = [work_item1, work_item2, work_item3, work_item4, work_item5]
> + self.assertWorkItemsTextContains(work_item1.specification, work_items)

This and other tests would read clearer if you first made a spec with
factory.makeSpecification() and then passed that in to each call for
factory.makeSpecificationWorkItem()

> + def test_workitems_text_with_milestone(self):
> + spec = self.factory.makeSpecification()
> + milestone = self.factory.makeMilestone(product=spec.product)
> + login_person(spec.owner)
> + work_item = spec.newWorkItem(
> + title=u'new-work-item', sequence=0,
> + status=SpecificationWorkItemStatus.TODO,
> + milestone=milestone)
> + expected_wi_text = ("Work items for %s:\n"
> + "%s: TODO" % \
> + (milestone.name, work_item.title))
> + self.assertEqual(expected_wi_text, spec.workitems_text)
> +
> + def test_workitems_text_with_implicit_and_explicit_milestone(self):
> + spec = self.factory.makeSpecification()
> + milestone = self.factory.makeMilestone(product=spec.product)
> + login_person(spec.owner)
> + work_item1 = spec.newWorkItem(
> + ...

Read more...

review: Needs Information
Revision history for this message
Mattias Backman (mabac) wrote :
Download full text (8.0 KiB)

> This looks pretty good. I have a few questions and nitpicks in the diff below.
>
>
> > === modified file 'lib/lp/blueprints/model/tests/test_specification.py'
> > --- lib/lp/blueprints/model/tests/test_specification.py 2012-02-28
> 04:24:19 +0000
> > +++ lib/lp/blueprints/model/tests/test_specification.py 2012-03-01
> 13:48:22 +0000
> > @@ -179,6 +184,98 @@
> > self.assertEqual(title, work_item.title)
> > self.assertEqual(milestone, work_item.milestone)
> >
> > + def test_workitems_text_no_workitems(self):
> > + spec = self.factory.makeSpecification()
> > + self.assertEqual('', spec.workitems_text)
> > +
> > + def test_workitems_text_deleted_workitem(self):
> > + work_item = self.factory.makeSpecificationWorkItem(deleted=True)
> > + self.assertEqual('', work_item.specification.workitems_text)
> > +
> > + def test_workitems_text_single_workitem(self):
> > + work_item = self.factory.makeSpecificationWorkItem()
> > + self.assertWorkItemsTextContains(work_item.specification,
> [work_item])
> > +
> > + def test_workitems_text_multi_workitems_all_statuses(self):
> > + work_item1 = self.factory.makeSpecificationWorkItem(
> > + status=SpecificationWorkItemStatus.TODO)
> > + work_item2 = self.factory.makeSpecificationWorkItem(
> > + specification=work_item1.specification,
> > + status=SpecificationWorkItemStatus.DONE)
> > + work_item3 = self.factory.makeSpecificationWorkItem(
> > + specification=work_item1.specification,
> > + status=SpecificationWorkItemStatus.POSTPONED)
> > + work_item4 = self.factory.makeSpecificationWorkItem(
> > + specification=work_item1.specification,
> > + status=SpecificationWorkItemStatus.INPROGRESS)
> > + work_item5 = self.factory.makeSpecificationWorkItem(
> > + specification=work_item1.specification,
> > + status=SpecificationWorkItemStatus.BLOCKED)
> > + work_items = [work_item1, work_item2, work_item3, work_item4,
> work_item5]
> > + self.assertWorkItemsTextContains(work_item1.specification,
> work_items)
>
> This and other tests would read clearer if you first made a spec with
> factory.makeSpecification() and then passed that in to each call for
> factory.makeSpecificationWorkItem()

I've changed that, hope I didn't miss any tests that can be improved.

>
> > + def test_workitems_text_with_milestone(self):
> > + spec = self.factory.makeSpecification()
> > + milestone = self.factory.makeMilestone(product=spec.product)
> > + login_person(spec.owner)
> > + work_item = spec.newWorkItem(
> > + title=u'new-work-item', sequence=0,
> > + status=SpecificationWorkItemStatus.TODO,
> > + milestone=milestone)
> > + expected_wi_text = ("Work items for %s:\n"
> > + "%s: TODO" % \
> > + (milestone.name, work_item.title))
> > + self.assertEqual(expected_wi_text, spec.workitems_text)
> > +
> > + def test_workitems_text_with_implicit_and_explicit_milestone(self):
> >...

Read more...

Revision history for this message
j.c.sackett (jcsackett) wrote :

Looks good, thanks.

review: Approve
Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
Guilherme Salgado (salgado) wrote :

There's a bug report for that already: bug 415365

Revision history for this message
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.

Revision history for this message
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.

Revision history for this message
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/blueprints/templates/specification-index.pt'
--- lib/lp/blueprints/templates/specification-index.pt 2012-03-02 15:14:14 +0000
+++ lib/lp/blueprints/templates/specification-index.pt 2012-03-02 18:15:07 +0000
@@ -325,7 +325,7 @@
     </div>

   <script type="text/javascript">
- LPJS.use('lp.anim', 'lp.ui', function(Y) {
+ LPJS.use('lp.anim', 'lp.ui', 'node', 'widget' function(Y) {

         Y.on('lp:context:implementation_status:changed', function(e) {
             var icon = Y.one('#informational-icon');
@@ -366,6 +366,24 @@
             window.document.title = title;
         });

+ // 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('#edit-whiteboard');
+ var whiteboard = Y.Widget.getByNode(whiteboard_node);
+ var notice_node = Y.Node.create('<p/>');
+ notice_node.set('id', 'wimessage');
+ notice_node.addClass('informational message');
+ notice_node.setContent('Please note that work items go in the separate Work Items input field below.');
+ whiteboard.editor.on('visibleChange', function (ev) {
+ // If we're visible, show the message
+ if (ev.newVal) {
+ var par = whiteboard_node.get('parentNode');
+ par.insertBefore(notice_node, whiteboard_node);
+ } else {
+ // Otherwise we need to remove the node
+ notice_node.remove(notice_node)
+ }
+ });
       });
     </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.

review: Needs Fixing (code*)
Revision history for this message
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/blueprints/templates/specification-index.pt'
> --- lib/lp/blueprints/templates/specification-index.pt 2012-03-02 15:14:14
> +0000
> +++ lib/lp/blueprints/templates/specification-index.pt 2012-03-02 18:15:07
> +0000
> @@ -325,7 +325,7 @@
> </div>
>
> <script type="text/javascript">
> - LPJS.use('lp.anim', 'lp.ui', function(Y) {
> + LPJS.use('lp.anim', 'lp.ui', 'node', 'widget' function(Y) {
>
> Y.on('lp:context:implementation_status:changed', function(e) {
> var icon = Y.one('#informational-icon');
> @@ -366,6 +366,24 @@
> window.document.title = title;
> });
>
> + // 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('#edit-whiteboard');
> + var whiteboard = Y.Widget.getByNode(whiteboard_node);
> + var notice_node = Y.Node.create('<p/>');
> + notice_node.set('id', 'wimessage');
> + notice_node.addClass('informational message');
> + notice_node.setContent('Please note that work items go in the
> separate Work Items input field below.');
> + whiteboard.editor.on('visibleChange', function (ev) {
> + // If we're visible, show the message
> + if (ev.newVal) {
> + var par = whiteboard_node.get('parentNode');
> + par.insertBefore(notice_node, whiteboard_node);
> + } else {
> + // Otherwise we need to remove the node
> + notice_node.remove(notice_node)
> + }
> + });
> });
> </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://bugs.launchpad.net/launchpad/+bug/946995

Revision history for this message
j.c.sackett (jcsackett) wrote :

These changes look fine to me. Thanks for factoring all of these notes into your branch, Mattias.

review: Approve
Revision history for this message
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)

Revision history for this message
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.

Revision history for this message
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://paste.ubuntu.com/874794/. Thanks for that paste, Mattias.

Revision history for this message
j.c.sackett (jcsackett) wrote :

Based on that diff the new revisions look 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/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">&nbsp;
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