Launchpad itself

AssertionError triggered renewing team membership

Bug #230801 reported by Diogo Matsubara
8
Affects Status Importance Assigned to Milestone
Launchpad itself
Fix Released
Low
Edwin Grubbs
Launchpad itself 3.1.12

Bug Description

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

See original description

Related branches

lp://qastaging/~edwin-grubbs/launchpad/bug-230801-renewing-membership
Michael Nelson (community): Approve (code)
Diff: 949 lines (+296/-79)
39 files modified
lib/lp/archiveuploader/nascentuploadfile.py (+1/-1)
lib/lp/archiveuploader/tests/nascentuploadfile.txt (+71/-0)
lib/lp/bugs/templates/bug-portlet-subscribers.pt (+1/-1)
lib/lp/bugs/templates/bugtarget-filebug-submit-bug.pt (+1/-1)
lib/lp/bugs/templates/bugtarget-portlet-bugfilters.pt (+1/-1)
lib/lp/bugs/templates/bugtarget-portlet-bugtags.pt (+1/-1)
lib/lp/bugs/templates/bugtask-index.pt (+3/-3)
lib/lp/bugs/templates/bugtask-tasks-and-nominations-table-row.pt (+1/-1)
lib/lp/bugs/templates/bugtasks-and-nominations-table.pt (+1/-1)
lib/lp/bugs/templates/official-bug-target-manage-tags.pt (+1/-1)
lib/lp/code/templates/branch-import-details.pt (+1/-1)
lib/lp/code/templates/branch-index.pt (+1/-1)
lib/lp/code/templates/branch-listing.pt (+1/-1)
lib/lp/code/templates/branch-portlet-subscribers.pt (+1/-1)
lib/lp/code/templates/branch-related-bugs-specs.pt (+1/-1)
lib/lp/code/templates/branchmergeproposal-generic-listing.pt (+1/-1)
lib/lp/registry/browser/person.py (+4/-1)
lib/lp/registry/browser/tests/productrelease-views.txt (+1/-1)
lib/lp/registry/templates/object-timeline-graph.pt (+1/-1)
lib/lp/registry/templates/person-macros.pt (+1/-1)
lib/lp/registry/templates/product-new.pt (+1/-1)
lib/lp/registry/templates/productrelease-add-from-series.pt (+1/-1)
lib/lp/registry/templates/teammembership-index.pt (+1/-1)
lib/lp/registry/templates/timeline-macros.pt (+1/-1)
lib/lp/soyuz/templates/archive-edit-dependencies.pt (+1/-1)
lib/lp/soyuz/templates/archive-macros.pt (+1/-1)
lib/lp/soyuz/templates/archive-packages.pt (+1/-1)
lib/lp/soyuz/templates/archive-subscribers.pt (+1/-1)
lib/lp/translations/browser/language.py (+8/-0)
lib/lp/translations/stories/distroseries/xx-distroseries-templates.txt (+56/-13)
lib/lp/translations/stories/productseries/xx-productseries-templates.txt (+27/-18)
lib/lp/translations/stories/standalone/xx-language.txt (+68/-7)
lib/lp/translations/templates/language-index.pt (+9/-4)
lib/lp/translations/templates/object-templates.pt (+20/-3)
lib/lp/translations/templates/pofile-export.pt (+1/-1)
lib/lp/translations/templates/pofile-translate.pt (+1/-1)
lib/lp/translations/templates/translation-import-queue-macros.pt (+1/-1)
lib/lp/translations/templates/translationimportqueueentry-index.pt (+1/-1)
lib/lp/translations/templates/translationmessage-translate.pt (+1/-1)
Revision history for this message
Guilherme Salgado (salgado) wrote :

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.

Changed in launchpad:
importance: Undecided → Low
status: New → Confirmed
Revision history for this message
Christian Reis (kiko) wrote :

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

Revision history for this message
Guilherme Salgado (salgado) wrote : Re: [Bug 230801] Re: AssertionError triggered renewing team membership

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).

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

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

Changed in launchpad-registry:
status: Confirmed → Invalid
Revision history for this message
Diogo Matsubara (matsubara) wrote :

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?

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

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

Changed in launchpad-registry:
status: Invalid → Triaged
Revision history for this message
Curtis Hovey (sinzui) wrote :

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.

Revision history for this message
Guilherme Salgado (salgado) wrote :

That sounds like a plan to me.

Ursula Junque (ursinha)
description: updated
Revision history for this message
Curtis Hovey (sinzui) wrote :

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.

Changed in launchpad-registry:
milestone: none → 3.1.12
Edwin Grubbs (edwin-grubbs)
Changed in launchpad-registry:
assignee: nobody → Edwin Grubbs (edwin-grubbs)
Edwin Grubbs (edwin-grubbs)
Changed in launchpad-registry:
status: Triaged → In Progress
Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote : Bug fixed by a commit

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

Changed in launchpad-registry:
status: In Progress → Fix Committed
Edwin Grubbs (edwin-grubbs)
Changed in launchpad-registry:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.
Subscribing...

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.