Code review comment for lp:~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part2

Revision history for this message
Curtis Hovey (sinzui) wrote :

Hi Edwin.

This is a nice addition. I have no remarks about new test. I have a suggestion about the team story

> === modified file 'lib/lp/registry/stories/team/xx-team-home.txt'
> --- lib/lp/registry/stories/team/xx-team-home.txt 2009-11-22 15:43:16 +0000
> +++ lib/lp/registry/stories/team/xx-team-home.txt 2009-12-21 19:53:58 +0000

...

> @@ -88,14 +93,14 @@
> Show received invitations
>
> If the team does not have any recently approved or proposed members,
> -the recent members sections are not displayed:
> +the recent members sections are hidden using the "unseen" css class:
>
> >>> browser.open('http://launchpad.dev/~launchpad')
> >>> print find_tag_by_id(browser.contents, 'recently-approved')
> - None
> + <td id="recently-approved" ... class="unseen">...
>
> - >>> print find_tag_by_id(browser.contents, 'recently-applied')
> - None
> + >>> print find_tag_by_id(browser.contents, 'recently-proposed')
> + <td id="recently-proposed" class="unseen">...

These tests will break if we reimplement this as a list. We should not print
markup in user stories because they do no read the source. The test is parsing
the tree many times. I think this can be expressed like this so that it runs
faster and is robust.

    >>> browser.open('http://launchpad.dev/~launchpad')
    >>> content = find_main_content(browser.contents)
    >>> tag = find_tag_by_id(content, 'recently-approved')
    >>> print tag['class']
    unseen

    >>> tag = find_tag_by_id(content, 'recently-proposed')
    >>> print tag['class']
    unseen

.

review: Approve (code and js)

« Back to merge proposal