AssertionError triggered renewing team membership

[Diogo Matsubara]

As seen in OOPS-865C2562, an AssertionError was triggered while renewing an expired membership of a team
Exception type AssertionError
Exception value This membership can't be renewed by the member himself.

More: OOPS-1144A1730

[Guilherme Salgado (salgado)]

So, either the user manually crafted the URL or the team changed its renewal policy from ONDEMAND (meaning members can renew) to something else (AUTOMATIC, it seems) in between the expiration warning being sent to the user and him following the link to renew the membership.

[Christian Reis (kiko)]

Is that page being rendered for the user, then, even though he can't renew it?

[Guilherme Salgado (salgado)]

Good point; the page is not rendered when he can't renew it, so it must
have been a stale page. that would explain why the user didn't try
submitting the form one more time like they usually do (when he
refreshed the page the form wasn't rendered).

[Curtis Hovey (sinzui)]

The assertion appears to be the right response in this case so I am marking the bug as invalid.

[Diogo Matsubara (matsubara)]

The thing about AssertionError is that we record an OOPS and show an error page to the user, while we should be displaying a nice error message to the user and not recording an OOPS. I don't think this is invalid but likely to be low. What do you think Curtis?

[Curtis Hovey (sinzui)]

Understood. I agree then we want to remove the oops report.

[Curtis Hovey (sinzui)]

OOPS-1119ED453 happened when the user used the link in the email to renew his membership. The user reports that his membership was renewed, but reading the code, I can see that it is not possible. Looking at ~motu/+membership, I can see the user is indeed not expired.

Both the model object and the template check TeamMembership.canBeRenewedByMember() to determine the message to display to the user and whether to render the action button. The method checks that the expiration date is less than the limit.

So this oops could be recreated by using the action, then reloading the page while the action is inprogress, such as double-clicking on the submit button.

I propose that the action check canBeRenewedByMember() to abort early. The page will display the state of the membership to inform the user that the renewal did happen.

[Guilherme Salgado (salgado)]

That sounds like a plan to me.

[Curtis Hovey (sinzui)]

There is a new occurrence of this: OOPS-1430EA1286. I think this too will be fixed by making the action check canBeRenewedByMember() to abort early.

[Edwin Grubbs (edwin-grubbs)]

Fixed in devel r9987 <http://bazaar.launchpad.net/~launchpad-pqm/launchpad/devel/revision/9987>