stop displaying and serving authorship information

revert owners fix order refine header comment test: add cases fix add author nope fix better msg fix rename api and route owner support without @ refine msg refine msg add more cases add owner add nonexistent_owner test rename fix test address comments fix test fix test add owners for topbar fmt code
rust-lang · jyn514 · Mar 22, 2021 · Mar 19, 2021 · Mar 22, 2021 · 68a8517f5f2b03e9e3e1d8bf75014e1d9dae7cf1
commit 68a8517f5f2b03e9e3e1d8bf75014e1d9dae7cf1
@@ -11,6 +11,7 @@ pub enum Nope {
  ResourceNotFound,
  BuildNotFound,
  CrateNotFound,
+ OwnerNotFound,
  VersionNotFound,
  NoResults,
  InternalServerError,
@@ -22,6 +23,7 @@ impl fmt::Display for Nope {
  Nope::ResourceNotFound => "Requested resource not found",
  Nope::BuildNotFound => "Requested build not found",
  Nope::CrateNotFound => "Requested crate not found",
+ Nope::OwnerNotFound => "Requested owner not found",
  Nope::VersionNotFound => "Requested crate does not have specified version",
  Nope::NoResults => "Search yielded no results",
  Nope::InternalServerError => "Internal server error",
@@ -39,6 +41,7 @@ impl From<Nope> for IronError {
  Nope::ResourceNotFound
  | Nope::BuildNotFound
  | Nope::CrateNotFound
+ | Nope::OwnerNotFound
  | Nope::VersionNotFound
  | Nope::NoResults => status::NotFound,
  Nope::InternalServerError => status::InternalServerError,
@@ -80,6 +83,13 @@ impl Handler for Nope {
  .into_response(req)
  }
 
+ Nope::OwnerNotFound => ErrorPage {
+ title: "The requested owner does not exist",
+ message: Some("no such owner".into()),
+ status: Status::NotFound,
+ }
+ .into_response(req),
+
  Nope::VersionNotFound => {
  // user tried to navigate to a crate with a version that does not exist
  // TODO: Display the attempted crate and version

@@ -109,61 +109,11 @@ pub(crate) fn get_releases(conn: &mut Client, page: i64, limit: i64, order: Orde
  .collect()
 }
 
-fn get_releases_by_author(
- conn: &mut Client,
- page: i64,
- limit: i64,
- author: &str,
-) -> (String, Vec<Release>) {
- let offset = (page - 1) * limit;
-
- let query = "
- SELECT crates.name,
- releases.version,
- releases.description,
- releases.target_name,
- releases.release_time,
- releases.rustdoc_status,
- github_repos.stars,
- authors.name
- FROM crates
- INNER JOIN releases ON releases.id = crates.latest_version_id
- INNER JOIN author_rels ON releases.id = author_rels.rid
- INNER JOIN authors ON authors.id = author_rels.aid
- LEFT JOIN github_repos ON releases.github_repo = github_repos.id
- WHERE authors.slug = $1
- ORDER BY github_repos.stars DESC NULLS LAST
- LIMIT $2 OFFSET $3";
- let query = conn.query(query, &[&author, &limit, &offset]).unwrap();
-
- let mut author_name = None;
- let packages = query
- .into_iter()
- .map(|row| {
- if author_name.is_none() {
- author_name = Some(row.get(7));
- }
-
- Release {
- name: row.get(0),
- version: row.get(1),
- description: row.get(2),
- target_name: row.get(3),
- release_time: row.get(4),
- rustdoc_status: row.get(5),
- stars: row.get::<_, Option<i32>>(6).unwrap_or(0),
- }
- })
- .collect();
-
- (author_name.unwrap_or_default(), packages)
-}
-
 fn get_releases_by_owner(
  conn: &mut Client,
  page: i64,
  limit: i64,
- author: &str,
+ owner: &str,
 ) -> (String, Vec<Release>) {
  let offset = (page - 1) * limit;
 
@@ -184,14 +134,14 @@ fn get_releases_by_owner(
  WHERE owners.login = $1
  ORDER BY github_repos.stars DESC NULLS LAST
  LIMIT $2 OFFSET $3";
- let query = conn.query(query, &[&author, &limit, &offset]).unwrap();
+ let query = conn.query(query, &[&owner, &limit, &offset]).unwrap();
 
- let mut author_name = None;
+ let mut owner_name = None;
  let packages = query
  .into_iter()
  .map(|row| {
- if author_name.is_none() {
- author_name = Some(if !row.get::<usize, String>(7).is_empty() {
+ if owner_name.is_none() {
+ owner_name = Some(if !row.get::<usize, String>(7).is_empty() {
  row.get(7)
  } else {
  row.get(8)
@@ -210,7 +160,7 @@ fn get_releases_by_owner(
  })
  .collect();
 
- (author_name.unwrap_or_default(), packages)
+ (owner_name.unwrap_or_default(), packages)
 }
 
 /// Get the search results for a crate search query
@@ -336,7 +286,7 @@ struct ViewReleases {
  show_next_page: bool,
  show_previous_page: bool,
  page_number: i64,
- author: Option<String>,
+ owner: Option<String>,
 }
 
 impl_webpage! {
@@ -350,7 +300,7 @@ pub(super) enum ReleaseType {
  Stars,
  RecentFailures,
  Failures,
- Author,
+ Owner,
  Search,
 }
 
@@ -369,8 +319,8 @@ fn releases_handler(req: &mut Request, release_type: ReleaseType) -> IronResult<
  Order::FailuresByGithubStars,
  ),
 
- ReleaseType::Author | ReleaseType::Search => panic!(
- "The authors and search page have special requirements and cannot use this handler",
+ ReleaseType::Owner | ReleaseType::Search => panic!(
+ "The owners and search page have special requirements and cannot use this handler",
  ),
  };
 
@@ -392,7 +342,7 @@ fn releases_handler(req: &mut Request, release_type: ReleaseType) -> IronResult<
  show_next_page,
  show_previous_page,
  page_number,
- author: None,
+ owner: None,
  }
  .into_response(req)
 }
@@ -413,39 +363,29 @@ pub fn releases_failures_by_stars_handler(req: &mut Request) -> IronResult<Respo
  releases_handler(req, ReleaseType::Failures)
 }
 
-pub fn author_handler(req: &mut Request) -> IronResult<Response> {
+pub fn owner_handler(req: &mut Request) -> IronResult<Response> {
  let router = extension!(req, Router);
  // page number of releases
  let page_number: i64 = router
  .find("page")
  .and_then(|page_num| page_num.parse().ok())
  .unwrap_or(1);
- let author = router
- .find("author")
- // TODO: Accurate error here, the author wasn't provided
- .ok_or(Nope::CrateNotFound)?;
+ let owner_route_value = router.find("owner").unwrap();
 
- let (author_name, releases) = {
+ let (owner_name, releases) = {
  let mut conn = extension!(req, Pool).get()?;
 
- if author.starts_with('@') {
- let mut author = author.split('@');
-
- get_releases_by_owner(
- &mut conn,
- page_number,
- RELEASES_IN_RELEASES,
- // TODO: Is this fallible?
- cexpect!(req, author.nth(1)),
- )
- } else {
- get_releases_by_author(&mut conn, page_number, RELEASES_IN_RELEASES, author)
+ // We need to keep the owner_route_value unchanged, as we may render paginated links in the page.
+ // Changing the owner_route_value directly will cause the link to change, for example: @foobar -> foobar.
+ let mut owner = owner_route_value;
+ if owner.starts_with('@') {
+ owner = &owner[1..];
  }
+ get_releases_by_owner(&mut conn, page_number, RELEASES_IN_RELEASES, owner)
  };
 
  if releases.is_empty() {
- // TODO: Accurate error here, the author wasn't found
- return Err(Nope::CrateNotFound.into());
+ return Err(Nope::OwnerNotFound.into());
  }
 
  // Show next and previous page buttons
@@ -456,12 +396,12 @@ pub fn author_handler(req: &mut Request) -> IronResult<Response> {
 
  ViewReleases {
  releases,
- description: format!("Crates from {}", author_name),
- release_type: ReleaseType::Author,
+ description: format!("Crates from {}", owner_name),
+ release_type: ReleaseType::Owner,
  show_next_page,
  show_previous_page,
  page_number,
- author: Some(author.into()),
+ owner: Some(owner_route_value.into()),
  }
  .into_response(req)
 }
@@ -754,6 +694,7 @@ pub fn build_queue_handler(req: &mut Request) -> IronResult<Response> {
 #[cfg(test)]
 mod tests {
  use super::*;
+ use crate::index::api::CrateOwner;
  use crate::test::{assert_redirect, assert_success, wrapper, TestFrontend};
  use chrono::{Duration, TimeZone};
  use failure::Error;
@@ -1446,29 +1387,75 @@ mod tests {
  }
 
  #[test]
- fn authors_page() {
+ fn nonexistent_owner_page() {
+ wrapper(|env| {
+ env.fake_release()
+ .name("some_random_crate")
+ .add_owner(CrateOwner {
+ login: "foobar".into(),
+ avatar: "https://example.org/foobar".into(),
+ name: "Foo Bar".into(),
+ email: "[email protected]".into(),
+ })
+ .create()?;
+ let page = kuchiki::parse_html().one(
+ env.frontend()
+ .get("/releases/random-author")
+ .send()?
+ .text()?,
+ );
+
+ assert_eq!(page.select("#crate-title").unwrap().count(), 1);
+ assert_eq!(
+ page.select("#crate-title")
+ .unwrap()
+ .next()
+ .unwrap()
+ .text_contents(),
+ "The requested owner does not exist",
+ );
+
+ Ok(())
+ });
+ }
+
+ #[test]
+ fn owners_page() {
  wrapper(|env| {
  let web = env.frontend();
  env.fake_release()
  .name("some_random_crate")
- .author("frankenstein <[email protected]>")
+ .add_owner(CrateOwner {
+ login: "foobar".into(),
+ avatar: "https://example.org/foobar".into(),
+ name: "Foo Bar".into(),
+ email: "[email protected]".into(),
+ })
  .create()?;
- assert_success("/releases/frankenstein", web)
+ // Request an owner without @ sign.
+ assert_success("/releases/foobar", web)?;
+ // Request an owner with @ sign.
+ assert_success("/releases/@foobar", web)
  })
  }
 
  #[test]
- fn authors_pagination() {
+ fn owners_pagination() {
  wrapper(|env| {
  let web = env.frontend();
  for i in 0..RELEASES_IN_RELEASES {
  env.fake_release()
  .name(&format!("some_random_crate_{}", i))
- .author("frankenstein <[email protected]")
+ .add_owner(CrateOwner {
+ login: "foobar".into(),
+ avatar: "https://example.org/foobar".into(),
+ name: "Foo Bar".into(),
+ email: "[email protected]".into(),
+ })
  .create()?;
  }
- let page = kuchiki::parse_html().one(web.get("/releases/frankenstein").send()?.text()?);
- let button = page.select_first("a[href='/releases/frankenstein/2']");
+ let page = kuchiki::parse_html().one(web.get("/releases/@foobar").send()?.text()?);
+ let button = page.select_first("a[href='/releases/@foobar/2']");
 
  assert!(button.is_ok());
 
@@ -1482,7 +1469,12 @@ mod tests {
  let web = env.frontend();
  env.fake_release()
  .name("some_random_crate")
- .author("frankenstein <[email protected]>")
+ .add_owner(CrateOwner {
+ login: "foobar".into(),
+ avatar: "https://example.org/foobar".into(),
+ name: "Foo Bar".into(),
+ email: "[email protected]".into(),
+ })
  .create()?;
 
  let mut urls = vec![];

@@ -41,8 +41,8 @@ pub(super) fn build_routes() -> Routes {
 
  routes.internal_page("/releases", super::releases::recent_releases_handler);
  routes.static_resource("/releases/feed", super::releases::releases_feed_handler);
- routes.internal_page("/releases/:author", super::releases::author_handler);
- routes.internal_page("/releases/:author/:page", super::releases::author_handler);
+ routes.internal_page("/releases/:owner", super::releases::owner_handler);
+ routes.internal_page("/releases/:owner/:page", super::releases::owner_handler);
  routes.internal_page("/releases/activity", super::releases::activity_handler);
  routes.internal_page("/releases/search", super::releases::search_handler);
  routes.internal_page("/releases/queue", super::releases::build_queue_handler);

@@ -38,16 +38,6 @@
  {%- endif -%}
  </li>
  {%- endif -%}
- {# List the release author's names and a link to their docs.rs profile #}
- <li class="pure-menu-heading">Authors</li>
- {%- for author in details.authors -%}
- <li class="pure-menu-item">
- <a href="/releases/{{ author[1] }}" class="pure-menu-link">
- {{ author[0] }}
- </a>
- </li>
- {%- endfor -%}
-
  <li class="pure-menu-heading">Links</li>
 
  {# If the crate has a homepage, show it #}

@@ -9,10 +9,9 @@
  * `failures`
  * `activity`
  * `queue`
- * `author`
- * `author` A string, used for the authors page
+ * `owner` A string, used for the owners page
 #}
-{% macro header(title, description, tab, author=false) %}
+{% macro header(title, description, tab, owner=false) %}
  <div class="cratesfyi-package-container">
  <div class="container">
  <div class="description-container">
@@ -68,11 +67,11 @@ <h1 id="crate-title">{{ title }}</h1>
  </a>
  </li>
 
- {%- if author -%}
+ {%- if owner -%}
  <li class="pure-menu-item">
- <a href="#" class="pure-menu-link{% if tab == 'author' %} pure-menu-active{% endif %}">
+ <a href="#" class="pure-menu-link{% if tab == 'owner' %} pure-menu-active{% endif %}">
  {{ "user" | fas(fw=true) }}
- <span class="title">{{ author }}</span>
+ <span class="title">{{ owner }}</span>
  </a>
  </li>
  {%- endif -%}