Merge lp:~henninge/launchpad/bug-435712 into lp:launchpad/db-devel

= Summary =

Fixes bug 435712 by moving the details about a question to the "registering" slot, like it is with bugs and branches. The language information was simply moved to the main body.

== Screenshots ==

Before and after:
http://people.canonical.com/~henninge/screenshots/answers-bug-435712-before.png
http://people.canonical.com/~henninge/screenshots/answers-bug-435712-after.pn

== Test command ==

bin/test -vvt question -t this-is-a-faq

== Demo/QA ==

Go to any question and verify that is looks like the "after" screenshot avbove.

Link fix:
http://people.canonical.com/~henninge/screenshots/answers-bug-435712-after.png

Hi Henning!

Thanks for getting in there and getting this page a bit more consistent!

I'm setting this to code-approved - as I'm guessing you've already got
it running through ec2test, but I'd like someone else to second the
ui as I've got some questions about the usage of the registration
slot.

> = Summary =
>
> Fixes bug 435712 by moving the details about a question to the "registering"
> slot, like it is with bugs and branches.

Right - I hadn't realised that we were putting more than the actual
registration info into that slot - but can see that it's already done
on the bugs page. That is, I hadn't realised we were going to communicate
status and/or repeat the context (bug XXX or question xxx) in that slot.

I personally prefer the branches page, where it is *just* the registration
info:

Created by Michael Nelson on 2009-09-23 and last modified on 2009-09-23

rather than what's on the bugs page:

Bug #201001 reported by Celso Providelo on 2008-03-11

which I reckon ought to be:

Reported by Celso Providelo on 2008-03-11

but I haven't been involved in any of the discussions - so perhaps there's
already been a decision there? I just think that yet another reference
to 'Bug #xxxxx' or '...question #xxxx' is unnecessary (and getting
noisy). What are your thoughts? The status could go below with the language
details in a dl (like on other 3-0 pages - the object details).

A note regarding the status - I don't think it's part of this branch,
but I think it should be "Answered question" rather than "Answered Question".
See: https://dev.launchpad.net/UserInterfaceWording#Capitalization

Ah, I just noticed that there's no leaf breadcrumb 'Answered question #xxx'.
But as you said in irc, that's a separate bug.

> The language information was simply
> moved to the main body.
>
> == Screenshots ==
>
> Before and after:
> http://people.canonical.com/~henninge/screenshots/answers-
> bug-435712-before.png
> http://people.canonical.com/~henninge/screenshots/answers-bug-435712-after.pn
>

Thanks for the screenshots!

> == Test command ==
>
> bin/test -vvt question -t this-is-a-faq
>
> == Demo/QA ==
>
> Go to any question and verify that is looks like the "after" screenshot
> avbove.

Great.

