Auto merge of #2623 - thiagoarrais:yank, r=locks

Add yank button to the web UI

Trying to fix #1659

This adds a yank/unyank button to the crate and versions pages.

By the way, I'm learning Ember and the crates.io codebase for this one. Please review accordingly. I'm assuming that there is a lot of things out of place. Just let me know where everything should go.

**Some questions:**

1. I've based this button on the edit email button from the account page, but had to make it smaller because of lack of screen real estate. Is that ok? If so, should this live in the component's `yank-button.module.css` or should I create a `.smaller` entry in `shared/buttons.module.css`?

2. Added `yank-button.js` just to specify an empty tag name that in turn makes Ember avoid generating an enclosing `div` that breaks styling. Is there a cleaner way to do that?

Author: bors

app/components/yank-button.hbs

@@ -0,0 +1,31 @@
+{{#if @version.yanked}}
+  <button
+    type="button"
+    local-class="{{this.localClass}}"
+    ...attributes
+    data-test-version-unyank-button={{@version.num}}
+    disabled={{@version.unyankTask.isRunning}}
+    {{on "click" (perform @version.unyankTask)}}
+  >
+    {{#if @version.unyankTask.isRunning }}
+       Unyanking...
+    {{else}}
+       Unyank
+    {{/if}}
+  </button>
+{{else}}
+  <button
+    type="button"
+    local-class="{{this.localClass}}"
+    ...attributes
+    data-test-version-yank-button={{@version.num}}
+    disabled={{@version.yankTask.isRunning}}
+    {{on "click" (perform @version.yankTask)}}
+  >
+    {{#if @version.yankTask.isRunning }}
+       Yanking...
+    {{else}}
+       Yank
+    {{/if}}
+  </button>
+{{/if}}

app/components/yank-button.js

@@ -0,0 +1,15 @@
+import Component from '@glimmer/component';
+
+export default class YankButton extends Component {
+  get tagName() {
+    return '';
+  }
+
+  get localClass() {
+    if (this.args.tan) {
+      return 'tan-button';
+    }
+
+    return 'yellow-button';
+  }
+}

app/components/yank-button.module.css

@@ -0,0 +1,7 @@
+.yellow-button {
+    composes: yellow-button small from '../styles/shared/buttons.module.css';
+}
+
+.tan-button {
+    composes: tan-button small from '../styles/shared/buttons.module.css';
+}

app/controllers/crate/versions.js

@@ -0,0 +1,11 @@
+import Controller from '@ember/controller';
+import { computed } from '@ember/object';
+import { inject as service } from '@ember/service';
+
+export default Controller.extend({
+  session: service(),
+
+  isOwner: computed('model.owner_user', 'session.currentUser.id', function () {
+    return this.get('model.owner_user').findBy('id', this.get('session.currentUser.id'));
+  }),
+});

app/models/version.js

@@ -60,4 +60,26 @@ export default class Version extends Model {
     }
   }).keepLatest())
   loadReadmeTask;
+
+  @(task(function* () {
+    let response = yield fetch(`/api/v1/crates/${this.crate.id}/${this.num}/yank`, { method: 'DELETE' });
+    if (!response.ok) {
+      throw new Error(`Yank request for ${this.crateName} v${this.num} failed`);
+    }
+    this.set('yanked', true);
+
+    return yield response.text();
+  }).keepLatest())
+  yankTask;
+
+  @(task(function* () {
+    let response = yield fetch(`/api/v1/crates/${this.crate.id}/${this.num}/unyank`, { method: 'PUT' });
+    if (!response.ok) {
+      throw new Error(`Unyank request for ${this.crateName} v${this.num} failed`);
+    }
+    this.set('yanked', false);
+
+    return yield response.text();
+  }).keepLatest())
+  unyankTask;
 }

app/templates/crate/version.hbs

@@ -6,6 +6,9 @@
       {{svg-jar "crate" local-class='crate-icon'}}
       <h1 data-test-crate-name>{{ this.crate.name }}</h1>
       <h2 data-test-crate-version>{{ this.currentVersion.num }}</h2>
+      {{#if this.isOwner }}
+        <YankButton @version={{this.currentVersion}} @tan={{true}} />
+      {{/if}}
     </div>
 
     {{#if this.session.currentUser}}

app/templates/crate/versions.hbs

@@ -17,9 +17,13 @@
           <span local-class='yanked'>yanked</span>
         {{/if}}
       </div>
-      <LinkTo @route="crate.version" @model={{version.num}} local-class="arrow">
-        {{svg-jar "right-arrow"}}
-      </LinkTo>
+      {{#if this.isOwner}}
+        <YankButton @version={{version}} />
+      {{else}}
+        <LinkTo @route="crate.version" @model={{version.num}} local-class="arrow">
+          {{svg-jar "right-arrow"}}
+        </LinkTo>
+      {{/if}}
     </div>
   {{/each}}
 </div>

mirage/route-handlers/crates.js

@@ -254,4 +254,28 @@ export function register(server) {
 
     return {};
   });
+
+  server.delete('/api/v1/crates/:crate_id/:version/yank', (schema, request) => {
+    const crateId = request.params.crate_id;
+    const versionNum = request.params.version;
+
+    const version = schema.versions.findBy({ crateId, num: versionNum });
+    if (!version) {
+      return notFound();
+    }
+
+    return {};
+  });
+
+  server.put('/api/v1/crates/:crate_id/:version/unyank', (schema, request) => {
+    const crateId = request.params.crate_id;
+    const versionNum = request.params.version;
+
+    const version = schema.versions.findBy({ crateId, num: versionNum });
+    if (!version) {
+      return notFound();
+    }
+
+    return {};
+  });
 }

src/tests/krate.rs

@@ -79,6 +79,24 @@ impl crate::util::MockTokenUser {
     }
 }
 
+impl crate::util::MockCookieUser {
+    /// Yank the specified version of the specified crate and run all pending background jobs
+    fn yank(&self, krate_name: &str, version: &str) -> crate::util::Response<OkBool> {
+        let url = format!("/api/v1/crates/{}/{}/yank", krate_name, version);
+        let response = self.delete(&url);
+        self.app().run_pending_background_jobs();
+        response
+    }
+
+    /// Unyank the specified version of the specified crate and run all pending background jobs
+    fn unyank(&self, krate_name: &str, version: &str) -> crate::util::Response<OkBool> {
+        let url = format!("/api/v1/crates/{}/{}/unyank", krate_name, version);
+        let response = self.put(&url, &[]);
+        self.app().run_pending_background_jobs();
+        response
+    }
+}
+
 #[test]
 fn index() {
     let (app, anon) = TestApp::init().empty();
@@ -1574,7 +1592,7 @@ fn following() {
 
 #[test]
 fn yank_works_as_intended() {
-    let (app, anon, _, token) = TestApp::full().with_token();
+    let (app, anon, cookie, token) = TestApp::full().with_token();
 
     // Upload a new crate, putting it in the git index
     let crate_to_publish = PublishBuilder::new("fyk");
@@ -1608,6 +1626,26 @@ fn yank_works_as_intended() {
 
     let json = anon.show_version("fyk", "1.0.0");
     assert!(!json.version.yanked);
+
+    // yank it
+    cookie.yank("fyk", "1.0.0").good();
+
+    let crates = app.crates_from_index_head("3/f/fyk");
+    assert!(crates.len() == 1);
+    assert!(crates[0].yanked.unwrap());
+
+    let json = anon.show_version("fyk", "1.0.0");
+    assert!(json.version.yanked);
+
+    // un-yank it
+    cookie.unyank("fyk", "1.0.0").good();
+
+    let crates = app.crates_from_index_head("3/f/fyk");
+    assert!(crates.len() == 1);
+    assert!(!crates[0].yanked.unwrap());
+
+    let json = anon.show_version("fyk", "1.0.0");
+    assert!(!json.version.yanked);
 }
 
 #[test]

tests/acceptance/crate-test.js

@@ -1,4 +1,4 @@
-import { click, fillIn, currentURL, currentRouteName, visit } from '@ember/test-helpers';
+import { click, fillIn, currentURL, currentRouteName, visit, waitFor } from '@ember/test-helpers';
 import { setupApplicationTest } from 'ember-qunit';
 import { module, test } from 'qunit';
 
@@ -219,6 +219,25 @@ module('Acceptance | crate page', function (hooks) {
     assert.dom('[data-test-license]').hasText('MIT/Apache-2.0');
   });
 
+  test('crates can be yanked by owner', async function (assert) {
+    this.server.loadFixtures();
+
+    let user = this.server.schema.users.findBy({ login: 'thehydroimpulse' });
+    this.authenticateAs(user);
+
+    await visit('/crates/nanomsg/0.5.0');
+    await click('[data-test-version-yank-button="0.5.0"]');
+    assert.dom('[data-test-version-yank-button="0.5.0"]').hasText('Yanking...');
+    assert.dom('[data-test-version-yank-button="0.5.0"]').isDisabled();
+
+    await waitFor('[data-test-version-unyank-button="0.5.0"]');
+    await click('[data-test-version-unyank-button="0.5.0"]');
+    assert.dom('[data-test-version-unyank-button="0.5.0"]').hasText('Unyanking...');
+    assert.dom('[data-test-version-unyank-button="0.5.0"]').isDisabled();
+
+    await waitFor('[data-test-version-yank-button="0.5.0"]');
+  });
+
   test('navigating to the owners page when not logged in', async function (assert) {
     this.server.loadFixtures();