Code review changes

data/migrations/versions/25aea439ad0e_add_support_for_team_member_invites.py

@@ -41,6 +41,7 @@ def upgrade():
         {'id':41, 'name':'org_invite_team_member'},
         {'id':42, 'name':'org_team_member_invite_accepted'},
         {'id':43, 'name':'org_team_member_invite_declined'},
+        {'id':44, 'name':'org_delete_team_member_invite'},
     ])
 
 
@@ -70,3 +71,8 @@ def downgrade():
         (logentrykind.delete()
          .where(logentrykind.c.name == op.inline_literal('org_team_member_invite_declined')))
         )
+
+    op.execute(
+        (logentrykind.delete()
+         .where(logentrykind.c.name == op.inline_literal('org_delete_team_member_invite')))
+        )

data/model/legacy.py

@@ -331,13 +331,7 @@ def remove_team(org_name, team_name, removed_by_username):
 def add_or_invite_to_team(inviter, team, user=None, email=None):
   # If the user is a member of the organization, then we simply add the
   # user directly to the team. Otherwise, an invite is created for the user/email.
-  # We return None if the user was directly added and the invite object if the user was invited.
+  # We return None if the user was directly added and the invite object if the user was invited.  
-  if email:
-    try:
-      user = User.get(email=email)
-    except User.DoesNotExist:
-      pass
-
   requires_invite = True
   if user:
     orgname = team.organization.username
@@ -1918,6 +1912,19 @@ def confirm_email_authorization_for_repo(code):
   return found
 
 
+def delete_team_email_invite(team, email):
+  found = TeamMemberInvite.get(TeamMemberInvite.email == email, TeamMemberInvite.team == team)
+  found.delete_instance()
+
+def delete_team_user_invite(team, user):
+  try:
+    found = TeamMemberInvite.get(TeamMemberInvite.user == user, TeamMemberInvite.team == team)
+  except TeamMemberInvite.DoesNotExist:
+    return False
+
+  found.delete_instance()
+  return True
+
 def lookup_team_invites(user):
   return TeamMemberInvite.select().where(TeamMemberInvite.user == user)
 

endpoints/api/team.py

@@ -211,7 +211,11 @@ class TeamMember(ApiResource):
         return member_view(user, invited=False)
     
       # User was invited.
-      log_action('org_invite_team_member', orgname, {'member': membername, 'team': teamname})
+      log_action('org_invite_team_member', orgname, {
+        'user': membername,
+        'member': membername,
+        'team': teamname
+      })
       return member_view(user, invited=True)
 
     raise Unauthorized()
@@ -219,11 +223,35 @@ class TeamMember(ApiResource):
   @require_scope(scopes.ORG_ADMIN)
   @nickname('deleteOrganizationTeamMember')
   def delete(self, orgname, teamname, membername):
-    """ Delete an existing member of a team. """
+    """ Delete a member of a team. If the user is merely invited to join
+        the team, then the invite is removed instead.
+    """
     permission = AdministerOrganizationPermission(orgname)
     if permission.can():
       # Remote the user from the team.
       invoking_user = get_authenticated_user().username
+
+      # Find the team.
+      try:
+        team = model.get_organization_team(orgname, teamname)
+      except model.InvalidTeamException:
+        raise NotFound()
+     
+      # Find the member.
+      member = model.get_user(membername)
+      if not member:
+        raise NotFound()
+
+      # First attempt to delete an invite for the user to this team. If none found,
+      # then we try to remove the user directly.
+      if model.delete_team_user_invite(team, member):
+        log_action('org_delete_team_member_invite', orgname, {
+          'user': membername,
+          'team': teamname,
+          'member': membername
+        })
+        return 'Deleted', 204
+
       model.remove_user_from_team(orgname, teamname, membername, invoking_user)
       log_action('org_remove_team_member', orgname, {'member': membername, 'team': teamname})
       return 'Deleted', 204
@@ -251,11 +279,40 @@ class InviteTeamMember(ApiResource):
       # Invite the email to the team.
       inviter = get_authenticated_user()
       invite = handle_addinvite_team(inviter, team, email=email)
-      log_action('org_invite_team_member', orgname, {'email': email, 'team': teamname})
+      log_action('org_invite_team_member', orgname, {
+        'email': email,
+        'team': teamname,
+        'member': email
+      })
       return invite_view(invite)
 
     raise Unauthorized()
 
+  @require_scope(scopes.ORG_ADMIN)
+  @nickname('deleteTeamMemberEmailInvite')
+  def delete(self, orgname, teamname, email):
+    """ Delete an invite of an email address to join a team. """
+    permission = AdministerOrganizationPermission(orgname)
+    if permission.can():
+      team = None
+
+      # Find the team.
+      try:
+        team = model.get_organization_team(orgname, teamname)
+      except model.InvalidTeamException:
+        raise NotFound()
+     
+      # Delete the invite.
+      model.delete_team_email_invite(team, email)
+      log_action('org_delete_team_member_invite', orgname, {
+        'email': email,
+        'team': teamname,
+        'member': email
+      })
+      return 'Deleted', 204
+
+    raise Unauthorized()
+
 
 @resource('/v1/teaminvite/<code>')
 @internal_only