> === 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-24 14:18:35 +0000
> @@ -755,6 +755,10 @@
> initial_focus_widget = None
>
> @property
> + def label(self):
> + return self.context.title
> +
> + @property
> def page_title(self):
> return smartquote('%s question #%d: "%s"') % (
> self.context.target.displayname,
>
> === modified file 'lib/lp/answers/stories/question-add.txt'
> --- lib/lp/answers/stories/question-add.txt 2009-09-23 14:40:53 +0000
> +++ lib/lp/answers/stories/question-add.txt 2009-09-24 14:18:35 +0000
> @@ -157,9 +157,9 @@
> >>> user_browser.title
> 'Questions : \xe2\x80\x9cmozilla-firefox\xe2\x80\x9d package : Ubuntu'
>
> - >>> contents = find_main_content(user_browser.contents)
> - >>> contents.findChild('div', id='question-details')
> + >>> prin...

Thanks for doing this!

Hi Michael,
thanks for your review. Let me just quickly comment on your suggestions
in the code.

Am 24.09.2009 17:11, Michael Nelson schrieb:
>> === modified file 'lib/lp/answers/stories/question-add.txt'
>> --- lib/lp/answers/stories/question-add.txt 2009-09-23 14:40:53 +0000
>> +++ lib/lp/answers/stories/question-add.txt 2009-09-24 14:18:35 +0000
>> @@ -157,9 +157,9 @@
>> >>> user_browser.title
>> 'Questions : \xe2\x80\x9cmozilla-firefox\xe2\x80\x9d package : Ubuntu'
>>
>> - >>> contents = find_main_content(user_browser.contents)
>> - >>> contents.findChild('div', id='question-details')
>> + >>> print find_tag_by_id(user_browser.contents, 'registration')
>> <div...>Open<... Question #..., asked...by...No Privileges Person...
>
> Any reason for not using extract_text here? Might be nicer to read...

No, none. It actually crossed my mind, too. Done.

>
>> + >>> contents = find_main_content(user_browser.contents)
>> >>> contents.find('div', 'report')
>
> ... and here: print extract_text(contents.find('div', 'report'))

Did that, too.

>
>> <div class="report"><p>I use Ubuntu on AMD64 ...</div>
>>
>>
>> === modified file 'lib/lp/answers/stories/question-confirm-url.txt'
>> --- lib/lp/answers/stories/question-confirm-url.txt 2009-08-25 21:30:02 +0000
>> +++ lib/lp/answers/stories/question-confirm-url.txt 2009-09-24 14:18:35 +0000
>> @@ -96,7 +96,7 @@
>> This adds his comment to the question and mark it as 'Solved.'
>>
>> >>> print extract_text(
>> - ... find_tag_by_id(owner_browser.contents, 'question-details'))
>> + ... find_tag_by_id(owner_browser.contents, 'registration'))
>> Solved ...
>
> Ah, so it's also communicating the status... - I personally am not
> convinced that it belongs there - but maybe you've had discussions
> with others about it?
>
> I think it should simply be:
>
> Asked on 2006-01-01 by Guilherme Salgado (and potentially last updated -
> not sure if that's in the db?)

You are probably right but that would include more work on the whole
page which would break more tests ... Too much for too little time ;-)

>
>> >>> print find_tags_by_class(
>> ... owner_browser.contents, 'boardCommentBody')[-1].renderContents()
>>
>> === 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-24 14:18:35 +0000
>> @@ -52,7 +52,7 @@
>>
>> >>> def print_question_status(browser):
>> ... print extract_text(
>> - ... find_tag_by_id(browser.contents, 'question-details'))
>> + ... find_tag_by_id(browser.contents, 'registration'))
>
> Hmm... perhaps it's worth moving this into something like
> l/c/lp/testing/pages.py? See what you think.

Haven't looked at that yet but usually I am all for this kind of stuff.
But ...

[...]
>> === modified file 'lib/lp/answers/templates/question-index.pt'
>> --- lib/lp/answers/templates/question-index.pt 2009-09-03 20:22:39 +0000
>> +++ lib/lp/answers/templates/question-index.pt 2009-09-24 14:18:3...

This patch is approved for the re-roll. It is not 'release-critical' but it is a safe fix and will help us achieve more consistency across apps.

Thanks for the fix.

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Henning Eggers wrote:
> Am 24.09.2009 17:11, Michael Nelson schrieb:
>>
>> I think it should simply be:
>>
>> Asked on 2006-01-01 by Guilherme Salgado (and potentially last updated -
>> not sure if that's in the db?)
>
> You are probably right but that would include more work on the whole
> page which would break more tests ... Too much for too little time ;-)
>

Yeah - totally. Your changes are great - we can look at standardising
the use of the registration slot later :)

Great work for getting it landed!

- --
Michael
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAkq7tYYACgkQGSan/irvan0VbgCcDtUfpxACW+bdW20SE9dr0eYq
MLYAn0ErEvvLZeLsIXctMfNmCZyPUU6O
=XVhv
-----END PGP SIGNATURE-----