Merge lp://staging/~danilo/launchpad/bug-772763-remove-unmute-dialog into lp://staging/launchpad/db-devel

Proposed by Данило Шеган
Status: Merged
Merged at revision: 10583
Proposed branch: lp://staging/~danilo/launchpad/bug-772763-remove-unmute-dialog
Merge into: lp://staging/launchpad/db-devel
Prerequisite: lp://staging/~danilo/launchpad/bug-772763-remove-unmute-dialog-part1
Diff against target: 393 lines (+111/-193)
2 files modified
lib/lp/bugs/javascript/bugtask_index_portlets.js (+110/-192)
lib/lp/bugs/javascript/subscriber.js (+1/-1)
To merge this branch: bzr merge lp://staging/~danilo/launchpad/bug-772763-remove-unmute-dialog
Reviewer Review Type Date Requested Status
Brad Crittenden (community) code Needs Fixing
Данило Шеган (community) Abstain
Review via email: mp+61780@code.staging.launchpad.net

Commit message

"Unmute bug mail" unmutes directly (and restores any previous subscription) without popping up a dialog.

Description of the change

= Bug 772763: remove unmute dialog =

As part of fixing 772763, we drop the pop-dialog when "unmute" is clicked and instead make it a direct action on click.

== Proposed fix ==

Turn "Unmute" link into a direct action (instead of popping up an "advanced overlay" with options to unmute and similar).

== Implementation details ==

This branch is entirely Gary's work. I am only shepherding it.

Basically, it's a very welcome refactoring of the entire mute handling for a single bug:

 - no use of JS Subscription class for muting/unmuting (it didn't really make sense since "mute" is not a subscription, but instead orthogonal to it)
 - no tests are provided because we plan to refactor this code further with our next bug (subscribers' list)

== Tests ==

Nada.

== Demo and Q/A ==

Go to https://bugs.launchpad.dev/debian/+source/mozilla-firefox/+bug/3 and play with muting/unmuting while having a subscription or not having one.

= Launchpad lint =

Checking for conflicts and issues in changed files.

Linting changed files:
  lib/lp/bugs/javascript/bugtask_index_portlets.js
  lib/lp/bugs/javascript/subscriber.js

To post a comment you must log in.
Revision history for this message
Данило Шеган (danilo) :
review: Abstain
Revision history for this message
Brad Crittenden (bac) wrote :

Hi Gary/Danilo,

The branch looks good with the exception that if I am muted and there are no other subscribers then "No subscribers" is correctly shown. However when unmuting, the person's name is added to the subscriber list but "No subscribers" is not removed. Looks like that action should happen around line 119 of the diff.

review: Needs Fixing (code)
Revision history for this message
Данило Шеган (danilo) wrote :

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches

to status/vote changes: