Skip to content

DO NOT MERGE until after 2019-03-01; Remove date checks for verified email requirement #1630

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 1, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/bin/update-downloads.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,9 +149,7 @@ mod test {
user_id,
)
.unwrap();
let version = version
.save(conn, &[], Some("[email protected]".into()))
.unwrap();
let version = version.save(conn, &[], "[email protected]").unwrap();
(krate, version)
}

Expand Down
116 changes: 11 additions & 105 deletions src/controllers/krate/publish.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,6 @@
use std::collections::HashMap;
use std::sync::Arc;

use chrono::offset::TimeZone;
use chrono::{DateTime, Utc};
use hex::ToHex;

use crate::git;
Expand Down Expand Up @@ -67,14 +65,13 @@ pub fn publish(req: &mut dyn Request) -> CargoResult<Response> {

let conn = app.diesel_database.get()?;

let mut other_warnings = vec![];
let verified_email_address = user.verified_email(&conn)?;

// This function can be inlined (with only the error-returning functionality) and its unit
// tests deleted after 2019-02-28; it was created to make injecting the date for tests easier.
// The integration tests in src/tests/krate.rs cover the current production behavior (and will
// need to be updated at that time)
verified_email_check(&mut other_warnings, &verified_email_address, Utc::now())?;
let verified_email_address = verified_email_address.ok_or_else(|| {
human(
"A verified email address is required to publish crates to crates.io. \
Visit https://crates.io/me to set and verify your email address.",
)
})?;

// Create a transaction on the database, if there are no errors,
// commit the transactions to record a new or updated crate.
Expand Down Expand Up @@ -150,7 +147,7 @@ pub fn publish(req: &mut dyn Request) -> CargoResult<Response> {
file_length as i32,
user.id,
)?
.save(&conn, &new_crate.authors, verified_email_address)?;
.save(&conn, &new_crate.authors, &verified_email_address)?;

// Link this new version to all dependencies
let git_deps = dependency::add_dependencies(&conn, &new_crate.deps, version.id)?;
Expand Down Expand Up @@ -210,10 +207,13 @@ pub fn publish(req: &mut dyn Request) -> CargoResult<Response> {
crate_bomb.path = None;
readme_bomb.path = None;

// The `other` field on `PublishWarnings` was introduced to handle a temporary warning
// that is no longer needed. As such, crates.io currently does not return any `other`
// warnings at this time, but if we need to, the field is available.
let warnings = PublishWarnings {
invalid_categories: ignored_invalid_categories,
invalid_badges: ignored_invalid_badges,
other: other_warnings,
other: vec![],
};

Ok(req.json(&GoodCrate {
Expand Down Expand Up @@ -268,97 +268,3 @@ fn parse_new_headers(req: &mut dyn Request) -> CargoResult<(EncodableCrateUpload
let user = req.user()?;
Ok((new, user.clone()))
}

fn verified_email_check(
other_warnings: &mut Vec<String>,
verified_email_address: &Option<String>,
now: DateTime<Utc>,
) -> CargoResult<()> {
match verified_email_address {
Some(_) => Ok(()),
None => {
if now < Utc.ymd(2019, 3, 1).and_hms(0, 0, 0) {
other_warnings.push(String::from(
"You do not currently have a verified email address associated with your \
crates.io account. Starting 2019-02-28, a verified email will be required to \
publish crates. Visit https://crates.io/me to set and verify your email \
address.",
));
Ok(())
} else {
Err(human(
"A verified email address is required to publish crates to crates.io. \
Visit https://crates.io/me to set and verify your email address.",
))
}
}
}
}

// These tests should be deleted after 2018-02-28; this functionality will then be covered by
// integration tests in src/tests/krate.rs.
#[cfg(test)]
mod tests {
use super::*;
use chrono::offset::TimeZone;

#[test]
fn allow_publish_with_verified_email_without_warning_before_2018_02_28() {
let mut warnings = vec![];

let fake_current_date = Utc.ymd(2019, 2, 27).and_hms(0, 0, 0);
let result = verified_email_check(
&mut warnings,
&Some("[email protected]".into()),
fake_current_date,
);

assert!(result.is_ok());
assert_eq!(warnings.len(), 0);
}

#[test]
fn allow_publish_with_verified_email_without_error_after_2018_02_28() {
let mut warnings = vec![];

let fake_current_date = Utc.ymd(2019, 3, 1).and_hms(0, 0, 0);
let result = verified_email_check(
&mut warnings,
&Some("[email protected]".into()),
fake_current_date,
);

assert!(result.is_ok());
assert_eq!(warnings.len(), 0);
}

#[test]
fn warn_without_verified_email_before_2018_02_28() {
let mut warnings = vec![];

let fake_current_date = Utc.ymd(2019, 2, 27).and_hms(0, 0, 0);
let result = verified_email_check(&mut warnings, &None, fake_current_date);

assert!(result.is_ok());
assert_eq!(warnings.len(), 1);
assert_eq!(warnings[0], "You do not currently have a verified email address associated \
with your crates.io account. Starting 2019-02-28, a verified email will be required to \
publish crates. Visit https://crates.io/me to set and verify your email address.");
}

#[test]
fn error_without_verified_email_after_2018_02_28() {
let mut warnings = vec![];

let fake_current_date = Utc.ymd(2019, 3, 1).and_hms(0, 0, 0);
let result = verified_email_check(&mut warnings, &None, fake_current_date);

assert!(result.is_err());
assert_eq!(
result.err().unwrap().description(),
"A verified email address is required to \
publish crates to crates.io. Visit https://crates.io/me to set and verify your email \
address."
);
}
}
18 changes: 7 additions & 11 deletions src/models/version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,12 +148,11 @@ impl NewVersion {
Ok(new_version)
}

// TODO: change `published_by_email` to be `String` after 2019-02-28
pub fn save(
&self,
conn: &PgConnection,
authors: &[String],
published_by_email: Option<String>,
published_by_email: &str,
) -> CargoResult<Version> {
use crate::schema::version_authors::{name, version_id};
use crate::schema::versions::dsl::*;
Expand All @@ -176,15 +175,12 @@ impl NewVersion {
.values(self)
.get_result::<Version>(conn)?;

// TODO: Remove the `if` around this insert after 2019-02-28
if let Some(published_by_email) = published_by_email {
insert_into(versions_published_by::table)
.values((
versions_published_by::version_id.eq(version.id),
versions_published_by::email.eq(published_by_email),
))
.execute(conn)?;
}
insert_into(versions_published_by::table)
.values((
versions_published_by::version_id.eq(version.id),
versions_published_by::email.eq(published_by_email),
))
.execute(conn)?;

let new_authors = authors
.iter()
Expand Down
2 changes: 1 addition & 1 deletion src/tests/builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ impl<'a> VersionBuilder<'a> {
self.size,
published_by,
)?
.save(connection, &[], Some("[email protected]".into()))?;
.save(connection, &[], "[email protected]")?;

if self.yanked {
vers = update(&vers)
Expand Down
73 changes: 0 additions & 73 deletions src/tests/http-data/krate_new_krate_with_unverified_email_warns

This file was deleted.

73 changes: 0 additions & 73 deletions src/tests/http-data/krate_new_krate_without_any_email_warns

This file was deleted.

Loading