tlhobbes's avatar
tlhobbes
New Contributor
9 years ago
Status:
New Idea

Do not remove Approval when Poked (both individual Pokes and via Poke Everyone)

If a Reviewer (or Observer) is in the Approval state, then it is totally unnecessary, and indeed quite annoying and time-wasting, to have that reviewer taken out of Approval upon that reviewer being poked.

 

It would be much better, if Poking a user (via individual Poke or via Poking Everyone) left users in their state of Approval.

 

This would allievate issues such as:

1) Author clicks "Poke Everyone" (perhaps because several reviewers need to look at the review), but the unintended consequence is that it can throw a couple other reviewers out of Approval (thus wasting their time, because nothing else changed up to that point that would have otherwise thrown them out of Approval).  Sure, the author could have individually poke only the necessary subset of users, but that less desirable alternative basically leaves the "Poke Everyone" button mostly useless.

2) Similarly, an Author may accidentaly poke a specific user; maybe missed the State column's claim that the user was in Approval and then miss-poked the user.  Either way, it's the same downsides as above, but with no alternative via Code Collab.

 

In general, Code Collaborator has several annoying and time-wasting cases of throwing reviewers out of approval; this is one of them; please do fix!  Thanks :-)

8 Comments

  • MrDubya's avatar
    MrDubya
    Occasional Contributor

    Rather than click "Poke Everyone", click "Email Everyone", this would still send everyone a notification, but will retain their approved status (and you could remove the approved users from the list before clicking send).  I think that's key differentiator between emailing and poking a user - poking sends the user an email, plus changes their status back to active, which is a useful feature to bring someone back into the review.

  • tlhobbes's avatar
    tlhobbes
    New Contributor

    MrDubya sure but "Email Everyone" won't change the "Waiting" statuses of users to "Active", so your suggested alternative is not a full solution; I concur with you on "changes their status back to active, which is a useful feature to bring someone back into the review" because, for example, that makes the review show near the top of their list of reviews (with a red up arrow instead of near the bottom with a yellow down arrow) and so on - which is better than simply getting only an email notification.

  • MrDubya's avatar
    MrDubya
    Occasional Contributor

    From my perspective, if you are asking a critical reviewer to re-review, then you are making them active again, and they need to indicate when they have finished by clicking the approve / waiting button.  Poking a user allows all this to happen with a single click, and I would not want to drop this functionality.

     

    So it looks like what would be needed is not to change poke or email, but a new function which would be somewhere between emailing a user & poking them - where they are notified of the review, and it goes to the top of their list or is highlighted somehow, but they don't need to re-approve.  I don't think that would also change their status back to active because that is a trigger to the tool to enforce the rules & ensure critical reviewers still have to approve.  Does that sound about right?

  • tlhobbes's avatar
    tlhobbes
    New Contributor

    I believe poke's UI still needs addressed as a destructive action.

     

    I see your point that there is a constructive use-case for poking a reviewer out of approval to get them to re-review (and I overlooked that case in my original post; I agree, that being able to do that should remain a feature in some way); however, there is still the undesirable destructive case of unintentionally/accidentally poking a reviewer out of approval, which is also done via the same single click (of Poke or Poke All).


    I see now that this is a classic UI design issue, and the general solution is: don't allow destructive actions via a single click, instead present the user with confirmation UI which explains the consequences and allows the user to proceed or cancel the action.  Doing so allows the action to still occur as feature, and also greatly helps prevent the action from accidentally occurring.

    In the case of Poke and Poke All: if there is one or more users that would be poked out of approval, then show a confirmation UI; otherwise, there is no destructive consequence so skip showing confirmation UI.

    When poking a single user out of Approval, then the confirmation can be as simple as: "Poke will pull <username> out of Approval, proceed?" Ok/Cancel. In the case of poking multiple users at once, where at least one of them would be poked out of approval, then the confirmation UI could be similar to the single-user-poke confirmation (e.g. list all of the users that would be poked out of approval); or perhaps it would show all of the users that would be poked, in two lists: 1) those that would be poked out of approval and 2) everyone else; then the confirmation UI could have 3 choices, such as: Poke All, Poke Non-Approved Only, Cancel.

    Your suggestion of a new feature, that is somewhere between emailing a user and poking them, may still be useful in addition to the above; but it isn't enough alone to address the destructive case of Poke/Poke All.

    I believe the most pertinent issues to address in codecollab (other than bugs that break the review process) is to further streamline and optimize the review process by eliminating (where possible) and otherwise preventing (via UI confirmation or options) all cases that would otherwise unnecessarily pull a Reviewer out of Approval; and secondarily similar cases that unnecessarily pull any user out of Waiting to Active.  All such annoying cases lead to confusion and lost development time.  Poke and Poke All are just one case of this bigger general issue.

  • MrDubya's avatar
    MrDubya
    Occasional Contributor

    I agree; my suggestion of a new feature between emailing & poking was more me trying to interpret what you were looking for.  I think there is definitely a use case for having a function that pokes only unapproved reviewers as a way to quickly remind everyone who has yet to finish their review, and do agree that providing a warning that when clicking "poke everyone" may cause approved reviewers to go back to active state is a good practice to follow.

  • tlhobbes's avatar
    tlhobbes
    New Contributor

    A warning also when clicking "poke" (in addition to "poke everyone") would also be great :-)

  • pbauer's avatar
    pbauer
    New Contributor

    We have experienced the same issue.  A warning of the destructive nature of the action would be a great way to deal with this situation.  Please consider adding it if it has not already happened.

     

    Thanks

  • MrDubya's avatar
    MrDubya
    Occasional Contributor

    This just came up for me today, and a warning about changing the approval status for Poke would have prevented a support ticket.  I do think there are two things that would be good to implement:

     

    • Warn the user when clicking Poke / Poke All that there are Approved users that will be sent back to "Active" & have to re-approve.
    • Add options to only poke / email all unapproved users (with one click).  The available workaround (click the individual Poke / email buttons next to the unapproved users) will be tedious for reviews having a large # of participants.