Code review comment for lp:~allenap/launchpad/just-comment-on-question-bug-114710

Wow, I am so psyched you are adding this. It's a huge annoyance every time
I'm CHR. Thanks!

We talked about the order of the buttons and since we want to encourage people
to answer the question, it seems like a better order would be "Add Answer"
"Add Information Request" "Add Comment".

We also discussed possible confusion that users may have as to what the
different actions mean. We agreed that it might be nice to add a (?) help
pop up but also that that's outside the scope of this branch.

I have only minor comments on style, but otherwise the code looks great.
merge-conditional, r=me with their consideration.

=== modified file 'lib/lp/answers/browser/question.py'
--- lib/lp/answers/browser/question.py 2009-09-22 15:43:52 +0000
+++ lib/lp/answers/browser/question.py 2009-09-25 09:01:14 +0000
> @@ -678,11 +683,16 @@
> editable_fields.append(field.__name__)
> self.form_fields = self.form_fields.select(*editable_fields)
>
> - @action(u"Continue", name="change")
> + @action(u"Save Changes", name="change")

Why do you use a u-string here but everywhere else you use the translation
marker _()? I think for consistency you should change this to
_('Save Changes')

> @@ -810,12 +823,10 @@
> def canAddComment(self, action):
> """Return whether the comment action should be displayed.
>
> - Comments (message without a status change) can be added when the
> - question is solved or invalid
> + Comments (message without a status change) can be added at any
> + time by any logged-in user.
> """
> - return (self.user is not None and
> - self.context.status in [
> - QuestionStatus.SOLVED, QuestionStatus.INVALID])
> + return (self.user is not None)

You don't need the parentheses any more.

=== modified file 'lib/lp/answers/stories/question-edit.txt'
--- lib/lp/answers/stories/question-edit.txt 2009-08-25 21:30:02 +0000
+++ lib/lp/answers/stories/question-edit.txt 2009-09-25 09:01:14 +0000
> @@ -14,6 +14,11 @@
> >>> user_browser.url
> '.../firefox/+question/2/+edit'
>
> +There is a cancel link should the user decide otherwise:
> +
> + >>> user_browser.getLink('Cancel').url
> + '.../firefox/+question/2'

You should print the url here and elsewhere to avoid the quote noise.

=== modified file 'lib/lp/answers/stories/question-reject-and-change-status.txt'
--- lib/lp/answers/stories/question-reject-and-change-status.txt 2009-08-25 21:30:02 +0000
+++ lib/lp/answers/stories/question-reject-and-change-status.txt 2009-09-25 09:01:14 +0000
> @@ -35,6 +35,12 @@
> There is 1 error.
> You must provide an explanation message.
>
> +At this point the user might decide this is a bad idea, so there is a
> +cancel link to take him back to the question:
> +
> + >>> admin_browser.getLink('Cancel').url
> + '.../firefox/+question/2'

print

> +
> Entering an explanation message and clicking the 'Reject' button,
> will reject the question.
>
> @@ -108,6 +114,11 @@
> >>> admin_browser.getControl('Status', index=0).displayValue
> ['Invalid']
>
> +There is also a cancel link should the user decide otherwise:
> +
> + >>> admin_browser.getLink('Cancel').url
> + '.../firefox/+question/2'

print

> +
> The user needs to select a status and enter a message explaining the
> status change:

=== modified file 'lib/lp/answers/stories/question-workflow.txt'
--- lib/lp/answers/stories/question-workflow.txt 2009-09-18 15:24:30 +0000
+++ lib/lp/answers/stories/question-workflow.txt 2009-09-25 09:01:14 +0000
> @@ -82,6 +82,25 @@
> Please enter a message.
>
>
> +== Adding a Comment ==
> +
> +A comment can be added at any point without altering the status. The
> +user simply enters the comment in the 'Message' box and clicks the
> +'Add Comment' button.
> +
> + >>> support_browser.getControl('Message').value = (
> + ... "I forgot to mention, in the meantime here is a workaround...")
> + >>> support_browser.getControl('Add Comment').click()
> +
> +This appends the comment to the question and it doesn't change its
> +status:
> +
> + >>> print find_request_status(support_browser.contents)
> + Needs information...
> + >>> print find_last_comment(support_browser.contents).renderContents()
> + <p>I forgot to mention, in the meantime here is a workaround...</p>

Maybe use extract_text() instead? the HTML tags aren't important here.