Code review comment for lp:~edwin-grubbs/launchpad/bug-482176-add-team-member-ajax-part2

Revision history for this message
Edwin Grubbs (edwin-grubbs) wrote :

Hi Martin,

Thanks for the review. For some reason, your comment didn't get added to the merge proposal, so I pasted it here again.

On Mon, Dec 21, 2009 at 1:35 PM, Martin Albisetti <email address hidden> wrote:
> Review: Needs Fixing ui
> Hi Edwin,
>
> The branch in general looks great, you solved the bulk of the challenges already.
> As per our conversation on IRC, the actual fix for the "user is already approved" message is not getting in the situation in the first place, so filing a bug about is a good start.

Opened bug 499210 for this.

> If we're going to keep the dialog around for a while, I would do one of two things:
>
> 1) Add buttons to dismiss the dialog or go back to the results (OK and BACK?)
> 2) Show the message instead of the results (ie, keep the search fields visible to re-search)

I added an "OK" button to dismiss the dialog.

> I would also re-phrase the message to something like "$user is already part of the team"

There actually are two potential errors, since a team could already be in an invited status, so it would be unclear to say that they are already part of the team. Here are screenshots of both error messages.

https://chinstrap.canonical.com/~egrubbs/addmember/new_approved_error_message.png
https://chinstrap.canonical.com/~egrubbs/addmember/new_invited_error_message.png

« Back to merge proposal