From bc182a578043847a5697e891750e6d2f2824bdb2 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Mon, 10 Jul 2017 14:03:56 -0400 Subject: [PATCH 01/37] query for recent_downloads --- src/krate.rs | 35 +++++++++++++++++++++++++++-------- 1 file changed, 27 insertions(+), 8 deletions(-) diff --git a/src/krate.rs b/src/krate.rs index 1a6435ce9ce..057ba7aaddf 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -19,6 +19,7 @@ use serde_json; use semver; use time::Timespec; use url::Url; +use chrono::NaiveDate; use app::{App, RequestApp}; use badge::EncodableBadge; @@ -39,7 +40,18 @@ use util::{RequestUtils, CargoResult, internal, ChainError, human}; use version::{EncodableVersion, NewVersion}; use {Model, User, Keyword, Version, Category, Badge, Replica}; -#[derive(Debug, Clone, Queryable, Identifiable, AsChangeset)] +#[derive(Queryable, Identifiable, Associations, AsChangeset)] +#[belongs_to(Crate)] +#[primary_key(crate_id, date)] +#[table_name="crate_downloads"] +pub struct CrateDownload { + pub crate_id: i32, + pub downloads: i32, + pub date: NaiveDate, +} + +#[derive(Debug, Clone, Queryable, Identifiable, Associations, AsChangeset)] +#[has_many(crate_downloads)] pub struct Crate { pub id: i32, pub name: String, @@ -728,16 +740,23 @@ impl Model for Crate { /// Handles the `GET /crates` route. pub fn index(req: &mut Request) -> CargoResult { - use diesel::expression::AsExpression; - use diesel::types::Bool; + use diesel::expression::{AsExpression, DayAndMonthIntervalDsl}; + use diesel::types::{Bool, BigInt}; + use diesel::expression::functions::date_and_time::{now, date}; + use diesel::expression::sql_literal::sql; let conn = req.db_conn()?; let (offset, limit) = req.pagination(10, 100)?; let params = req.query(); let sort = params.get("sort").map(|s| &**s).unwrap_or("alpha"); + let recent_downloads = sql::("SUM(crate_downloads.downloads)"); + let mut query = crates::table - .select((ALL_COLUMNS, AsExpression::::as_expression(false))) + .inner_join(crate_downloads::table) + .group_by(crates::id) + .filter(crate_downloads::date.gt(date(now - 90.days()))) + .select((ALL_COLUMNS, AsExpression::::as_expression(false), recent_downloads.clone())) .into_boxed(); if sort == "downloads" { @@ -754,7 +773,7 @@ pub fn index(req: &mut Request) -> CargoResult { ), )); - query = query.select((ALL_COLUMNS, crates::name.eq(q_string))); + query = query.select((ALL_COLUMNS, crates::name.eq(q_string), recent_downloads.clone())); let perfect_match = crates::name.eq(q_string).desc(); if sort == "downloads" { query = query.order((perfect_match, crates::downloads.desc())); @@ -824,14 +843,14 @@ pub fn index(req: &mut Request) -> CargoResult { )); } - let data = query.paginate(limit, offset).load::<((Crate, bool), i64)>( + let data = query.paginate(limit, offset).load::<((Crate, bool, i64), i64)>( &*conn, )?; let total = data.first().map(|&(_, t)| t).unwrap_or(0); let crates = data.iter() - .map(|&((ref c, _), _)| c.clone()) + .map(|&((ref c, _, _), _)| c.clone()) .collect::>(); - let perfect_matches = data.into_iter().map(|((_, b), _)| b).collect::>(); + let perfect_matches = data.into_iter().map(|((_, b, _), _)| b).collect::>(); let versions = Version::belonging_to(&crates) .load::(&*conn)? From 36fe81cffbf47f0176c1d5a7229aa3ce46d9fb7e Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Mon, 10 Jul 2017 15:18:30 -0400 Subject: [PATCH 02/37] correctly displays top recently downloaded crates --- src/krate.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/krate.rs b/src/krate.rs index 057ba7aaddf..79619c79b58 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -51,7 +51,6 @@ pub struct CrateDownload { } #[derive(Debug, Clone, Queryable, Identifiable, Associations, AsChangeset)] -#[has_many(crate_downloads)] pub struct Crate { pub id: i32, pub name: String, @@ -760,7 +759,7 @@ pub fn index(req: &mut Request) -> CargoResult { .into_boxed(); if sort == "downloads" { - query = query.order(crates::downloads.desc()) + query = query.order(recent_downloads.clone().desc()) } else { query = query.order(crates::name.asc()) } From 5017507eb35153f218a00d089652b00ed3933053 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Mon, 10 Jul 2017 16:17:08 -0400 Subject: [PATCH 03/37] Added recent_downloads to encodable_crate --- src/krate.rs | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/src/krate.rs b/src/krate.rs index 79619c79b58..606410986df 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -111,6 +111,7 @@ pub struct EncodableCrate { pub badges: Option>, pub created_at: String, pub downloads: i32, + pub recent_downloads: i64, pub max_version: String, pub description: Option, pub homepage: Option, @@ -495,8 +496,9 @@ impl Crate { max_version: semver::Version, badges: Option>, exact_match: bool, + recent_downloads: i64, ) -> EncodableCrate { - self.encodable(max_version, None, None, None, badges, exact_match) + self.encodable(max_version, None, None, None, badges, exact_match, recent_downloads) } pub fn encodable( @@ -507,6 +509,7 @@ impl Crate { categories: Option<&[Category]>, badges: Option>, exact_match: bool, + recent_downloads: i64, ) -> EncodableCrate { let Crate { name, @@ -532,6 +535,7 @@ impl Crate { updated_at: ::encode_time(updated_at), created_at: ::encode_time(created_at), downloads: downloads, + recent_downloads: recent_downloads, versions: versions, keywords: keyword_ids, categories: category_ids, @@ -759,6 +763,8 @@ pub fn index(req: &mut Request) -> CargoResult { .into_boxed(); if sort == "downloads" { + query = query.order(crates::downloads.desc()) + } else if sort == "recent-downloads" { query = query.order(recent_downloads.clone().desc()) } else { query = query.order(crates::name.asc()) @@ -776,6 +782,8 @@ pub fn index(req: &mut Request) -> CargoResult { let perfect_match = crates::name.eq(q_string).desc(); if sort == "downloads" { query = query.order((perfect_match, crates::downloads.desc())); + } else if sort == "recent-downloads" { + query = query.order((perfect_match, recent_downloads.clone().desc())); } else { let rank = ts_rank_cd(crates::textsearchable_index_col, q); query = query.order((perfect_match, rank.desc())) @@ -849,7 +857,8 @@ pub fn index(req: &mut Request) -> CargoResult { let crates = data.iter() .map(|&((ref c, _, _), _)| c.clone()) .collect::>(); - let perfect_matches = data.into_iter().map(|((_, b, _), _)| b).collect::>(); + let perfect_matches = data.clone().into_iter().map(|((_, b, _), _)| b).collect::>(); + let recent_downloads = data.clone().into_iter().map(|((_, _, s), _)| s).collect::>(); let versions = Version::belonging_to(&crates) .load::(&*conn)? @@ -860,7 +869,8 @@ pub fn index(req: &mut Request) -> CargoResult { let crates = versions .zip(crates) .zip(perfect_matches) - .map(|((max_version, krate), perfect_match)| { + .zip(recent_downloads) + .map(|(((max_version, krate), perfect_match), recent_downloads)| { // FIXME: If we add crate_id to the Badge enum we can eliminate // this N+1 let badges = badges::table @@ -870,6 +880,7 @@ pub fn index(req: &mut Request) -> CargoResult { max_version, Some(badges), perfect_match, + recent_downloads, )) }) .collect::>()?; @@ -909,7 +920,7 @@ pub fn summary(req: &mut Request) -> CargoResult { .map(|versions| Version::max(versions.into_iter().map(|v| v.num))) .zip(krates) .map(|(max_version, krate)| { - Ok(krate.minimal_encodable(max_version, None, false)) + Ok(krate.minimal_encodable(max_version, None, false, 0)) }) .collect() }; @@ -1006,6 +1017,7 @@ pub fn show(req: &mut Request) -> CargoResult { Some(&cats), Some(badges), false, + 0, ), versions: versions .into_iter() @@ -1148,7 +1160,7 @@ pub fn new(req: &mut Request) -> CargoResult { warnings: Warnings<'a>, } Ok(req.json(&R { - krate: krate.minimal_encodable(max_version, None, false), + krate: krate.minimal_encodable(max_version, None, false, 0), warnings: warnings, })) }) From 776ed77d394bc3f96465730cad596bbbbeb635d1 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Mon, 10 Jul 2017 16:44:38 -0400 Subject: [PATCH 04/37] addition of dropdown selection for ability to sort by recent downloads --- app/controllers/crates.js | 10 +++++++++- app/controllers/search.js | 9 ++++++++- app/models/crate.js | 1 + app/templates/crates.hbs | 7 ++++++- app/templates/search.hbs | 7 ++++++- 5 files changed, 30 insertions(+), 4 deletions(-) diff --git a/app/controllers/crates.js b/app/controllers/crates.js index 185d93ca697..154fa17573a 100644 --- a/app/controllers/crates.js +++ b/app/controllers/crates.js @@ -14,6 +14,14 @@ export default Controller.extend(PaginationMixin, { totalItems: computed.readOnly('model.meta.total'), currentSortBy: computed('sort', function() { - return (this.get('sort') === 'downloads') ? 'Downloads' : 'Alphabetical'; + if (this.get('sort') === 'downloads') { + return 'All-Time Downloads'; + } else if (this.get('sort') === 'recent-downloads') { + return 'Recent Downloads'; + } else { + return 'Alphabetical'; + } + + //return (this.get('sort') === 'downloads') ? 'Downloads' : 'Alphabetical'; }), }); diff --git a/app/controllers/search.js b/app/controllers/search.js index a590e1348a9..74a4e8fa37a 100644 --- a/app/controllers/search.js +++ b/app/controllers/search.js @@ -13,7 +13,14 @@ export default Controller.extend(PaginationMixin, { totalItems: computed.readOnly('model.meta.total'), currentSortBy: computed('sort', function() { - return (this.get('sort') === 'downloads') ? 'Downloads' : 'Relevance'; + if (this.get('sort' === 'downloads')) { + return 'All-Time Downloads'; + } else if (this.get('sort') === 'recent-downloads') { + return 'Recent Downloads'; + } else { + return 'Relevance'; + } + //return (this.get('sort') === 'downloads') ? 'Downloads' : 'Relevance'; }), hasItems: computed.bool('totalItems'), diff --git a/app/models/crate.js b/app/models/crate.js index 67ed18cf4c3..44cf9bb710e 100644 --- a/app/models/crate.js +++ b/app/models/crate.js @@ -4,6 +4,7 @@ import DS from 'ember-data'; export default DS.Model.extend({ name: DS.attr('string'), downloads: DS.attr('number'), + recent_downloads: DS.attr('number'), created_at: DS.attr('date'), updated_at: DS.attr('date'), max_version: DS.attr('string'), diff --git a/app/templates/crates.hbs b/app/templates/crates.hbs index cc8a760b228..692ebd8a6a4 100644 --- a/app/templates/crates.hbs +++ b/app/templates/crates.hbs @@ -52,7 +52,12 @@
  • {{#link-to (query-params page=1 sort="downloads")}} - Downloads + All-Time Downloads + {{/link-to}} +
    • +
  • + {{#link-to (query-params page=1 sort="recent-downloads")}} + Recent Downloads {{/link-to}}
    • {{/rl-dropdown}} diff --git a/app/templates/search.hbs b/app/templates/search.hbs index e256327fb60..f221c60dce1 100644 --- a/app/templates/search.hbs +++ b/app/templates/search.hbs @@ -38,7 +38,12 @@
  • {{#link-to (query-params page=1 sort="downloads")}} - Downloads + All-Time Downloads + {{/link-to}} +
    • +
  • + {{#link-to (query-params page=1 sort="recent-downloads")}} + Recent Downloads {{/link-to}}
    • {{/rl-dropdown}} From 642a095f87b0b0ae961432b5d9e9f85d280d31ab Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 11 Jul 2017 12:32:20 -0400 Subject: [PATCH 05/37] all-time and in past 90 days downloads displayed in list, currently both totals are listed but has ability to show just one according to sort --- app/components/crate-row.js | 28 +++++++++++++++++++++++++- app/styles/crate.scss | 11 +++++++++- app/templates/components/crate-row.hbs | 6 +++++- app/templates/crates.hbs | 2 +- 4 files changed, 43 insertions(+), 4 deletions(-) diff --git a/app/components/crate-row.js b/app/components/crate-row.js index 8e2527acb16..ad3de53bdcd 100644 --- a/app/components/crate-row.js +++ b/app/components/crate-row.js @@ -1,5 +1,31 @@ import Component from '@ember/component'; +import { computed } from '@ember/object'; export default Component.extend({ - classNames: ['crate', 'row'] + classNames: ['crate', 'row'], + sort: null, + + isDownloads: computed('sort', function() { + if (this.get('sort') === 'All-Time Downloads') { + return true; + } else { + return false; + } + }), + + isRecentDownloads: computed('sort', function() { + if (this.get('sort') === 'Recent Downloads') { + return true; + } else { + return false; + } + }), + + isAlpha: computed('sort', function() { + if (this.get('sort') === 'Alphabetical') { + return true; + } else { + return false; + } + }) }); diff --git a/app/styles/crate.scss b/app/styles/crate.scss index ddf3f99ada5..5f3b9744c34 100644 --- a/app/styles/crate.scss +++ b/app/styles/crate.scss @@ -136,7 +136,16 @@ width: 15%; color: $main-color-light; } - .downloads { @include display-flex; @include align-items(center); } + .downloads { + @include display-flex; + @include align-items(center); + padding-bottom: 5px; + } + .recent-downloads { + @include display-flex; + @include align-items(center); + + } .rev-dep-downloads {padding-left: 7px} } diff --git a/app/templates/components/crate-row.hbs b/app/templates/components/crate-row.hbs index daf61b07799..f9318133dbe 100644 --- a/app/templates/components/crate-row.hbs +++ b/app/templates/components/crate-row.hbs @@ -20,7 +20,11 @@
    {{svg-jar "download"}} - {{ format-num crate.downloads }} + All-Time: {{ format-num crate.downloads }} +
    +
    + {{svg-jar "download"}} + Recent: {{ format-num crate.recent_downloads }}
    diff --git a/app/templates/crates.hbs b/app/templates/crates.hbs index 692ebd8a6a4..7d0d6acb239 100644 --- a/app/templates/crates.hbs +++ b/app/templates/crates.hbs @@ -67,7 +67,7 @@
    {{#each model as |crate|}} - {{crate-row crate=crate}} + {{crate-row crate=crate sort=currentSortBy}} {{/each}}
    From c105307f430bc8d6ffab9c70dc9d8167167b770e Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 11 Jul 2017 13:36:11 -0400 Subject: [PATCH 06/37] increased size of stats div --- app/styles/crate.scss | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/styles/crate.scss b/app/styles/crate.scss index 5f3b9744c34..4c0f7078441 100644 --- a/app/styles/crate.scss +++ b/app/styles/crate.scss @@ -119,7 +119,7 @@ padding-top: 5px; @include display-flex; @include flex-direction(column); - width: 85%; + width: 75%; } .info a { @@ -133,7 +133,7 @@ } .stats { - width: 15%; + width: 25%; color: $main-color-light; } .downloads { From 1f9c68a93d4008c198a0b99804aaef41f73cdbc8 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Wed, 12 Jul 2017 11:15:56 -0400 Subject: [PATCH 07/37] Remove unused code for showing only either recent downloads or all time downloads in results --- app/components/crate-row.js | 28 +--------------------------- app/templates/crates.hbs | 2 +- 2 files changed, 2 insertions(+), 28 deletions(-) diff --git a/app/components/crate-row.js b/app/components/crate-row.js index ad3de53bdcd..8e2527acb16 100644 --- a/app/components/crate-row.js +++ b/app/components/crate-row.js @@ -1,31 +1,5 @@ import Component from '@ember/component'; -import { computed } from '@ember/object'; export default Component.extend({ - classNames: ['crate', 'row'], - sort: null, - - isDownloads: computed('sort', function() { - if (this.get('sort') === 'All-Time Downloads') { - return true; - } else { - return false; - } - }), - - isRecentDownloads: computed('sort', function() { - if (this.get('sort') === 'Recent Downloads') { - return true; - } else { - return false; - } - }), - - isAlpha: computed('sort', function() { - if (this.get('sort') === 'Alphabetical') { - return true; - } else { - return false; - } - }) + classNames: ['crate', 'row'] }); diff --git a/app/templates/crates.hbs b/app/templates/crates.hbs index 7d0d6acb239..692ebd8a6a4 100644 --- a/app/templates/crates.hbs +++ b/app/templates/crates.hbs @@ -67,7 +67,7 @@
    {{#each model as |crate|}} - {{crate-row crate=crate sort=currentSortBy}} + {{crate-row crate=crate}} {{/each}}
    From 13491f1591437b23f4bca41dd4a16daf42535795 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Wed, 12 Jul 2017 11:17:33 -0400 Subject: [PATCH 08/37] Misplaced paren, fixes weird searching bug --- app/controllers/crates.js | 2 -- app/controllers/search.js | 3 +-- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/app/controllers/crates.js b/app/controllers/crates.js index 154fa17573a..274e25ee739 100644 --- a/app/controllers/crates.js +++ b/app/controllers/crates.js @@ -21,7 +21,5 @@ export default Controller.extend(PaginationMixin, { } else { return 'Alphabetical'; } - - //return (this.get('sort') === 'downloads') ? 'Downloads' : 'Alphabetical'; }), }); diff --git a/app/controllers/search.js b/app/controllers/search.js index 74a4e8fa37a..50914fd2fdc 100644 --- a/app/controllers/search.js +++ b/app/controllers/search.js @@ -13,14 +13,13 @@ export default Controller.extend(PaginationMixin, { totalItems: computed.readOnly('model.meta.total'), currentSortBy: computed('sort', function() { - if (this.get('sort' === 'downloads')) { + if (this.get('sort') === 'downloads') { return 'All-Time Downloads'; } else if (this.get('sort') === 'recent-downloads') { return 'Recent Downloads'; } else { return 'Relevance'; } - //return (this.get('sort') === 'downloads') ? 'Downloads' : 'Relevance'; }), hasItems: computed.bool('totalItems'), From 3ba4e41c216f05ae0e6b677a7d5925fc0035f740 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Wed, 12 Jul 2017 11:52:43 -0400 Subject: [PATCH 09/37] Add recent download sort to user page --- app/controllers/user.js | 8 +++++++- app/templates/user.hbs | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/controllers/user.js b/app/controllers/user.js index c2c6a9b0893..91f4f5f3deb 100644 --- a/app/controllers/user.js +++ b/app/controllers/user.js @@ -13,6 +13,12 @@ export default Controller.extend(PaginationMixin, { totalItems: computed.readOnly('model.crates.meta.total'), currentSortBy: computed('sort', function() { - return (this.get('sort') === 'downloads') ? 'Downloads' : 'Alphabetical'; + if (this.get('sort') === 'downloads') { + return 'All Time Downloads'; + } else if (this.get('sort') === 'recent-downloads') { + return 'Recent Downloads'; + } else { + return 'Alphabetical'; + } }), }); diff --git a/app/templates/user.hbs b/app/templates/user.hbs index 98b1d54b833..ff70a442d5e 100644 --- a/app/templates/user.hbs +++ b/app/templates/user.hbs @@ -38,7 +38,12 @@
  • {{#link-to (query-params sort="downloads")}} - Downloads + All Time Downloads + {{/link-to}} +
    • +
  • + {{#link-to (query-params sort="recent-downloads")}} + Recent Downloads {{/link-to}}
    • {{/rl-dropdown}} From 62a21100b14adbb6e62f7865c7fc0a0277041c72 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Wed, 12 Jul 2017 11:58:22 -0400 Subject: [PATCH 10/37] Added sort by recent downloads to team page --- app/controllers/team.js | 8 +++++++- app/templates/team.hbs | 7 ++++++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/app/controllers/team.js b/app/controllers/team.js index 53830bf9153..0a973616d72 100644 --- a/app/controllers/team.js +++ b/app/controllers/team.js @@ -12,6 +12,12 @@ export default Controller.extend(PaginationMixin, { totalItems: computed.readOnly('model.crates.meta.total'), currentSortBy: computed('sort', function() { - return (this.get('sort') === 'downloads') ? 'Downloads' : 'Alphabetical'; + if (this.get('sort') === 'downloads') { + return 'All Time Downloads'; + } else if (this.get('sort') === 'recent-downloads') { + return 'Recent Downloads'; + } else { + return 'Alphabetical'; + } }), }); diff --git a/app/templates/team.hbs b/app/templates/team.hbs index cde38b755a0..de9faf4c5c7 100644 --- a/app/templates/team.hbs +++ b/app/templates/team.hbs @@ -47,7 +47,12 @@
  • {{#link-to (query-params sort="downloads")}} - Downloads + All Time Downloads + {{/link-to}} +
    • +
  • + {{#link-to (query-params sort="recent-downloads")}} + Recent Downloads {{/link-to}}
    • {{/rl-dropdown}} From 908ad0656e2a1ae68fd46df7a9cf678fa70d5adc Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Wed, 12 Jul 2017 12:28:45 -0400 Subject: [PATCH 11/37] Fixes bug on team page where dropdown selection was not changing sort of crates displayed --- app/routes/team.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/routes/team.js b/app/routes/team.js index 6a7c642964b..f87e63c9400 100644 --- a/app/routes/team.js +++ b/app/routes/team.js @@ -1,13 +1,13 @@ -import RSVP from 'rsvp'; -import { inject as service } from '@ember/service'; import Route from '@ember/routing/route'; +import { inject as service } from '@ember/service'; +import RSVP from 'rsvp'; export default Route.extend({ flashMessages: service(), queryParams: { - page: { refreshedModel: true }, - sort: { refreshedModel: true }, + page: { refreshModel: true }, + sort: { refreshModel: true }, }, data: {}, From 420b97af286da85e54cb9d91360838cfb401e39f Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Thu, 13 Jul 2017 10:40:48 -0400 Subject: [PATCH 12/37] fix broken test for list of crates returned by search --- tests/acceptance/search-test.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/acceptance/search-test.js b/tests/acceptance/search-test.js index 76defefa1bd..4df580b9e7a 100644 --- a/tests/acceptance/search-test.js +++ b/tests/acceptance/search-test.js @@ -18,13 +18,13 @@ test('searching for "rust"', async function(assert) { assert.equal(document.title, 'Search Results for \'rust\' - Cargo: packages for Rust'); hasText(assert, '#crates-heading', 'Search Results for \'rust\''); - hasText(assert, '#results', 'Displaying 1-8 of 8 total results Sort by Relevance Relevance Downloads'); + hasText(assert, '#results', 'Displaying 1-8 of 8 total results Sort by Relevance Relevance All-Time Downloads Recent Downloads'); hasText(assert, '#crates .row:first .desc .info', 'kinetic-rust'); findWithAssert('#crates .row:first .desc .info .vers img[alt="0.0.16"]'); hasText(assert, '#crates .row:first .desc .summary', 'A Kinetic protocol library written in Rust'); - hasText(assert, '#crates .row:first .downloads', '225'); + hasText(assert, '#crates .row:first .downloads', 'All-Time: 225'); }); test('pressing S key to focus the search bar', async function(assert) { From 2d87b064bb396693cad7e911257f01b999e4c1c8 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Fri, 14 Jul 2017 10:44:15 -0400 Subject: [PATCH 13/37] null error when trying to get count --- src/krate.rs | 20 +++++++++++--------- src/tests/krate.rs | 2 +- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/krate.rs b/src/krate.rs index 606410986df..033b691319a 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -111,7 +111,7 @@ pub struct EncodableCrate { pub badges: Option>, pub created_at: String, pub downloads: i32, - pub recent_downloads: i64, + pub recent_downloads: Option, pub max_version: String, pub description: Option, pub homepage: Option, @@ -496,7 +496,7 @@ impl Crate { max_version: semver::Version, badges: Option>, exact_match: bool, - recent_downloads: i64, + recent_downloads: Option, ) -> EncodableCrate { self.encodable(max_version, None, None, None, badges, exact_match, recent_downloads) } @@ -509,7 +509,7 @@ impl Crate { categories: Option<&[Category]>, badges: Option>, exact_match: bool, - recent_downloads: i64, + recent_downloads: Option, ) -> EncodableCrate { let Crate { name, @@ -747,6 +747,7 @@ pub fn index(req: &mut Request) -> CargoResult { use diesel::types::{Bool, BigInt}; use diesel::expression::functions::date_and_time::{now, date}; use diesel::expression::sql_literal::sql; + use diesel::query_source::joins::LeftOuter; let conn = req.db_conn()?; let (offset, limit) = req.pagination(10, 100)?; @@ -756,9 +757,8 @@ pub fn index(req: &mut Request) -> CargoResult { let recent_downloads = sql::("SUM(crate_downloads.downloads)"); let mut query = crates::table - .inner_join(crate_downloads::table) + .join(crate_downloads::table, LeftOuter, crates::id.eq(crate_downloads::crate_id).and(crate_downloads::date.gt(date(now - 90.days())))) .group_by(crates::id) - .filter(crate_downloads::date.gt(date(now - 90.days()))) .select((ALL_COLUMNS, AsExpression::::as_expression(false), recent_downloads.clone())) .into_boxed(); @@ -850,6 +850,8 @@ pub fn index(req: &mut Request) -> CargoResult { )); } + println!("crate count: {:?}", crates::table.count().get_result::(&*conn)); + let data = query.paginate(limit, offset).load::<((Crate, bool, i64), i64)>( &*conn, )?; @@ -880,7 +882,7 @@ pub fn index(req: &mut Request) -> CargoResult { max_version, Some(badges), perfect_match, - recent_downloads, + Some(recent_downloads), )) }) .collect::>()?; @@ -920,7 +922,7 @@ pub fn summary(req: &mut Request) -> CargoResult { .map(|versions| Version::max(versions.into_iter().map(|v| v.num))) .zip(krates) .map(|(max_version, krate)| { - Ok(krate.minimal_encodable(max_version, None, false, 0)) + Ok(krate.minimal_encodable(max_version, None, false, Some(0))) }) .collect() }; @@ -1017,7 +1019,7 @@ pub fn show(req: &mut Request) -> CargoResult { Some(&cats), Some(badges), false, - 0, + Some(0), ), versions: versions .into_iter() @@ -1160,7 +1162,7 @@ pub fn new(req: &mut Request) -> CargoResult { warnings: Warnings<'a>, } Ok(req.json(&R { - krate: krate.minimal_encodable(max_version, None, false, 0), + krate: krate.minimal_encodable(max_version, None, false, Some(0)), warnings: warnings, })) }) diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 6df9a921e79..829af62da7b 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -101,7 +101,7 @@ fn new_crate(name: &str) -> u::NewCrate { } #[test] -fn index() { +fn index_sql_test() { let (_b, app, middle) = ::app(); let mut req = ::req(app.clone(), Method::Get, "/api/v1/crates"); let mut response = ok_resp!(middle.call(&mut req)); From 6036807ced3b09adf6a780e33be2c12754fa0968 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Fri, 14 Jul 2017 11:03:20 -0400 Subject: [PATCH 14/37] Fixes error in tests where crates with zero downloads could not be grabbed --- src/krate.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/src/krate.rs b/src/krate.rs index 033b691319a..4651ea8ca8a 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -744,7 +744,7 @@ impl Model for Crate { /// Handles the `GET /crates` route. pub fn index(req: &mut Request) -> CargoResult { use diesel::expression::{AsExpression, DayAndMonthIntervalDsl}; - use diesel::types::{Bool, BigInt}; + use diesel::types::{Bool, BigInt, Nullable}; use diesel::expression::functions::date_and_time::{now, date}; use diesel::expression::sql_literal::sql; use diesel::query_source::joins::LeftOuter; @@ -754,7 +754,7 @@ pub fn index(req: &mut Request) -> CargoResult { let params = req.query(); let sort = params.get("sort").map(|s| &**s).unwrap_or("alpha"); - let recent_downloads = sql::("SUM(crate_downloads.downloads)"); + let recent_downloads = sql::>("SUM(crate_downloads.downloads)"); let mut query = crates::table .join(crate_downloads::table, LeftOuter, crates::id.eq(crate_downloads::crate_id).and(crate_downloads::date.gt(date(now - 90.days())))) @@ -851,8 +851,7 @@ pub fn index(req: &mut Request) -> CargoResult { } println!("crate count: {:?}", crates::table.count().get_result::(&*conn)); - - let data = query.paginate(limit, offset).load::<((Crate, bool, i64), i64)>( + let data = query.paginate(limit, offset).load::<((Crate, bool, Option), i64)>( &*conn, )?; let total = data.first().map(|&(_, t)| t).unwrap_or(0); @@ -860,7 +859,7 @@ pub fn index(req: &mut Request) -> CargoResult { .map(|&((ref c, _, _), _)| c.clone()) .collect::>(); let perfect_matches = data.clone().into_iter().map(|((_, b, _), _)| b).collect::>(); - let recent_downloads = data.clone().into_iter().map(|((_, _, s), _)| s).collect::>(); + let recent_downloads = data.clone().into_iter().map(|((_, _, s), _)| s.unwrap_or(0)).collect::>(); let versions = Version::belonging_to(&crates) .load::(&*conn)? From 121fdbf9962226558f0687c2f3a045add313d3f1 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Mon, 17 Jul 2017 17:01:33 -0400 Subject: [PATCH 15/37] tests for adding crates when sort = recent-downloads --- src/krate.rs | 2 +- src/tests/all.rs | 36 ++++++++++- src/tests/krate.rs | 145 ++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 178 insertions(+), 5 deletions(-) diff --git a/src/krate.rs b/src/krate.rs index 4651ea8ca8a..003e33a295f 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -40,7 +40,7 @@ use util::{RequestUtils, CargoResult, internal, ChainError, human}; use version::{EncodableVersion, NewVersion}; use {Model, User, Keyword, Version, Category, Badge, Replica}; -#[derive(Queryable, Identifiable, Associations, AsChangeset)] +#[derive(Insertable, Queryable, Identifiable, Associations, AsChangeset)] #[belongs_to(Crate)] #[primary_key(crate_id, date)] #[table_name="crate_downloads"] diff --git a/src/tests/all.rs b/src/tests/all.rs index f4dec1e5b8f..9ed34ca3136 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -1,4 +1,4 @@ -#![deny(warnings)] +//#![deny(warnings)] #[macro_use] extern crate serde_derive; @@ -7,6 +7,7 @@ extern crate diesel; extern crate diesel_codegen; extern crate bufstream; extern crate cargo_registry; +extern crate chrono; extern crate conduit; extern crate conduit_middleware; extern crate conduit_test; @@ -32,7 +33,7 @@ use cargo_registry::category::NewCategory; use cargo_registry::db::{self, RequestTransaction}; use cargo_registry::dependency::NewDependency; use cargo_registry::keyword::Keyword; -use cargo_registry::krate::NewCrate; +use cargo_registry::krate::{NewCrate, CrateDownload}; use cargo_registry::schema::dependencies; use cargo_registry::upload as u; use cargo_registry::user::NewUser; @@ -308,6 +309,7 @@ struct CrateBuilder<'a> { owner_id: i32, krate: NewCrate<'a>, downloads: Option, + recent_downloads: Option, versions: Vec>, keywords: Vec<&'a str>, } @@ -321,6 +323,7 @@ impl<'a> CrateBuilder<'a> { ..NewCrate::default() }, downloads: None, + recent_downloads: None, versions: Vec::new(), keywords: Vec::new(), } @@ -356,6 +359,11 @@ impl<'a> CrateBuilder<'a> { self } + fn recent_downloads(mut self, recent_downloads: i32) -> Self { + self.recent_downloads = Some(recent_downloads); + self + } + fn version>>(mut self, version: T) -> Self { self.versions.push(version.into()); self @@ -367,17 +375,39 @@ impl<'a> CrateBuilder<'a> { } fn build(mut self, connection: &PgConnection) -> CargoResult { - use diesel::update; + use diesel::{insert, update}; let mut krate = self.krate.create_or_update(connection, None, self.owner_id)?; // Since we are using `NewCrate`, we can't set all the // crate properties in a single DB call. + + let old_downloads = self.downloads.unwrap_or(0) - self.recent_downloads.unwrap_or(0); + let now = chrono::Utc::now(); + let old_date = now.naive_utc().date() - chrono::Duration::days(91); + if let Some(downloads) = self.downloads { + let crate_download = CrateDownload { + crate_id: krate.id, + downloads: old_downloads, + date: old_date, + }; + + insert(&crate_download).into(crate_downloads::table).execute(connection)?; krate.downloads = downloads; update(&krate).set(&krate).execute(connection)?; } + if let Some(recent_downloads) = self.recent_downloads { + let crate_download = CrateDownload { + crate_id: krate.id, + downloads: self.recent_downloads.unwrap(), + date: now.naive_utc().date(), + }; + + insert(&crate_download).into(crate_downloads::table).execute(connection)?; + } + if self.versions.is_empty() { self.versions.push(VersionBuilder::new("0.99.0")); } diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 829af62da7b..f5cfefd3e9b 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -101,7 +101,7 @@ fn new_crate(name: &str) -> u::NewCrate { } #[test] -fn index_sql_test() { +fn index() { let (_b, app, middle) = ::app(); let mut req = ::req(app.clone(), Method::Get, "/api/v1/crates"); let mut response = ok_resp!(middle.call(&mut req)); @@ -1920,3 +1920,146 @@ fn check_ownership_one_crate() { assert_eq!(json.users[0].kind, "user"); assert_eq!(json.users[0].name, user.name); } + +/* Test recent download count + insert crate_downloads for a crate + 2 crates: + - Date more than 90 days ago and number + - Date less than 90 days ago and number: + verify index returns recent downloads only + verify index returns total downloads that is the total +*/ +#[test] +fn test_recent_download_count() { + let (_b, app, middle) = ::app(); + + { + let conn = app.diesel_database.get().unwrap(); + let user = ::new_user("Oskar").create_or_update(&conn).unwrap(); + + // More than 90 days ago + ::CrateBuilder::new("green_ball", user.id) + .description("For fetching") + .downloads(10) + .recent_downloads(0) + .expect_build(&conn); + + ::CrateBuilder::new("sweet_potato_snack", user.id) + .description("For when better than usual") + .downloads(5) + .recent_downloads(2) + .expect_build(&conn); + } + + let mut req = ::req(app, Method::Get, "/api/v1/crates"); + let mut response = ok_resp!(middle.call(req.with_query("sort=recent-downloads"))); + let json:CrateList = ::json(&mut response); + + assert_eq!(json.meta.total, 2); + + assert_eq!(json.crates[0].name, "sweet_potato_snack"); + assert_eq!(json.crates[1].name, "green_ball"); + + assert_eq!(json.crates[0].recent_downloads, Some(2)); + assert_eq!(json.crates[0].downloads, 5); + + assert_eq!(json.crates[1].recent_downloads, Some(0)); + assert_eq!(json.crates[1].downloads, 10); +} + +/* Test zero downloads + - Crate should show up in index results + - Shows up with 0 counts for downloads + */ +#[test] +fn test_zero_downloads() { + let (_b, app, middle) = ::app(); + + { + let conn = app.diesel_database.get().unwrap(); + let user = ::new_user("Oskar").create_or_update(&conn).unwrap(); + + // More than 90 days ago + ::CrateBuilder::new("green_ball", user.id) + .description("For fetching") + .downloads(0) + .recent_downloads(0) + .expect_build(&conn); + } + + let mut req = ::req(app, Method::Get, "/api/v1/crates"); + let mut response = ok_resp!(middle.call(req.with_query("sort=recent-downloads"))); + let json:CrateList = ::json(&mut response); + + assert_eq!(json.meta.total, 1); + + assert_eq!(json.crates[0].name, "green_ball"); + assert_eq!(json.crates[0].recent_downloads, Some(0)); + assert_eq!(json.crates[0].downloads, 0); +} + +/* Test that index for categories and keywords is sorted + by recent downloads by default + - set up 2 crates, one with more all-time, one with + more recent + + recent total + c1 10 10 + c2 0 50 +*/ +/*#[test] +fn test_default_sort_recent() { + let (_b, app, middle) = ::app(); + + { + let conn = app.diesel_database.get().unwrap(); + let user = ::new_user("Oskar").create_or_update(&conn).unwrap(); + + // More than 90 days ago + ::CrateBuilder::new("green_ball", user.id) + .description("For fetching") + .downloads(10) + .recent_downloads(10) + .expect_build(&conn); + + ::CrateBuilder::new("sweet_potato_snack", user.id) + .description("For when better than usual") + .downloads(20) + .recent_downloads(0) + .expect_build(&conn); + } + + // test that index for categories is sorted by recent_downloads + // by default + let mut req = ::req(app.clone(), Method::Get, "/api/v1/categories"); + let mut response = ok_resp!(middle.call(&mut req)); + let json:CrateList = ::json(&mut response); + + assert_eq!(json.meta.total, 2); + + assert_eq!(json.crates[0].name, "green_ball"); + assert_eq!(json.crates[1].name, "sweet_potato_snack"); + + assert_eq!(json.crates[0].recent_downloads, Some(10)); + assert_eq!(json.crates[0].downloads, 10); + + assert_eq!(json.crates[1].recent_downloads, Some(0)); + assert_eq!(json.crates[1].downloads, 20); + + // test that index for keywords is sorted by recent_downloads + // by default + let mut req = ::req(app.clone(), Method::Get, "/api/v1/keywords"); + let mut response = ok_resp!(middle.call(&mut req)); + let json:CrateList = ::json(&mut response); + + assert_eq!(json.meta.total, 2); + + assert_eq!(json.crates[0].name, "green_ball"); + assert_eq!(json.crates[1].name, "sweet_potato_snack"); + + assert_eq!(json.crates[0].recent_downloads, Some(10)); + assert_eq!(json.crates[0].downloads, 10); + + assert_eq!(json.crates[1].recent_downloads, Some(0)); + assert_eq!(json.crates[1].downloads, 20); +}*/ From 7d66ac021463f9ef8cec37cdd3fdeb932c1a3b37 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 12:42:10 -0400 Subject: [PATCH 16/37] Change default sort from alpha to recent-downloads overall, alpha to relevance with search query --- src/krate.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/krate.rs b/src/krate.rs index 003e33a295f..177b243525a 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -752,7 +752,7 @@ pub fn index(req: &mut Request) -> CargoResult { let conn = req.db_conn()?; let (offset, limit) = req.pagination(10, 100)?; let params = req.query(); - let sort = params.get("sort").map(|s| &**s).unwrap_or("alpha"); + let sort = params.get("sort").map(|s| &**s).unwrap_or("recent-downloads"); let recent_downloads = sql::>("SUM(crate_downloads.downloads)"); @@ -771,6 +771,7 @@ pub fn index(req: &mut Request) -> CargoResult { } if let Some(q_string) = params.get("q") { + let sort = params.get("sort").map(|s| &**s).unwrap_or("relevance"); let q = plainto_tsquery(q_string); query = query.filter(q.matches(crates::textsearchable_index_col).or( crates::name.eq( From 8946d2a09aa58ce5f33d42d3492daf1e598a248b Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 12:43:43 -0400 Subject: [PATCH 17/37] Add test for checking that the keyword index page and category index page are default sorted by recent downloads --- src/tests/krate.rs | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/src/tests/krate.rs b/src/tests/krate.rs index f5cfefd3e9b..5857630bce2 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -2007,32 +2007,36 @@ fn test_zero_downloads() { c1 10 10 c2 0 50 */ -/*#[test] +#[test] fn test_default_sort_recent() { let (_b, app, middle) = ::app(); - { + let (green_crate, potato_crate) = { let conn = app.diesel_database.get().unwrap(); let user = ::new_user("Oskar").create_or_update(&conn).unwrap(); // More than 90 days ago - ::CrateBuilder::new("green_ball", user.id) + let green_crate = ::CrateBuilder::new("green_ball", user.id) .description("For fetching") + .keyword("dog") .downloads(10) .recent_downloads(10) .expect_build(&conn); - ::CrateBuilder::new("sweet_potato_snack", user.id) + let potato_crate = ::CrateBuilder::new("sweet_potato_snack", user.id) .description("For when better than usual") + .keyword("dog") .downloads(20) .recent_downloads(0) .expect_build(&conn); - } - // test that index for categories is sorted by recent_downloads + (green_crate, potato_crate) + }; + + // test that index for keywords is sorted by recent_downloads // by default - let mut req = ::req(app.clone(), Method::Get, "/api/v1/categories"); - let mut response = ok_resp!(middle.call(&mut req)); + let mut req = ::req(app.clone(), Method::Get, "/api/v1/crates"); + let mut response = ok_resp!(middle.call(req.with_query("keyword=dog"))); let json:CrateList = ::json(&mut response); assert_eq!(json.meta.total, 2); @@ -2046,10 +2050,18 @@ fn test_default_sort_recent() { assert_eq!(json.crates[1].recent_downloads, Some(0)); assert_eq!(json.crates[1].downloads, 20); - // test that index for keywords is sorted by recent_downloads + { + let conn = app.diesel_database.get().unwrap(); + ::new_category("Animal", "animal") + .create_or_update(&conn) + .unwrap(); + Category::update_crate(&conn, &green_crate, &["animal"]).unwrap(); + Category::update_crate(&conn, &potato_crate, &["animal"]).unwrap(); + } + + // test that index for categories is sorted by recent_downloads // by default - let mut req = ::req(app.clone(), Method::Get, "/api/v1/keywords"); - let mut response = ok_resp!(middle.call(&mut req)); + let mut response = ok_resp!(middle.call(req.with_query("category=animal"))); let json:CrateList = ::json(&mut response); assert_eq!(json.meta.total, 2); @@ -2062,4 +2074,4 @@ fn test_default_sort_recent() { assert_eq!(json.crates[1].recent_downloads, Some(0)); assert_eq!(json.crates[1].downloads, 20); -}*/ +} From 549683ff749f431c7a5de2445e6483d6ab3c86ba Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 13:07:22 -0400 Subject: [PATCH 18/37] Add test to check that when searching exact match is first and the list of crates returned is sorted by recent-downloads --- src/tests/krate.rs | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 5857630bce2..09ef24d908b 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -311,11 +311,13 @@ fn exact_match_on_queries_with_sort() { ::CrateBuilder::new("foo_sort", user.id) .description("bar_sort baz_sort const") .downloads(50) + .recent_downloads(50) .expect_build(&conn); ::CrateBuilder::new("bar_sort", user.id) .description("foo_sort baz_sort foo_sort baz_sort const") .downloads(3333) + .recent_downloads(0) .expect_build(&conn); ::CrateBuilder::new("baz_sort", user.id) @@ -323,6 +325,7 @@ fn exact_match_on_queries_with_sort() { "foo_sort bar_sort foo_sort bar_sort foo_sort bar_sort const", ) .downloads(100000) + .recent_downloads(10) .expect_build(&conn); ::CrateBuilder::new("other_sort", user.id) @@ -331,6 +334,7 @@ fn exact_match_on_queries_with_sort() { .expect_build(&conn); } + // Sort by downloads let mut req = ::req(app, Method::Get, "/api/v1/crates"); let mut response = ok_resp!(middle.call(req.with_query("q=foo_sort&sort=downloads"))); let json: CrateList = ::json(&mut response); @@ -360,6 +364,14 @@ fn exact_match_on_queries_with_sort() { assert_eq!(json.crates[1].name, "baz_sort"); assert_eq!(json.crates[2].name, "bar_sort"); assert_eq!(json.crates[3].name, "foo_sort"); + + // Sort by recent-downloads + let mut response = ok_resp!(middle.call(req.with_query("q=bar_sort&sort=recent-downloads"))); + let json: CrateList = ::json(&mut response); + assert_eq!(json.meta.total, 3); + assert_eq!(json.crates[0].name, "bar_sort"); + assert_eq!(json.crates[1].name, "foo_sort"); + assert_eq!(json.crates[2].name, "baz_sort"); } #[test] From 15975d11d5c8ad341d6c0a78392f86f7a92b0f4a Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 13:09:57 -0400 Subject: [PATCH 19/37] delete debug statement, clean up join function --- src/krate.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/krate.rs b/src/krate.rs index 177b243525a..21229d5c0db 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -757,7 +757,13 @@ pub fn index(req: &mut Request) -> CargoResult { let recent_downloads = sql::>("SUM(crate_downloads.downloads)"); let mut query = crates::table - .join(crate_downloads::table, LeftOuter, crates::id.eq(crate_downloads::crate_id).and(crate_downloads::date.gt(date(now - 90.days())))) + .join( + crate_downloads::table, + LeftOuter, + crates::id + .eq(crate_downloads::crate_id) + .and(crate_downloads::date.gt(date(now - 90.days()))) + ) .group_by(crates::id) .select((ALL_COLUMNS, AsExpression::::as_expression(false), recent_downloads.clone())) .into_boxed(); @@ -851,7 +857,6 @@ pub fn index(req: &mut Request) -> CargoResult { )); } - println!("crate count: {:?}", crates::table.count().get_result::(&*conn)); let data = query.paginate(limit, offset).load::<((Crate, bool, Option), i64)>( &*conn, )?; From 6609e04ca6ca3f6d62c187182b099a97e32cf03b Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 13:23:48 -0400 Subject: [PATCH 20/37] add comments for tests written --- src/tests/krate.rs | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 09ef24d908b..8f6d8263b71 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -1933,13 +1933,12 @@ fn check_ownership_one_crate() { assert_eq!(json.users[0].name, user.name); } -/* Test recent download count - insert crate_downloads for a crate - 2 crates: - - Date more than 90 days ago and number - - Date less than 90 days ago and number: - verify index returns recent downloads only - verify index returns total downloads that is the total +/* Given two crates, one with downloads less than 90 days ago, the + other with all downloads greater than 90 days ago, check that + the order returned is by recent downloads, descending. Check + also that recent download counts are returned in recent_downloads, + and total downloads counts are returned in downloads, and that + these numbers do not overlap. */ #[test] fn test_recent_download_count() { @@ -1979,9 +1978,9 @@ fn test_recent_download_count() { assert_eq!(json.crates[1].downloads, 10); } -/* Test zero downloads - - Crate should show up in index results - - Shows up with 0 counts for downloads +/* Given one crate with zero downloads, check that the crate + still shows up in index results, but that it displays 0 + for both recent downloads and downloads. */ #[test] fn test_zero_downloads() { @@ -2010,14 +2009,9 @@ fn test_zero_downloads() { assert_eq!(json.crates[0].downloads, 0); } -/* Test that index for categories and keywords is sorted - by recent downloads by default - - set up 2 crates, one with more all-time, one with - more recent - - recent total - c1 10 10 - c2 0 50 +/* Given two crates, one with more all-time downloads, the other with + more downloads in the past 90 days, check that the index page for + categories and keywords is sorted by recent downlaods by default. */ #[test] fn test_default_sort_recent() { From 0fb2e1826ca2270b914d7517b2b0561979262a39 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 13:53:33 -0400 Subject: [PATCH 21/37] Add recent download option to category index page, change default sort to recent-downloads --- app/controllers/category/index.js | 12 ++++++++++-- app/templates/category/index.hbs | 7 ++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/app/controllers/category/index.js b/app/controllers/category/index.js index 52aed1e844c..88f33032255 100644 --- a/app/controllers/category/index.js +++ b/app/controllers/category/index.js @@ -7,7 +7,7 @@ export default Controller.extend(PaginationMixin, { queryParams: ['page', 'per_page', 'sort'], page: '1', per_page: 10, - sort: 'downloads', + sort: 'recent-downloads', totalItems: computed.readOnly('model.meta.total'), @@ -15,6 +15,14 @@ export default Controller.extend(PaginationMixin, { category: computed.alias('categoryController.model'), currentSortBy: computed('sort', function() { - return (this.get('sort') === 'downloads') ? 'Downloads' : 'Alphabetical'; + if (this.get('sort') === 'downloads') { + return 'All Time Downloads'; + } else if (this.get('sort') === 'recent-downloads') { + return 'Recent Downloads'; + } else if (this.get('sort') === 'alpha') { + return 'Alphabetical'; + } else { + return 'Recent Downloads'; + } }), }); diff --git a/app/templates/category/index.hbs b/app/templates/category/index.hbs index da7ceab66b8..def0cf3f9ca 100644 --- a/app/templates/category/index.hbs +++ b/app/templates/category/index.hbs @@ -61,7 +61,12 @@
  • {{#link-to (query-params sort="downloads")}} - Downloads + All Time Downloads + {{/link-to}} +
    • +
  • + {{#link-to (query-params sort="recent-downloads")}} + Recent Downloads {{/link-to}}
    • {{/rl-dropdown}} From e11724a73400c00c6bfa298f95f65a480df5745a Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 13:54:43 -0400 Subject: [PATCH 22/37] Add recent download option to keyword index page, change default sort to recent-downloads --- app/controllers/keyword/index.js | 12 ++++++++++-- app/templates/keyword/index.hbs | 7 ++++++- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/app/controllers/keyword/index.js b/app/controllers/keyword/index.js index c0650b00664..f7a42711e12 100644 --- a/app/controllers/keyword/index.js +++ b/app/controllers/keyword/index.js @@ -7,11 +7,19 @@ export default Controller.extend(PaginationMixin, { queryParams: ['page', 'per_page', 'sort'], page: '1', per_page: 10, - sort: 'alpha', + sort: 'recent-downloads', totalItems: computed.readOnly('model.meta.total'), currentSortBy: computed('sort', function() { - return (this.get('sort') === 'downloads') ? 'Downloads' : 'Alphabetical'; + if (this.get('sort') === 'downloads') { + return "All Time Downloads"; + } else if (this.get('sort') === 'recent-downloads') { + return "Recent Downloads"; + } else if (this.get('sort') === 'alpha') { + return "Alphabetical"; + } else { + return "Recent Downloads"; + } }), }); diff --git a/app/templates/keyword/index.hbs b/app/templates/keyword/index.hbs index 352cf8d1459..3a1d3ac9477 100644 --- a/app/templates/keyword/index.hbs +++ b/app/templates/keyword/index.hbs @@ -32,7 +32,12 @@
  • {{#link-to (query-params sort="downloads")}} - Downloads + All Time Downloads + {{/link-to}} +
    • +
  • + {{#link-to (query-params sort="recent-downloads")}} + Recent Downloads {{/link-to}}
    • {{/rl-dropdown}} From 34a30d5df19a7e8e77f0c28257cce5dac1e008b6 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 16:07:54 -0400 Subject: [PATCH 23/37] add recent_downloads to fixture data --- mirage/fixtures/crates.js | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/mirage/fixtures/crates.js b/mirage/fixtures/crates.js index 2bb1d4ea037..0b55068e662 100644 --- a/mirage/fixtures/crates.js +++ b/mirage/fixtures/crates.js @@ -6,6 +6,7 @@ export default [{ "description": "A high-level, Rust idiomatic wrapper around nanomsg.", "documentation": "https://github.com/thehydroimpulse/nanomsg.rs", "downloads": 3888, + "recent_downloads": 800, "homepage": "https://github.com/thehydroimpulse/nanomsg.rs", "id": "nanomsg", "keywords": [ @@ -47,6 +48,7 @@ export default [{ "description": "Yo dawg, use Rust to generate Rust, right in your Rust. (See\n`external_mixin` to use scripting languages.)\n", "documentation": "https://github.com/huonw/external_mixin#rust_mixin", "downloads": 477, + "recent_downloads": 100, "exact_match": true, "homepage": "https://github.com/huonw/external_mixin", "id": "rust_mixin", @@ -109,6 +111,7 @@ export default [{ "description": "Use your favourite interpreted language to generate your Rust, right\nin your Rust. Supports Python, Ruby and shell (`sh`) out of the box,\nwith an extensible macro to support any others. (See `rust_mixin` to\nbe able to use your all-time favourite language to generate your Rust.)\n", "documentation": "https://github.com/huonw/external_mixin#external_mixin", "downloads": 497, + "recent_downloads": 497, "homepage": "https://github.com/huonw/external_mixin", "id": "external_mixin", "keywords": ["python", "ruby", "shell", "plugin", "code-generation"], @@ -122,6 +125,7 @@ export default [{ "description": "Backing library for `rust_mixin` and `external_mixin` to keep them\nDRY.\n", "documentation": "https://github.com/huonw/external_mixin#external_mixin_base", "downloads": 989, + "recent_downloads": 0, "homepage": "https://github.com/huonw/external_mixin", "id": "external_mixin_umbrella", "keywords": ["plugin", "code-generation"], @@ -135,6 +139,7 @@ export default [{ "description": "Adds String based inflections for Rust. Snake, kebab, camel, sentence, class, title, upper, and lower cases as well as ordinalize, deordinalize, demodulize, and foreign key are supported as both traits and pure functions acting on String types.\n", "documentation": "http://whatisinternet.github.io/inflector/doc/inflector/", "downloads": 57, + "recent_downloads": 1, "homepage": "https://github.com/whatisinternet/inflector", "id": "Inflector", "keywords": ["string", "case", "camel", "snake", "inflection"], @@ -148,6 +153,7 @@ export default [{ "description": "Client for the ElasticSearch REST API", "documentation": "http://benashford.github.io/rs-es/rs_es/index.html", "downloads": 321, + "recent_downloads": 21, "homepage": null, "id": "rs-es", "keywords": ["elasticsearch", "elastic"], @@ -161,6 +167,7 @@ export default [{ "description": "A (mostly) pure-Rust implementation of various common cryptographic algorithms.", "documentation": null, "downloads": 21573, + "recent_downloads": 2000, "homepage": "https://github.com/DaGenix/rust-crypto/", "id": "rust-crypto", "keywords": ["Crypto", "MD5", "Sha1", "Sha2", "AES"], @@ -174,6 +181,7 @@ export default [{ "description": "This library provides HTSlib bindings and a high level Rust API for reading and writing BAM files.", "documentation": null, "downloads": 485, + "recent_downloads": 85, "homepage": null, "id": "rust-htslib", "keywords": ["htslib", "bam", "bioinformatics", "pileup", "sequencing"], @@ -187,6 +195,7 @@ export default [{ "description": "A Kinetic protocol library written in Rust", "documentation": "https://icorderi.github.io/kinetic-rust/doc/kinetic/", "downloads": 225, + "recent_downloads": 125, "homepage": "https://icorderi.github.io/icorderi/kinetic-rust", "id": "kinetic-rust", "keywords": ["Protocol", "Kinetic", "Storage"], @@ -200,6 +209,7 @@ export default [{ "description": "Rustless is a REST-like API micro-framework for Rust.", "documentation": null, "downloads": 554, + "recent_downloads": 500, "homepage": "https://github.com/rustless/rustless", "id": "rustless", "keywords": ["api", "web", "hyper", "iron", "rest"], @@ -213,6 +223,7 @@ export default [{ "description": "A generic serialization/deserialization framework", "documentation": "https://serde-rs.github.io/serde/serde/serde/index.html", "downloads": 50854, + "recent_downloads": 854, "homepage": null, "id": "serde", "keywords": ["serde", "serialization"], @@ -226,6 +237,7 @@ export default [{ "description": "Send cypher queries to a neo4j database", "documentation": "http://livioribeiro.github.io/rusted_cypher/rusted_cypher/", "downloads": 156, + "recent_downloads": 54, "homepage": "https://github.com/livioribeiro/rusted-cypher", "id": "rusted_cypher", "keywords": ["neo4j", "database", "query", "cypher", "graph"], @@ -239,6 +251,7 @@ export default [{ "description": "An (incomplete) port of zlib to Rust. The decompressor works, but the compressor has not yet been ported.", "documentation": null, "downloads": 223, + "recent_downloads": 23, "homepage": null, "id": "zlib", "keywords": [], @@ -252,6 +265,7 @@ export default [{ "description": "A light HTTP framework, with some REST-like features and the ambition of being simple, modular and non-intrusive.", "documentation": "http://ogeon.github.io/docs/rustful/master/rustful/index.html", "downloads": 576, + "recent_downloads": 76, "homepage": null, "id": "rustful", "keywords": ["web", "rest", "framework", "http", "routing"], @@ -265,6 +279,7 @@ export default [{ "description": "A native PostgreSQL driver", "documentation": "https://sfackler.github.io/rust-postgres/doc/v0.10.1/postgres", "downloads": 13449, + "recent_downloads": 13, "homepage": null, "id": "postgres", "keywords": ["database", "sql"], @@ -278,6 +293,7 @@ export default [{ "description": "Automatic property based testing with shrinking.", "documentation": "http://burntsushi.net/rustdoc/quickcheck/", "downloads": 19271, + "recent_downloads": 143, "homepage": "https://github.com/BurntSushi/quickcheck", "id": "quickcheck", "keywords": ["testing", "quickcheck", "property", "shrinking", "fuzz"], @@ -291,6 +307,7 @@ export default [{ "description": "A macro attribute for quickcheck.", "documentation": "http://burntsushi.net/rustdoc/quickcheck/", "downloads": 3796, + "recent_downloads": 768, "homepage": "https://github.com/BurntSushi/quickcheck", "id": "quickcheck_macros", "keywords": ["testing", "quickcheck", "property", "shrinking", "fuzz"], @@ -304,6 +321,7 @@ export default [{ "description": "Lexical analysers generator for Rust, written in Rust (crate dedicated to rumblebars, divergences written by Nicolas Cherel)", "documentation": null, "downloads": 109, + "recent_downloads": 0, "homepage": "https://github.com/nicolas-cherel/rustlex", "id": "nc_rustlex", "keywords": ["lexer", "lexical", "analyser", "generator"], @@ -317,6 +335,7 @@ export default [{ "description": "A byte-oriented, zero-copy, parser combinators library", "documentation": "http://rust.unhandledexpression.com/nom/", "downloads": 5169, + "recent_downloads": 69, "homepage": null, "id": "nom", "keywords": ["parser", "parser-combinators", "parsing", "streaming", "bit"], From d3f1e1680e181b384cbff587d261fc89fb05548b Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 16:09:49 -0400 Subject: [PATCH 24/37] fix double quote linting error --- app/controllers/keyword/index.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/keyword/index.js b/app/controllers/keyword/index.js index f7a42711e12..9f2a3d2c782 100644 --- a/app/controllers/keyword/index.js +++ b/app/controllers/keyword/index.js @@ -13,13 +13,13 @@ export default Controller.extend(PaginationMixin, { currentSortBy: computed('sort', function() { if (this.get('sort') === 'downloads') { - return "All Time Downloads"; + return 'All Time Downloads'; } else if (this.get('sort') === 'recent-downloads') { - return "Recent Downloads"; + return 'Recent Downloads'; } else if (this.get('sort') === 'alpha') { - return "Alphabetical"; + return 'Alphabetical'; } else { - return "Recent Downloads"; + return 'Recent Downloads'; } }), }); From b1a9aa5acb966e6886466077ff9004854dc8df73 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 16:11:46 -0400 Subject: [PATCH 25/37] frontend tests for displaying and sorting by recent downloads --- mirage/config.js | 6 ++++++ tests/acceptance/categories-test.js | 9 +++++++++ tests/acceptance/crates-test.js | 25 +++++++++++++++++++++++++ tests/acceptance/keyword-test.js | 14 ++++++++++++++ tests/acceptance/search-test.js | 14 ++++++++++++++ 5 files changed, 68 insertions(+) create mode 100644 tests/acceptance/keyword-test.js diff --git a/mirage/config.js b/mirage/config.js index 49671f2fb53..a75eae275ed 100644 --- a/mirage/config.js +++ b/mirage/config.js @@ -170,6 +170,12 @@ export default function() { return withMeta(this.serialize(categories), { total }); }); + this.get('/categories/:category_id', function(schema, request) { + let catId = request.params.category_id; + let category = schema.categories.find(catId); + return category ? category : notFound(); + }); + this.get('/keywords', function(schema, request) { let { start, end } = pageParams(request); diff --git a/tests/acceptance/categories-test.js b/tests/acceptance/categories-test.js index dc23ac70593..32e57755f6c 100644 --- a/tests/acceptance/categories-test.js +++ b/tests/acceptance/categories-test.js @@ -15,3 +15,12 @@ test('listing categories', async function(assert) { hasText(assert, '.row:eq(1) .desc .info span', '1 crate'); hasText(assert, '.row:eq(2) .desc .info span', '3,910 crates'); }); + +test('category/:category_id index default sort is recent-downloads', async function(assert) { + server.create('category', { category: 'Algorithms', crates_cnt: 1 }); + + await visit('/categories/algorithms'); + + const $sort = findWithAssert('div.sort div.dropdown-container a.dropdown'); + hasText(assert, $sort, 'Recent Downloads'); +}); diff --git a/tests/acceptance/crates-test.js b/tests/acceptance/crates-test.js index df57654b806..b282e166b43 100644 --- a/tests/acceptance/crates-test.js +++ b/tests/acceptance/crates-test.js @@ -43,3 +43,28 @@ test('navigating to next page of crates', async function(assert) { hasText(assert, '.amt.small .cur', '11-19'); hasText(assert, '.amt.small .total', '19'); }); + +test('crates default sort is alphabetical', async function(assert) { + server.loadFixtures(); + + await visit('/crates'); + + const $sort = findWithAssert('div.sort div.dropdown-container a.dropdown'); + hasText(assert, $sort, 'Alphabetical'); +}); + +test('downloads appears for each crate on crate list', async function(assert) { + server.loadFixtures(); + + await visit('/crates'); + const $recentDownloads = findWithAssert('div.downloads:first span.num'); + hasText(assert, $recentDownloads, 'All-Time: 497'); +}); + +test('recent downloads appears for each crate on crate list', async function(assert) { + server.loadFixtures(); + + await visit('/crates'); + const $recentDownloads = findWithAssert('div.recent-downloads:first span.num'); + hasText(assert, $recentDownloads, 'Recent:'); +}); diff --git a/tests/acceptance/keyword-test.js b/tests/acceptance/keyword-test.js new file mode 100644 index 00000000000..b89a2b94060 --- /dev/null +++ b/tests/acceptance/keyword-test.js @@ -0,0 +1,14 @@ +import { test } from 'qunit'; +import moduleForAcceptance from 'cargo/tests/helpers/module-for-acceptance'; +import hasText from 'cargo/tests/helpers/has-text'; + +moduleForAcceptance('Acceptance | keywords'); + +test('keyword/:keyword_id index default sort is recent-downloads', async function(assert) { + server.create('keyword', { id: 'network', keyword: 'network', crates_cnt: 38 }); + + await visit('/keywords/network'); + + const $sort = findWithAssert('div.sort div.dropdown-container a.dropdown'); + hasText(assert, $sort, 'Recent Downloads'); +}); diff --git a/tests/acceptance/search-test.js b/tests/acceptance/search-test.js index 4df580b9e7a..10066be8a04 100644 --- a/tests/acceptance/search-test.js +++ b/tests/acceptance/search-test.js @@ -55,3 +55,17 @@ test('pressing S key to focus the search bar', async function(assert) { await keyEvent(document, 'keydown', KEYCODE_S); assertSearchBarIsFocused(); }); + +test('check search results are by default displayed by relevance', async function(assert) { + server.loadFixtures(); + + await visit('/'); + await fillIn('input.search', 'rust'); + + findWithAssert('form.search').submit(); + + await wait(); + + const $sort = findWithAssert('div.sort div.dropdown-container a.dropdown'); + hasText(assert, $sort, 'Relevance'); +}); From 625f04ce6ad373e1eb50d96bd465cedd9ccc4582 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 16:22:19 -0400 Subject: [PATCH 26/37] change 'All Time' to 'All-Time' for consistency --- app/controllers/category/index.js | 2 +- app/controllers/keyword/index.js | 2 +- app/controllers/team.js | 2 +- app/controllers/user.js | 2 +- app/templates/category/index.hbs | 2 +- app/templates/keyword/index.hbs | 2 +- app/templates/team.hbs | 2 +- app/templates/user.hbs | 2 +- 8 files changed, 8 insertions(+), 8 deletions(-) diff --git a/app/controllers/category/index.js b/app/controllers/category/index.js index 88f33032255..82a54a76888 100644 --- a/app/controllers/category/index.js +++ b/app/controllers/category/index.js @@ -16,7 +16,7 @@ export default Controller.extend(PaginationMixin, { currentSortBy: computed('sort', function() { if (this.get('sort') === 'downloads') { - return 'All Time Downloads'; + return 'All-Time Downloads'; } else if (this.get('sort') === 'recent-downloads') { return 'Recent Downloads'; } else if (this.get('sort') === 'alpha') { diff --git a/app/controllers/keyword/index.js b/app/controllers/keyword/index.js index 9f2a3d2c782..9325858ffcc 100644 --- a/app/controllers/keyword/index.js +++ b/app/controllers/keyword/index.js @@ -13,7 +13,7 @@ export default Controller.extend(PaginationMixin, { currentSortBy: computed('sort', function() { if (this.get('sort') === 'downloads') { - return 'All Time Downloads'; + return 'All-Time Downloads'; } else if (this.get('sort') === 'recent-downloads') { return 'Recent Downloads'; } else if (this.get('sort') === 'alpha') { diff --git a/app/controllers/team.js b/app/controllers/team.js index 0a973616d72..5b559694086 100644 --- a/app/controllers/team.js +++ b/app/controllers/team.js @@ -13,7 +13,7 @@ export default Controller.extend(PaginationMixin, { currentSortBy: computed('sort', function() { if (this.get('sort') === 'downloads') { - return 'All Time Downloads'; + return 'All-Time Downloads'; } else if (this.get('sort') === 'recent-downloads') { return 'Recent Downloads'; } else { diff --git a/app/controllers/user.js b/app/controllers/user.js index 91f4f5f3deb..ab3088fd4e9 100644 --- a/app/controllers/user.js +++ b/app/controllers/user.js @@ -14,7 +14,7 @@ export default Controller.extend(PaginationMixin, { currentSortBy: computed('sort', function() { if (this.get('sort') === 'downloads') { - return 'All Time Downloads'; + return 'All-Time Downloads'; } else if (this.get('sort') === 'recent-downloads') { return 'Recent Downloads'; } else { diff --git a/app/templates/category/index.hbs b/app/templates/category/index.hbs index def0cf3f9ca..d77f15916ce 100644 --- a/app/templates/category/index.hbs +++ b/app/templates/category/index.hbs @@ -61,7 +61,7 @@
  • {{#link-to (query-params sort="downloads")}} - All Time Downloads + All-Time Downloads {{/link-to}}
  • diff --git a/app/templates/keyword/index.hbs b/app/templates/keyword/index.hbs index 3a1d3ac9477..5aac1585c99 100644 --- a/app/templates/keyword/index.hbs +++ b/app/templates/keyword/index.hbs @@ -32,7 +32,7 @@
  • {{#link-to (query-params sort="downloads")}} - All Time Downloads + All-Time Downloads {{/link-to}}
  • diff --git a/app/templates/team.hbs b/app/templates/team.hbs index de9faf4c5c7..aeca821014c 100644 --- a/app/templates/team.hbs +++ b/app/templates/team.hbs @@ -47,7 +47,7 @@
  • {{#link-to (query-params sort="downloads")}} - All Time Downloads + All-Time Downloads {{/link-to}}
  • diff --git a/app/templates/user.hbs b/app/templates/user.hbs index ff70a442d5e..3602d54277f 100644 --- a/app/templates/user.hbs +++ b/app/templates/user.hbs @@ -38,7 +38,7 @@
  • {{#link-to (query-params sort="downloads")}} - All Time Downloads + All-Time Downloads {{/link-to}}
  • From 9970d6d1568c836f9ee067df656a120602d20c60 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 18 Jul 2017 16:39:32 -0400 Subject: [PATCH 27/37] remove unused variable --- src/tests/all.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/all.rs b/src/tests/all.rs index 9ed34ca3136..d4cd046a31d 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -398,7 +398,7 @@ impl<'a> CrateBuilder<'a> { update(&krate).set(&krate).execute(connection)?; } - if let Some(recent_downloads) = self.recent_downloads { + if self.recent_downloads.is_some() { let crate_download = CrateDownload { crate_id: krate.id, downloads: self.recent_downloads.unwrap(), From e9cdc7693fe5072a6727e789966616d193b29323 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Wed, 19 Jul 2017 10:37:19 -0400 Subject: [PATCH 28/37] cargo fmt --- src/krate.rs | 55 +++++++++++++++++++++++++++++++++------------- src/tests/all.rs | 8 +++++-- src/tests/krate.rs | 12 +++++----- 3 files changed, 53 insertions(+), 22 deletions(-) diff --git a/src/krate.rs b/src/krate.rs index 21229d5c0db..e4d15f0a902 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -43,7 +43,7 @@ use {Model, User, Keyword, Version, Category, Badge, Replica}; #[derive(Insertable, Queryable, Identifiable, Associations, AsChangeset)] #[belongs_to(Crate)] #[primary_key(crate_id, date)] -#[table_name="crate_downloads"] +#[table_name = "crate_downloads"] pub struct CrateDownload { pub crate_id: i32, pub downloads: i32, @@ -498,7 +498,15 @@ impl Crate { exact_match: bool, recent_downloads: Option, ) -> EncodableCrate { - self.encodable(max_version, None, None, None, badges, exact_match, recent_downloads) + self.encodable( + max_version, + None, + None, + None, + badges, + exact_match, + recent_downloads, + ) } pub fn encodable( @@ -752,7 +760,9 @@ pub fn index(req: &mut Request) -> CargoResult { let conn = req.db_conn()?; let (offset, limit) = req.pagination(10, 100)?; let params = req.query(); - let sort = params.get("sort").map(|s| &**s).unwrap_or("recent-downloads"); + let sort = params.get("sort").map(|s| &**s).unwrap_or( + "recent-downloads", + ); let recent_downloads = sql::>("SUM(crate_downloads.downloads)"); @@ -760,12 +770,16 @@ pub fn index(req: &mut Request) -> CargoResult { .join( crate_downloads::table, LeftOuter, - crates::id - .eq(crate_downloads::crate_id) - .and(crate_downloads::date.gt(date(now - 90.days()))) - ) + crates::id.eq(crate_downloads::crate_id).and( + crate_downloads::date.gt(date(now - 90.days())), + ), + ) .group_by(crates::id) - .select((ALL_COLUMNS, AsExpression::::as_expression(false), recent_downloads.clone())) + .select(( + ALL_COLUMNS, + AsExpression::::as_expression(false), + recent_downloads.clone(), + )) .into_boxed(); if sort == "downloads" { @@ -785,7 +799,11 @@ pub fn index(req: &mut Request) -> CargoResult { ), )); - query = query.select((ALL_COLUMNS, crates::name.eq(q_string), recent_downloads.clone())); + query = query.select(( + ALL_COLUMNS, + crates::name.eq(q_string), + recent_downloads.clone(), + )); let perfect_match = crates::name.eq(q_string).desc(); if sort == "downloads" { query = query.order((perfect_match, crates::downloads.desc())); @@ -857,15 +875,21 @@ pub fn index(req: &mut Request) -> CargoResult { )); } - let data = query.paginate(limit, offset).load::<((Crate, bool, Option), i64)>( - &*conn, - )?; + let data = query + .paginate(limit, offset) + .load::<((Crate, bool, Option), i64)>(&*conn)?; let total = data.first().map(|&(_, t)| t).unwrap_or(0); let crates = data.iter() .map(|&((ref c, _, _), _)| c.clone()) .collect::>(); - let perfect_matches = data.clone().into_iter().map(|((_, b, _), _)| b).collect::>(); - let recent_downloads = data.clone().into_iter().map(|((_, _, s), _)| s.unwrap_or(0)).collect::>(); + let perfect_matches = data.clone() + .into_iter() + .map(|((_, b, _), _)| b) + .collect::>(); + let recent_downloads = data.clone() + .into_iter() + .map(|((_, _, s), _)| s.unwrap_or(0)) + .collect::>(); let versions = Version::belonging_to(&crates) .load::(&*conn)? @@ -877,7 +901,8 @@ pub fn index(req: &mut Request) -> CargoResult { .zip(crates) .zip(perfect_matches) .zip(recent_downloads) - .map(|(((max_version, krate), perfect_match), recent_downloads)| { + .map(|(((max_version, krate), perfect_match), + recent_downloads)| { // FIXME: If we add crate_id to the Badge enum we can eliminate // this N+1 let badges = badges::table diff --git a/src/tests/all.rs b/src/tests/all.rs index d4cd046a31d..679de95cee4 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -393,7 +393,9 @@ impl<'a> CrateBuilder<'a> { date: old_date, }; - insert(&crate_download).into(crate_downloads::table).execute(connection)?; + insert(&crate_download) + .into(crate_downloads::table) + .execute(connection)?; krate.downloads = downloads; update(&krate).set(&krate).execute(connection)?; } @@ -405,7 +407,9 @@ impl<'a> CrateBuilder<'a> { date: now.naive_utc().date(), }; - insert(&crate_download).into(crate_downloads::table).execute(connection)?; + insert(&crate_download) + .into(crate_downloads::table) + .execute(connection)?; } if self.versions.is_empty() { diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 8f6d8263b71..a53d2f4c220 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -366,7 +366,9 @@ fn exact_match_on_queries_with_sort() { assert_eq!(json.crates[3].name, "foo_sort"); // Sort by recent-downloads - let mut response = ok_resp!(middle.call(req.with_query("q=bar_sort&sort=recent-downloads"))); + let mut response = ok_resp!(middle.call( + req.with_query("q=bar_sort&sort=recent-downloads"), + )); let json: CrateList = ::json(&mut response); assert_eq!(json.meta.total, 3); assert_eq!(json.crates[0].name, "bar_sort"); @@ -1964,7 +1966,7 @@ fn test_recent_download_count() { let mut req = ::req(app, Method::Get, "/api/v1/crates"); let mut response = ok_resp!(middle.call(req.with_query("sort=recent-downloads"))); - let json:CrateList = ::json(&mut response); + let json: CrateList = ::json(&mut response); assert_eq!(json.meta.total, 2); @@ -2000,7 +2002,7 @@ fn test_zero_downloads() { let mut req = ::req(app, Method::Get, "/api/v1/crates"); let mut response = ok_resp!(middle.call(req.with_query("sort=recent-downloads"))); - let json:CrateList = ::json(&mut response); + let json: CrateList = ::json(&mut response); assert_eq!(json.meta.total, 1); @@ -2043,7 +2045,7 @@ fn test_default_sort_recent() { // by default let mut req = ::req(app.clone(), Method::Get, "/api/v1/crates"); let mut response = ok_resp!(middle.call(req.with_query("keyword=dog"))); - let json:CrateList = ::json(&mut response); + let json: CrateList = ::json(&mut response); assert_eq!(json.meta.total, 2); @@ -2068,7 +2070,7 @@ fn test_default_sort_recent() { // test that index for categories is sorted by recent_downloads // by default let mut response = ok_resp!(middle.call(req.with_query("category=animal"))); - let json:CrateList = ::json(&mut response); + let json: CrateList = ::json(&mut response); assert_eq!(json.meta.total, 2); From b086a6da0efc9e72098a628b78bb98c750eadadd Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 25 Jul 2017 13:58:09 -0400 Subject: [PATCH 29/37] fixes bug where null downloads would be listed first when sorting by descending --- src/krate.rs | 4 ++-- src/tests/krate.rs | 13 +++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/krate.rs b/src/krate.rs index e4d15f0a902..312c8f46f09 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -785,7 +785,7 @@ pub fn index(req: &mut Request) -> CargoResult { if sort == "downloads" { query = query.order(crates::downloads.desc()) } else if sort == "recent-downloads" { - query = query.order(recent_downloads.clone().desc()) + query = query.order(recent_downloads.clone().desc().nulls_last()) } else { query = query.order(crates::name.asc()) } @@ -808,7 +808,7 @@ pub fn index(req: &mut Request) -> CargoResult { if sort == "downloads" { query = query.order((perfect_match, crates::downloads.desc())); } else if sort == "recent-downloads" { - query = query.order((perfect_match, recent_downloads.clone().desc())); + query = query.order((perfect_match, recent_downloads.clone().desc().nulls_last())); } else { let rank = ts_rank_cd(crates::textsearchable_index_col, q); query = query.order((perfect_match, rank.desc())) diff --git a/src/tests/krate.rs b/src/tests/krate.rs index a53d2f4c220..6e19197bc58 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -374,6 +374,19 @@ fn exact_match_on_queries_with_sort() { assert_eq!(json.crates[0].name, "bar_sort"); assert_eq!(json.crates[1].name, "foo_sort"); assert_eq!(json.crates[2].name, "baz_sort"); + + // Test for bug with showing null results first when sorting + // by descending + // This has nothing to do with querying for exact match I'm sorry + let mut response = ok_resp!(middle.call( + req.with_query("sort=recent-downloads"), + )); + let json: CrateList = ::json(&mut response); + assert_eq!(json.meta.total, 4); + assert_eq!(json.crates[0].name, "foo_sort"); + assert_eq!(json.crates[1].name, "baz_sort"); + assert_eq!(json.crates[2].name, "bar_sort"); + assert_eq!(json.crates[3].name, "other_sort"); } #[test] From d5455cc179cc4e4c7dddd0106c25b73e8c142161 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 25 Jul 2017 13:59:30 -0400 Subject: [PATCH 30/37] put back deny warnings --- src/tests/all.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/tests/all.rs b/src/tests/all.rs index 679de95cee4..d53a1379e16 100644 --- a/src/tests/all.rs +++ b/src/tests/all.rs @@ -1,4 +1,4 @@ -//#![deny(warnings)] +#![deny(warnings)] #[macro_use] extern crate serde_derive; From fb719cc537ece855052098037385f19d9569ddcc Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 25 Jul 2017 14:01:30 -0400 Subject: [PATCH 31/37] Add Debug to CrateDownload struct --- src/krate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/krate.rs b/src/krate.rs index 312c8f46f09..8f5e72fa4ff 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -40,7 +40,7 @@ use util::{RequestUtils, CargoResult, internal, ChainError, human}; use version::{EncodableVersion, NewVersion}; use {Model, User, Keyword, Version, Category, Badge, Replica}; -#[derive(Insertable, Queryable, Identifiable, Associations, AsChangeset)] +#[derive(Debug, Insertable, Queryable, Identifiable, Associations, AsChangeset)] #[belongs_to(Crate)] #[primary_key(crate_id, date)] #[table_name = "crate_downloads"] From b13c4327ba973a060cb1ed282d6f748e1d21b324 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 25 Jul 2017 14:22:52 -0400 Subject: [PATCH 32/37] add recent_downloads to crate serializer, update test to match --- mirage/serializers/crate.js | 1 + tests/acceptance/crates-test.js | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/mirage/serializers/crate.js b/mirage/serializers/crate.js index fca074d67af..87ec6db827c 100644 --- a/mirage/serializers/crate.js +++ b/mirage/serializers/crate.js @@ -8,6 +8,7 @@ export default BaseSerializer.extend({ 'description', 'documentation', 'downloads', + 'recent_downloads', 'homepage', 'id', 'keywords', diff --git a/tests/acceptance/crates-test.js b/tests/acceptance/crates-test.js index b282e166b43..594f30cfa68 100644 --- a/tests/acceptance/crates-test.js +++ b/tests/acceptance/crates-test.js @@ -66,5 +66,5 @@ test('recent downloads appears for each crate on crate list', async function(ass await visit('/crates'); const $recentDownloads = findWithAssert('div.recent-downloads:first span.num'); - hasText(assert, $recentDownloads, 'Recent:'); + hasText(assert, $recentDownloads, 'Recent: 497'); }); From 1c3f3c60ca55be4cb141a7d7b413c7ba414fe15c Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 25 Jul 2017 14:27:39 -0400 Subject: [PATCH 33/37] delete duplicate code & whitespace --- app/controllers/category/index.js | 2 -- app/controllers/keyword/index.js | 2 -- app/styles/crate.scss | 1 - 3 files changed, 5 deletions(-) diff --git a/app/controllers/category/index.js b/app/controllers/category/index.js index 82a54a76888..079a61cecca 100644 --- a/app/controllers/category/index.js +++ b/app/controllers/category/index.js @@ -17,8 +17,6 @@ export default Controller.extend(PaginationMixin, { currentSortBy: computed('sort', function() { if (this.get('sort') === 'downloads') { return 'All-Time Downloads'; - } else if (this.get('sort') === 'recent-downloads') { - return 'Recent Downloads'; } else if (this.get('sort') === 'alpha') { return 'Alphabetical'; } else { diff --git a/app/controllers/keyword/index.js b/app/controllers/keyword/index.js index 9325858ffcc..76deffc376d 100644 --- a/app/controllers/keyword/index.js +++ b/app/controllers/keyword/index.js @@ -14,8 +14,6 @@ export default Controller.extend(PaginationMixin, { currentSortBy: computed('sort', function() { if (this.get('sort') === 'downloads') { return 'All-Time Downloads'; - } else if (this.get('sort') === 'recent-downloads') { - return 'Recent Downloads'; } else if (this.get('sort') === 'alpha') { return 'Alphabetical'; } else { diff --git a/app/styles/crate.scss b/app/styles/crate.scss index 4c0f7078441..9a39f5c60ee 100644 --- a/app/styles/crate.scss +++ b/app/styles/crate.scss @@ -144,7 +144,6 @@ .recent-downloads { @include display-flex; @include align-items(center); - } .rev-dep-downloads {padding-left: 7px} From 20f42d979a082715d597a87a9d6518470e21e3d9 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 25 Jul 2017 14:33:16 -0400 Subject: [PATCH 34/37] allow too many arguments on krate::encodable --- src/krate.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/krate.rs b/src/krate.rs index 8f5e72fa4ff..8a01d257cba 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -509,6 +509,7 @@ impl Crate { ) } + #[allow(too_many_arguments)] pub fn encodable( self, max_version: semver::Version, From 1acfbef72335e54ee07846f7e99a36054f3183ae Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 25 Jul 2017 14:39:59 -0400 Subject: [PATCH 35/37] cargo fmt --- src/krate.rs | 5 ++++- src/tests/krate.rs | 4 +--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/krate.rs b/src/krate.rs index 8a01d257cba..290833fcc9c 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -809,7 +809,10 @@ pub fn index(req: &mut Request) -> CargoResult { if sort == "downloads" { query = query.order((perfect_match, crates::downloads.desc())); } else if sort == "recent-downloads" { - query = query.order((perfect_match, recent_downloads.clone().desc().nulls_last())); + query = query.order(( + perfect_match, + recent_downloads.clone().desc().nulls_last(), + )); } else { let rank = ts_rank_cd(crates::textsearchable_index_col, q); query = query.order((perfect_match, rank.desc())) diff --git a/src/tests/krate.rs b/src/tests/krate.rs index 6e19197bc58..0eaad61eff0 100644 --- a/src/tests/krate.rs +++ b/src/tests/krate.rs @@ -378,9 +378,7 @@ fn exact_match_on_queries_with_sort() { // Test for bug with showing null results first when sorting // by descending // This has nothing to do with querying for exact match I'm sorry - let mut response = ok_resp!(middle.call( - req.with_query("sort=recent-downloads"), - )); + let mut response = ok_resp!(middle.call(req.with_query("sort=recent-downloads"))); let json: CrateList = ::json(&mut response); assert_eq!(json.meta.total, 4); assert_eq!(json.crates[0].name, "foo_sort"); From dfb21b87f957498ca4caba08fffb6b85316a4273 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 25 Jul 2017 15:04:35 -0400 Subject: [PATCH 36/37] really allow too many arguments on krate::encodable --- src/krate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/krate.rs b/src/krate.rs index 290833fcc9c..75c66b85952 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -509,7 +509,7 @@ impl Crate { ) } - #[allow(too_many_arguments)] + #[cfg_attr(feature = "cargo-clippy", allow(too_many_arguments))] pub fn encodable( self, max_version: semver::Version, From 8c5845193045b635d14ecc880763c86bc8b443d3 Mon Sep 17 00:00:00 2001 From: Natalie Boehm Date: Tue, 25 Jul 2017 16:05:12 -0400 Subject: [PATCH 37/37] actually allow too many arguments on krate::encodable (change feature=cargo-clippy to feature=clippy) --- src/krate.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/krate.rs b/src/krate.rs index 75c66b85952..7f6554d7b37 100644 --- a/src/krate.rs +++ b/src/krate.rs @@ -509,7 +509,7 @@ impl Crate { ) } - #[cfg_attr(feature = "cargo-clippy", allow(too_many_arguments))] + #[cfg_attr(feature = "clippy", allow(too_many_arguments))] pub fn encodable( self, max_version: semver::Version,