Auto merge of #2214 - RobbieClarken:sort-by-newly-added, r=jtgeibel

bors · bors · commit a72846f9e1f3 · 2020-02-25T01:27:00.000Z
Enable sorting crates by most recently added This change adds the option to sort crates by "Newly Added" on the following pages: - `/crates` - `/search` - `/users/:user_id` - `/teams/:team_id` - `/keywords/:keyword_id` - `/categories/:category_id` - `/me/crates` Sorting is performed using the `crates.created_at` column. I've put "Newly Added" as the last option in the dropdown: <img width="258" alt="sort-by-newly-added" src="https://user-images.githubusercontent.com/663161/75087201-2a7ff500-558d-11ea-896b-2e82de37811d.png"> Let me know if you would like a different label or for the order to be different. I couldn't find any existing frontend tests of changing sort order. I'd be happy to add some as part of this PR if they would be valuable. Closes #1919.
diff --git a/app/controllers/category/index.js b/app/controllers/category/index.js
@@ -19,6 +19,8 @@ export default Controller.extend(PaginationMixin, {
       return 'All-Time Downloads';
     } else if (this.sort === 'alpha') {
       return 'Alphabetical';
+    } else if (this.sort === 'new') {
+      return 'Newly Added';
     } else if (this.get('sort') === 'recent-updates') {
       return 'Recent Updates';
     } else {
diff --git a/app/controllers/crates.js b/app/controllers/crates.js
@@ -21,6 +21,8 @@ export default Controller.extend(PaginationMixin, {
       return 'Recent Downloads';
     } else if (this.get('sort') === 'recent-updates') {
       return 'Recent Updates';
+    } else if (this.sort === 'new') {
+      return 'Newly Added';
     } else {
       return 'Alphabetical';
     }
diff --git a/app/controllers/keyword/index.js b/app/controllers/keyword/index.js
@@ -17,6 +17,8 @@ export default Controller.extend(PaginationMixin, {
       return 'All-Time Downloads';
     } else if (this.sort === 'alpha') {
       return 'Alphabetical';
+    } else if (this.sort === 'new') {
+      return 'Newly Added';
     } else if (this.get('sort') === 'recent-updates') {
       return 'Recent Updates';
     } else {
diff --git a/app/controllers/me/crates.js b/app/controllers/me/crates.js
@@ -21,6 +21,8 @@ export default Controller.extend(PaginationMixin, {
       return 'Recent Downloads';
     } else if (this.get('sort') === 'recent-updates') {
       return 'Recent Updates';
+    } else if (this.sort === 'new') {
+      return 'Newly Added';
     } else {
       return 'Alphabetical';
     }
diff --git a/app/controllers/search.js b/app/controllers/search.js
@@ -33,6 +33,8 @@ export default Controller.extend(PaginationMixin, {
       return 'Recent Downloads';
     } else if (this.get('sort') === 'recent-updates') {
       return 'Recent Updates';
+    } else if (this.sort === 'new') {
+      return 'Newly Added';
     } else {
       return 'Relevance';
     }
diff --git a/app/controllers/team.js b/app/controllers/team.js
@@ -19,6 +19,8 @@ export default Controller.extend(PaginationMixin, {
       return 'Recent Downloads';
     } else if (this.get('sort') === 'recent-updates') {
       return 'Recent Updates';
+    } else if (this.sort === 'new') {
+      return 'Newly Added';
     } else {
       return 'Alphabetical';
     }
diff --git a/app/controllers/user.js b/app/controllers/user.js
@@ -20,6 +20,8 @@ export default Controller.extend(PaginationMixin, {
       return 'Recent Downloads';
     } else if (this.get('sort') === 'recent-updates') {
       return 'Recent Updates';
+    } else if (this.sort === 'new') {
+      return 'Newly Added';
     } else {
       return 'Alphabetical';
     }
diff --git a/app/templates/category/index.hbs b/app/templates/category/index.hbs
@@ -77,6 +77,11 @@
             Recent Updates
           </LinkTo>
         </li>
+        <li>
+          <LinkTo @query={{hash sort="new"}}>
+            Newly Added
+          </LinkTo>
+        </li>
       </dd.Content>
     </Dropdown>
   </div>
@@ -88,4 +93,4 @@
   {{/each}}
 </div>
 
-<Pagination @pages={{this.pages}} @prevPage={{this.prevPage}} @nextPage={{this.nextPage}} />
+<Pagination @pages={{this.pages}} @prevPage={{this.prevPage}} @nextPage={{this.nextPage}} />
diff --git a/app/templates/crates.hbs b/app/templates/crates.hbs
@@ -65,6 +65,11 @@
             Recent Updates
           </LinkTo>
         </li>
+        <li>
+          <LinkTo @query={{hash page=1 sort="new"}}>
+            Newly Added
+          </LinkTo>
+        </li>
       </dd.Content>
     </Dropdown>
   </div>
diff --git a/app/templates/keyword/index.hbs b/app/templates/keyword/index.hbs
@@ -45,6 +45,11 @@
             Recent Updates
           </LinkTo>
         </li>
+        <li>
+          <LinkTo @query={{hash sort="new"}}>
+            Newly Added
+          </LinkTo>
+        </li>
       </dd.Content>
     </Dropdown>
   </div>
diff --git a/app/templates/me/crates.hbs b/app/templates/me/crates.hbs
@@ -46,6 +46,11 @@
             Recent Updates
           </LinkTo>
         </li>
+        <li>
+          <LinkTo @query={{hash sort="new"}}>
+            Newly Added
+          </LinkTo>
+        </li>
       </dd.Content>
     </Dropdown>
   </div>
@@ -57,4 +62,4 @@
   {{/each}}
 </div>
 
-<Pagination @pages={{this.pages}} @prevPage={{this.prevPage}} @nextPage={{this.nextPage}} />
+<Pagination @pages={{this.pages}} @prevPage={{this.prevPage}} @nextPage={{this.nextPage}} />
diff --git a/app/templates/search.hbs b/app/templates/search.hbs
@@ -57,6 +57,11 @@
               Recent Updates
             </LinkTo>
           </li>
+          <li>
+            <LinkTo @query={{hash page=1 sort="new"}}>
+              Newly Added
+            </LinkTo>
+          </li>
         </dd.Content>
       </Dropdown>
     </div>
diff --git a/app/templates/team.hbs b/app/templates/team.hbs
@@ -60,6 +60,11 @@
                 Recent Updates
               </LinkTo>
             </li>
+            <li>
+              <LinkTo @query={{hash sort="new"}}>
+                Newly Added
+              </LinkTo>
+            </li>
           </dd.Content>
         </Dropdown>
       </div>
diff --git a/app/templates/user.hbs b/app/templates/user.hbs
@@ -51,6 +51,11 @@
                 Recent Updates
               </LinkTo>
             </li>
+            <li>
+              <LinkTo @query={{hash sort="new"}}>
+                Newly Added
+              </LinkTo>
+            </li>
           </dd.Content>
         </Dropdown>
       </div>
diff --git a/src/controllers/krate/search.rs b/src/controllers/krate/search.rs
@@ -178,6 +178,8 @@ pub fn search(req: &mut dyn Request) -> AppResult<Response> {
         query = query.then_order_by(recent_crate_downloads::downloads.desc().nulls_last())
     } else if sort == Some("recent-updates") {
         query = query.order(crates::updated_at.desc());
+    } else if sort == Some("new") {
+        query = query.order(crates::created_at.desc());
     } else {
         query = query.then_order_by(crates::name.asc())
     }
diff --git a/src/tests/krate.rs b/src/tests/krate.rs
@@ -243,6 +243,7 @@ fn exact_match_first_on_queries() {
 }
 
 #[test]
+#[allow(clippy::cognitive_complexity)]
 fn index_sorting() {
     let (app, anon, user) = TestApp::init().with_user();
     let user = user.as_model();
@@ -271,6 +272,24 @@ fn index_sorting() {
             .downloads(999_999)
             .expect_build(conn);
 
+        // Set the created at column for each crate
+        update(&krate1)
+            .set(crates::created_at.eq(now - 4.weeks()))
+            .execute(conn)
+            .unwrap();
+        update(&krate2)
+            .set(crates::created_at.eq(now - 1.weeks()))
+            .execute(conn)
+            .unwrap();
+        update(&krate3)
+            .set(crates::created_at.eq(now - 2.weeks()))
+            .execute(conn)
+            .unwrap();
+        update(&krate4)
+            .set(crates::created_at.eq(now - 3.weeks()))
+            .execute(conn)
+            .unwrap();
+
         // Set the updated at column for each crate
         update(&krate1)
             .set(crates::updated_at.eq(now - 3.weeks()))
@@ -314,6 +333,14 @@ fn index_sorting() {
     assert_eq!(json.crates[2].name, "bar_sort");
     assert_eq!(json.crates[3].name, "foo_sort");
 
+    // Sort by new
+    let json = anon.search("sort=new");
+    assert_eq!(json.meta.total, 4);
+    assert_eq!(json.crates[0].name, "bar_sort");
+    assert_eq!(json.crates[1].name, "baz_sort");
+    assert_eq!(json.crates[2].name, "other_sort");
+    assert_eq!(json.crates[3].name, "foo_sort");
+
     // Test for bug with showing null results first when sorting
     // by descending downloads
     let json = anon.search("sort=recent-downloads");
@@ -354,6 +381,24 @@ fn exact_match_on_queries_with_sort() {
             .downloads(999_999)
             .expect_build(conn);
 
+        // Set the created at column for each crate
+        update(&krate1)
+            .set(crates::created_at.eq(now - 4.weeks()))
+            .execute(conn)
+            .unwrap();
+        update(&krate2)
+            .set(crates::created_at.eq(now - 1.weeks()))
+            .execute(conn)
+            .unwrap();
+        update(&krate3)
+            .set(crates::created_at.eq(now - 2.weeks()))
+            .execute(conn)
+            .unwrap();
+        update(&krate4)
+            .set(crates::created_at.eq(now - 3.weeks()))
+            .execute(conn)
+            .unwrap();
+
         // Set the updated at column for each crate
         update(&krate1)
             .set(crates::updated_at.eq(now - 3.weeks()))
@@ -413,6 +458,13 @@ fn exact_match_on_queries_with_sort() {
     assert_eq!(json.crates[1].name, "bar_sort");
     assert_eq!(json.crates[2].name, "foo_sort");
 
+    // Sort by new
+    let json = anon.search("q=bar_sort&sort=new");
+    assert_eq!(json.meta.total, 3);
+    assert_eq!(json.crates[0].name, "bar_sort");
+    assert_eq!(json.crates[1].name, "baz_sort");
+    assert_eq!(json.crates[2].name, "foo_sort");
+
     // Test for bug with showing null results first when sorting
     // by descending downloads
     let json = anon.search("sort=recent-downloads");
diff --git a/tests/acceptance/search-test.js b/tests/acceptance/search-test.js
@@ -37,7 +37,7 @@ module('Acceptance | search', function(hooks) {
     assert.dom('[data-test-search-nav]').hasText('Displaying 1-7 of 7 total results');
     assert
       .dom('[data-test-search-sort]')
-      .hasText('Sort by Relevance Relevance All-Time Downloads Recent Downloads Recent Updates');
+      .hasText('Sort by Relevance Relevance All-Time Downloads Recent Downloads Recent Updates Newly Added');
     assert.dom('[data-test-crate-row="0"] [data-test-crate-link]').hasText('kinetic-rust');
     assert.dom('[data-test-crate-row="0"] [data-test-version-badge]').hasAttribute('alt', '0.0.16');
 

Original file line number	Diff line number	Diff line change
`@@ -21,6 +21,8 @@ export default Controller.extend(PaginationMixin, {`
`21`	`21`	`return 'Recent Downloads';`
`22`	`22`	`} else if (this.get('sort') === 'recent-updates') {`
`23`	`23`	`return 'Recent Updates';`
	`24`	`+ } else if (this.sort === 'new') {`
	`25`	`+ return 'Newly Added';`
`24`	`26`	`} else {`
`25`	`27`	`return 'Alphabetical';`
`26`	`28`	`}`
Original file line number	Diff line number	Diff line change
`@@ -33,6 +33,8 @@ export default Controller.extend(PaginationMixin, {`
`33`	`33`	`return 'Recent Downloads';`
`34`	`34`	`} else if (this.get('sort') === 'recent-updates') {`
`35`	`35`	`return 'Recent Updates';`
	`36`	`+ } else if (this.sort === 'new') {`
	`37`	`+ return 'Newly Added';`
`36`	`38`	`} else {`
`37`	`39`	`return 'Relevance';`
`38`	`40`	`}`
Original file line number	Diff line number	Diff line change
`@@ -19,6 +19,8 @@ export default Controller.extend(PaginationMixin, {`
`19`	`19`	`return 'Recent Downloads';`
`20`	`20`	`} else if (this.get('sort') === 'recent-updates') {`
`21`	`21`	`return 'Recent Updates';`
	`22`	`+ } else if (this.sort === 'new') {`
	`23`	`+ return 'Newly Added';`
`22`	`24`	`} else {`
`23`	`25`	`return 'Alphabetical';`
`24`	`26`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,8 @@ export default Controller.extend(PaginationMixin, {`
`20`	`20`	`return 'Recent Downloads';`
`21`	`21`	`} else if (this.get('sort') === 'recent-updates') {`
`22`	`22`	`return 'Recent Updates';`
	`23`	`+ } else if (this.sort === 'new') {`
	`24`	`+ return 'Newly Added';`
`23`	`25`	`} else {`
`24`	`26`	`return 'Alphabetical';`
`25`	`27`	`}`