Auto merge of #2253 - RobbieClarken:teams-in-manage-owners, r=locks

Display teams on the crate owners page

This change updates the crate owners page (`/crates/<crate-name>/owners`) to display teams which are set to be crate owner. It also adds a "Remove" button for each team which allows removing teams from owning the crate in the same way that user type owners can be removed.

I have displayed the team as `<organisation>/<team>` rather than `github:<organisation>:<team>` because I think it looks cleaner (eg `rust-lang/core` vs `github:rust-lang:core`). Happy to change it if people disagree.

<img width="976" alt="Screen Shot 2020-03-06 at 5 17 50 pm" src="https://user-images.githubusercontent.com/663161/76058622-08489700-5fd1-11ea-8656-febb234f58ad.png">

I haven't touched adding teams via the owners page in this PR. From what I can tell, it is currently possible although the message that is returned is misleading. I think that should be tackled in a separate PR.

Addresses #1617.

Author: bors

app/controllers/crate/owners.js

@@ -31,14 +31,20 @@ export default Controller.extend({
       }
     },
 
-    async removeOwner(user) {
+    async removeOwner(owner) {
       this.set('removed', false);
-
       try {
-        await this.crate.removeOwner(user.get('login'));
-        this.set('removed', `User ${user.get('login')} removed as crate owner`);
-
-        this.get('crate.owner_user').removeObject(user);
+        await this.crate.removeOwner(owner.get('login'));
+        switch (owner.kind) {
+          case 'user':
+            this.set('removed', `User ${owner.get('login')} removed as crate owner`);
+            this.get('crate.owner_user').removeObject(owner);
+            break;
+          case 'team':
+            this.set('removed', `Team ${owner.get('display_name')} removed as crate owner`);
+            this.get('crate.owner_team').removeObject(owner);
+            break;
+        }
       } catch (error) {
         if (error.errors) {
           this.set('removed', `Error removing owner: ${error.errors[0].detail}`);

app/models/team.js

@@ -14,4 +14,8 @@ export default Model.extend({
     let login_split = login.split(':');
     return login_split[1];
   }),
+  display_name: computed('name', 'org_name', function() {
+    let { name, org_name } = this.getProperties('name', 'org_name');
+    return `${org_name}/${name}`;
+  }),
 });

app/templates/crate/owners.hbs

@@ -44,8 +44,32 @@
 {{/if}}
 
 <div class='owners white-rows'>
+  {{#each this.crate.owner_team as |team|}}
+    <div class='crate row' data-test-owner-team={{team.login}}>
+      <div>
+        <LinkTo @route={{team.kind}} @model={{team.login}}>
+          <UserAvatar @user={{team}} @size="medium-small" />
+        </LinkTo>
+      </div>
+      <div>
+        <LinkTo @route={{team.kind}} @model={{team.login}}>
+          {{team.display_name}}
+        </LinkTo>
+      </div>
+      <div class='stats'>
+        {{#if team.email}}
+          {{team.email}}
+        {{else}}
+          &nbsp;
+        {{/if}}
+      </div>
+      <div class='stats downloads'>
+        <button type="button" class='remove-owner small yellow-button' {{action 'removeOwner' team}}>Remove</button>
+      </div>
+    </div>
+  {{/each}}
   {{#each this.crate.owner_user as |user|}}
-    <div class='crate row'>
+    <div class='crate row' data-test-owner-user={{user.login}}>
       <div>
         <LinkTo @route={{user.kind}} @model={{user.login}}>
           <UserAvatar @user={{user}} @size="medium-small" />

mirage/route-handlers/crates.js

@@ -181,9 +181,9 @@ export function register(server) {
 
     const body = JSON.parse(request.requestBody);
     const [ownerId] = body.owners;
-    const user = schema.users.findBy({ login: ownerId });
+    const owner = schema.users.findBy({ login: ownerId }) || schema.teams.findBy({ login: ownerId });
 
-    if (!user) {
+    if (!owner) {
       return notFound();
     }
 

tests/acceptance/crate-test.js

@@ -255,7 +255,9 @@ module('Acceptance | crate page', function(hooks) {
 
     await visit('/crates/nanomsg/owners');
 
-    assert.dom('.owners .row').exists({ count: 2 });
+    assert.dom('.owners .row').exists({ count: 4 });
+    assert.dom('a[href="/teams/github:org:thehydroimpulse"]').exists();
+    assert.dom('a[href="/teams/github:org:blabaere"]').exists();
     assert.dom('a[href="/users/thehydroimpulse"]').exists();
     assert.dom('a[href="/users/blabaere"]').exists();
   });
@@ -268,7 +270,7 @@ module('Acceptance | crate page', function(hooks) {
 
     assert.dom('.error').exists();
     assert.dom('.error').hasText('Please enter a username');
-    assert.dom('.owners .row').exists({ count: 2 });
+    assert.dom('.owners .row').exists({ count: 4 });
   });
 
   test('attempting to add non-existent owner', async function(assert) {
@@ -280,7 +282,7 @@ module('Acceptance | crate page', function(hooks) {
 
     assert.dom('.error').exists();
     assert.dom('.error').hasText('Error sending invite: Not Found');
-    assert.dom('.owners .row').exists({ count: 2 });
+    assert.dom('.owners .row').exists({ count: 4 });
   });
 
   test('add a new owner', async function(assert) {
@@ -292,16 +294,26 @@ module('Acceptance | crate page', function(hooks) {
 
     assert.dom('.invited').exists();
     assert.dom('.invited').hasText('An invite has been sent to iain8');
-    assert.dom('.owners .row').exists({ count: 2 });
+    assert.dom('.owners .row').exists({ count: 4 });
   });
 
-  test('remove a crate owner', async function(assert) {
+  test('remove a crate owner when owner is a user', async function(assert) {
     this.server.loadFixtures();
 
     await visit('/crates/nanomsg/owners');
-    await click('.owners .row:first-child .remove-owner');
+    await click('[data-test-owner-user="thehydroimpulse"] .remove-owner');
 
-    assert.dom('.removed').exists();
-    assert.dom('.owners .row').exists({ count: 1 });
+    assert.dom('.removed').hasText('User thehydroimpulse removed as crate owner');
+    assert.dom('[data-test-owner-user]').exists({ count: 1 });
+  });
+
+  test('remove a crate owner when owner is a team', async function(assert) {
+    this.server.loadFixtures();
+
+    await visit('/crates/nanomsg/owners');
+    await click('[data-test-owner-team="github:org:thehydroimpulse"] .remove-owner');
+
+    assert.dom('.removed').hasText('Team org/thehydroimpulseteam removed as crate owner');
+    assert.dom('[data-test-owner-team]').exists({ count: 1 });
   });
 });