@@ -269,7 +326,7 @@ class TeamMemberInvite(ApiResource):
     try:
       (team, inviter) = model.confirm_team_invite(code, get_authenticated_user())
     except model.DataModelException:
-      raise NotFound()
+      raise NotFound()      
 
     model.delete_matching_notifications(get_authenticated_user(), 'org_team_invite', code=code)
 

initdb.py

@@ -213,6 +213,7 @@ def initialize_database():
   LogEntryKind.create(name='org_create_team')
   LogEntryKind.create(name='org_delete_team')
   LogEntryKind.create(name='org_invite_team_member')
+  LogEntryKind.create(name='org_delete_team_member_invite')
   LogEntryKind.create(name='org_add_team_member')
   LogEntryKind.create(name='org_team_member_invite_accepted')
   LogEntryKind.create(name='org_team_member_invite_declined')

static/js/app.js

@@ -562,6 +562,8 @@ quayApp = angular.module('quay', quayDependencies, function($provide, cfpLoading
          var fieldIcons = {
           'inviter': 'user',
           'username': 'user',
+          'user': 'user',
+          'email': 'envelope',
           'activating_username': 'user',
           'delegate_user': 'user',
           'delegate_team': 'group',
@@ -2364,7 +2366,7 @@ quayApp.directive('signinForm', function () {
           // forms get removed before the location changes.
           $timeout(function() {
             var redirectUrl = getRedirectUrl();
-            if (redirectUrl == $location.path()) {
+            if (redirectUrl == $location.path() || redirectUrl == null) {
               return;
             }
             window.location = (redirectUrl ? redirectUrl : '/');
@@ -2733,7 +2735,20 @@ quayApp.directive('logsView', function () {
           'org_delete_team': 'Delete team: {team}',
           'org_add_team_member': 'Add member {member} to team {team}',
           'org_remove_team_member': 'Remove member {member} from team {team}',
-          'org_invite_team_member': 'Invite user {member} to team {team}',
+          'org_invite_team_member': function(metadata) {
+            if (metadata.user) {
+              return 'Invite {user} to team {team}';
+            } else {
+              return 'Invite {email} to team {team}';
+            }
+          },
+          'org_delete_team_member_invite': function(metadata) {
+            if (metadata.user) {
+              return 'Rescind invite of {user} to team {team}';
+            } else {
+              return 'Rescind invite of {email} to team {team}';
+            }
+          },
 
           'org_team_member_invite_accepted': 'User {member}, invited by {inviter}, joined team {team}',
           'org_team_member_invite_declined': 'User {member}, invited by {inviter}, declined to join team {team}',
@@ -2819,6 +2834,7 @@ quayApp.directive('logsView', function () {
         'org_delete_team': 'Delete team',
         'org_add_team_member': 'Add team member',
         'org_invite_team_member': 'Invite team member',
+        'org_delete_team_member_invite': 'Rescind team member invitation',
         'org_remove_team_member': 'Remove team member',
         'org_team_member_invite_accepted': 'Team invite accepted',
         'org_team_member_invite_declined': 'Team invite declined',

static/js/controllers.js

@@ -1,4 +1,5 @@
 function SignInCtrl($scope, $location) {
+  $scope.redirectUrl = '/';
 }
 
 function GuideCtrl() {
@@ -2337,6 +2338,31 @@ function TeamViewCtrl($rootScope, $scope, Restangular, ApiService, $routeParams)
     }, errorHandler);
   };
 
+  $scope.revokeInvite = function(inviteInfo) {
+    if (inviteInfo.kind == 'invite') {
+      // E-mail invite.
+      $scope.revokeEmailInvite(inviteInfo.email);
+    } else {
+      // User invite.
+      $scope.removeMember(inviteInfo.name);
+    }
+  };
+
+  $scope.revokeEmailInvite = function(email) {
+    var params = {
+      'orgname': orgname,
+      'teamname': teamname,
+      'email': email
+    };
+
+    ApiService.deleteTeamMemberEmailInvite(null, params).then(function(resp) {
+      if (!$scope.memberMap[email]) { return; }
+      var index = $.inArray($scope.memberMap[email], $scope.members);
+      $scope.members.splice(index, 1);
+      delete $scope.memberMap[email];
+    }, ApiService.errorDisplay('Cannot revoke team invite'));
+  };
+
   $scope.removeMember = function(username) {
     var params = {
       'orgname': orgname,
@@ -2345,17 +2371,11 @@ function TeamViewCtrl($rootScope, $scope, Restangular, ApiService, $routeParams)
     };
 
     ApiService.deleteOrganizationTeamMember(null, params).then(function(resp) {
-      for (var i = $scope.members.length - 1; i >= 0; --i) {
+      if (!$scope.memberMap[username]) { return; }
-        var current = $scope.members[i];
+      var index = $.inArray($scope.memberMap[username], $scope.members);
-        if (current.name == username) {
+      $scope.members.splice(index, 1);
-          $scope.members.splice(i, 1);
+      delete $scope.memberMap[username];
-          delete $scope.memberMap[username];
+    }, ApiService.errorDisplay('Cannot remove team member'));
-          break;
-        }
-      }      
-    }, function() {
-      $('#cannotChangeMembersModal').modal({});
-    });
   };
 
   $scope.updateForDescription = function(content) {
@@ -2738,6 +2758,9 @@ function ConfirmInviteCtrl($scope, $location, UserService, ApiService, Notificat
 
   UserService.updateUserIn($scope, function(user) {
     if (!user.anonymous && !$scope.loading) {
+      // Make sure to not redirect now that we have logged in. We'll conduct the redirect
+      // manually.
+      $scope.redirectUrl = null;
       $scope.loading = true;
 
       var params = {
@@ -2754,5 +2777,5 @@ function ConfirmInviteCtrl($scope, $location, UserService, ApiService, Notificat
     }
   });
   
-  $scope.redirectUrl = 'confirminvite?code=' + $location.search()['code'];
+  $scope.redirectUrl = window.location.href;
 }

static/partials/team-view.html

@@ -38,7 +38,7 @@
             <span class="entity-reference" entity="member" namespace="organization.name" show-gravatar="true" gravatar-size="32"></span>
           </td>
           <td>
-            <span class="delete-ui" delete-title="'Remove ' + member.name  + ' From Team'" button-title="'Remove'"
+            <span class="delete-ui" delete-title="'Remove ' + member.name  + ' from team'" button-title="'Remove'"
                   perform-delete="removeMember(member.name)" ng-if="canEditMembers"></span>
           </td>
         </tr>     
@@ -53,7 +53,7 @@
             <span class="entity-reference" entity="member" namespace="organization.name"></span>
           </td>
           <td>
-            <span class="delete-ui" delete-title="'Remove ' + member.name  + ' From Team'" button-title="'Remove'"
+            <span class="delete-ui" delete-title="'Remove ' + member.name  + ' from team'" button-title="'Remove'"
                   perform-delete="removeMember(member.name)" ng-if="canEditMembers"></span>
           </td>
         </tr>
@@ -74,6 +74,8 @@
             </span>
           </td>
           <td>
+            <span class="delete-ui" delete-title="'Revoke invite to join team'" button-title="'Revoke'"
+                  perform-delete="revokeInvite(member)" ng-if="canEditMembers"></span>
           </td>
         </tr>
       </table>

test/test_api_usage.py

@@ -131,6 +131,10 @@ class ApiTestCase(unittest.TestCase):
 
   def deleteResponse(self, resource_name, params={}, expected_code=204):
     rv = self.app.delete(self.url_for(resource_name, params))
+
+    if rv.status_code != expected_code:
+      print 'Mismatch data for resource DELETE %s: %s' % (resource_name, rv.data)
+
     self.assertEquals(rv.status_code, expected_code)
     return rv.data
 
@@ -827,27 +831,6 @@ class TestAcceptTeamMemberInvite(ApiTestCase):
 
     self.fail(membername + ' not found in team: ' + json.dumps(data))
 
-  def test_accept_wronguser(self):
-    self.login(ADMIN_ACCESS_USER)
-
-    # Create the invite.
-    membername = NO_ACCESS_USER
-    response = self.putJsonResponse(TeamMember,
-                                    params=dict(orgname=ORGANIZATION, teamname='owners',
-                                                membername=membername))
-    
-    self.assertEquals(True, response['invited'])
-
-    # Try to accept the invite.
-    user = model.get_user(membername)
-    invites = list(model.lookup_team_invites(user))
-    self.assertEquals(1, len(invites))
-
-    self.putResponse(TeamMemberInvite,
-                     params=dict(code=invites[0].invite_token),
-                     expected_code=404)
-
-
   def test_accept(self):
     self.login(ADMIN_ACCESS_USER)
 
@@ -935,6 +918,40 @@ class TestDeclineTeamMemberInvite(ApiTestCase):
 
 
 class TestDeleteOrganizationTeamMember(ApiTestCase):
+  def test_deletememberinvite(self):
+    self.login(ADMIN_ACCESS_USER)
+
+    membername = NO_ACCESS_USER
+    response = self.putJsonResponse(TeamMember,
+                                    params=dict(orgname=ORGANIZATION, teamname='readers',
+                                                membername=membername))
+     
+
+    self.assertEquals(True, response['invited'])
+
+    # Verify the invite was added.
+    json = self.getJsonResponse(TeamMemberList,
+                                params=dict(orgname=ORGANIZATION,
+                                            teamname='readers',
+                                            includePending=True))
+
+    assert len(json['members']) == 3
+
+    # Delete the invite.
+    self.deleteResponse(TeamMember,
+                        params=dict(orgname=ORGANIZATION, teamname='readers',
+                                    membername=membername))
+    
+
+    # Verify the user was removed from the team.
+    json = self.getJsonResponse(TeamMemberList,
+                                params=dict(orgname=ORGANIZATION,
+                                            teamname='readers',
+                                            includePending=True))
+
+    assert len(json['members']) == 2
+
+
   def test_deletemember(self):
     self.login(ADMIN_ACCESS_USER)