launchpad-reviewers team mailing list archive

[Curtis Hovey <curtis.hovey@xxxxxxxxxxxxx>]

Review: Needs Fixing code

Hi Benjamin.

The proposed changes do not conform to PEP 8 style standards that are checked via `make lint`

Linting changed files:
  lib/lp/registry/browser/team.py

./lib/lp/registry/browser/team.py
    1006: Could not compile; unexpected indent: widget_class='field subordinate')
    1003: E501 line too long (81 characters)
    1006: E113 unexpected indentation
    1006: E225 missing whitespace around operator
    1008: unindent does not match any outer indentation level: widget_class='field subordinate')

Trivial white-space has no effect on Python. You can revert these changes.

This issue is about a string was passed to the team object, but an int was expected. This is probably a widget issue, which is used to render and adapt user input.

I can see that these conditions are implicitly tested in
    lib/lp/registry/browser/tests/private-team-creation-views.txt
about line 94. The values entered are almost identical to the values in the screen shot. *but* this test is for team creation, the screen shot shows editing a team is broken!

We must be missing a test for team/+edit that enters these same values.

We want a test added to TestTeamEditView. This quickly written test demonstrates the error seen in the bug:

    def test_expiration_and_renewal(self):
        # The team's membership expiration and renewal rules can be set.
        owner = self.factory.makePerson()
        team = self.factory.makeTeam(name="team", owner=owner)
        form = {
            'field.name': team.name,
            'field.displayname': team.displayname,
            'field.defaultmembershipperiod': '180',
            'field.defaultrenewalperiod': '365',
            'field.membership_policy': 'RESTRICTED',
            'field.renewal_policy': 'ONDEMAND',
            'field.actions.save': 'Save',
            }
        login_person(owner)
        view = create_initialized_view(team, '+edit', form=form)
        self.assertEqual(0, len(view.errors))
        self.assertEqual(
            TeamMembershipPolicy.RESTRICTED, team.membership_policy)
        self.assertEqual(180, team.defaultmembershipperiod)
        self.assertEqual(365, team.defaultrenewalperiod)
        self.assertEqual(
            TeamMembershipRenewalPolicy.ONDEMAND, team.renewal_policy)

I ran this from the command line to verify that the test fails because there is on form validation error:
    ./bin/test -vvc -t TestTeamEditView lp.registry.browser.tests.test_team

The fix would be to add update TeamEditView to also use the custom IntWidget as teamAddView does. I think the fix is in lib/lp/registry/browser/team.py around line 302.

TeamEditView
    ...
    custom_widget('defaultrenewalperiod', IntWidget,
        widget_class='field subordinate')

Lint will report that StrippedTextWidget is unused so we can delete the import.

PS. I broken view a few weeks ago.
-- 
https://code.launchpad.net/~bkerensa/launchpad/fix-for-1044457/+merge/122414
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.