Skip to content

Stop showing documentation link when documentation is removed from Cargo.toml #948

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 5 commits into from
Mar 15, 2018
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
1 change: 1 addition & 0 deletions src/models/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ type ByName<'a> = diesel::dsl::Filter<All, WithName<'a>>;

#[derive(Insertable, AsChangeset, Default, Debug)]
#[table_name = "crates"]
#[changeset_options(treat_none_as_null = "true")]
#[primary_key(name, max_upload_size)] // This is actually just to skip updating them
pub struct NewCrate<'a> {
pub name: &'a str,
Expand Down
11 changes: 11 additions & 0 deletions src/tests/all.rs
Original file line number Diff line number Diff line change
Expand Up @@ -548,6 +548,17 @@ fn new_req(app: Arc<App>, krate: &str, version: &str) -> MockRequest {
new_req_full(app, ::krate(krate), version, Vec::new())
}

fn new_req_with_documentation(
app: Arc<App>,
krate: &str,
version: &str,
documentation: &str,
) -> MockRequest {
let mut krate = ::krate(krate);
krate.documentation = Some(documentation.into());
new_req_full(app, krate, version, Vec::new())
}

fn new_req_full(
app: Arc<App>,
krate: Crate,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
[{"request":{"uri":"http://alexcrichton-test.s3.amazonaws.com/crates/docscrate/docscrate-0.2.1.crate","method":"PUT","headers":[["Proxy-Connection","Keep-Alive"],["Content-Type","application/x-tar"],["Accept","*/*"],["Date","Fri, 15 Sep 2017 07:53:06 -0700"],["Authorization","AWS AKIAICL5IWUZYWWKA7JA:RkpxRf+NUDyzjWrEMc1ikasbtZI="],["Content-Length","35"],["Host","alexcrichton-test.s3.amazonaws.com"]],"body":[31,139,8,0,0,0,0,0,0,255,237,192,1,1,0,0,0,130,32,255,175,110,72,80,192,171,1,46,175,181,239,0,4,0,0]},"response":{"status":200,"headers":[["ETag","\"f9016ad360cebb4fe2e6e96e5949f022\""],["Content-Length","0"],["Date","Fri, 15 Sep 2017 14:53:07 GMT"],["x-amz-id-2","Es2wWCc+tXkMbbp+bEcjLFQn6yGqPeAUiBI5XqXZ8mAZUIpe7vYiCfBfoU727trNpEFMymyhgZY="],["Server","AmazonS3"],["x-amz-request-id","98298F12367C7A0B"]],"body":[]}},{"request":{"uri":"http://alexcrichton-test.s3.amazonaws.com/crates/docscrate/docscrate-0.2.2.crate","method":"PUT","headers":[["Date","Fri, 15 Sep 2017 07:53:06 -0700"],["Authorization","AWS AKIAICL5IWUZYWWKA7JA:umuWs3XoGqsDipgMq04QAhq9Spc="],["Content-Type","application/x-tar"],["Host","alexcrichton-test.s3.amazonaws.com"],["Accept","*/*"],["Proxy-Connection","Keep-Alive"],["Content-Length","35"]],"body":[31,139,8,0,0,0,0,0,0,255,237,192,1,1,0,0,0,130,32,255,175,110,72,80,192,171,1,46,175,181,239,0,4,0,0]},"response":{"status":200,"headers":[["x-amz-request-id","2E4EDB9109077234"],["x-amz-id-2","gnBA5fngpIAF7AVe+goe2YZkL6gB7yRp25LNjBl7wqyMNeIXGMLTyWd/46bkoVNv/S9t9BZ5IB4="],["ETag","\"f9016ad360cebb4fe2e6e96e5949f022\""],["Date","Fri, 15 Sep 2017 14:53:07 GMT"],["Server","AmazonS3"],["Content-Length","0"]],"body":[]}}]
60 changes: 60 additions & 0 deletions src/tests/krate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1573,6 +1573,66 @@ fn publish_after_yank_max_version() {
assert_eq!(json.krate.max_version, "2.0.0");
}

#[test]
fn publish_after_removing_documentation() {
let (_b, app, middle) = ::app();

let user;

// 1. Start with a crate with no documentation
{
let conn = app.diesel_database.get().unwrap();
user = ::new_user("foo").create_or_update(&conn).unwrap();
::CrateBuilder::new("docscrate", user.id)
.version("0.2.0")
.expect_build(&conn);
}

// Verify that crates start without any documentation so the next assertion can *prove*
// that it was the one that added the documentation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, this block (L1568-1575) tests the same thing that the previous test does in L1464-1471. I can't think of a scenario where one of them would pass and the other would fail. Could you remove this block please?

{
let mut req = ::req(Arc::clone(&app), Method::Get, "/api/v1/crates/docscrate");
let mut response = ok_resp!(middle.call(&mut req));
let json: CrateResponse = ::json(&mut response);
assert_eq!(json.krate.documentation, None);
}

// 2. Add documentation
{
let mut req =
::new_req_with_documentation(Arc::clone(&app), "docscrate", "0.2.1", "http://foo.rs");
::sign_in_as(&mut req, &user);
let mut response = ok_resp!(middle.call(&mut req));
let json: GoodCrate = ::json(&mut response);
assert_eq!(json.krate.documentation, Some("http://foo.rs".to_owned()));
}

// Ensure latest version also has the same documentation
{
let mut req = ::req(Arc::clone(&app), Method::Get, "/api/v1/crates/docscrate");
let mut response = ok_resp!(middle.call(&mut req));
let json: CrateResponse = ::json(&mut response);
assert_eq!(json.krate.documentation, Some("http://foo.rs".to_owned()));
}

// 3. Remove the documentation
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And again, imo lines 1637-1691 are not providing any additional coverage. Please remove, thank you!

{
let mut req = ::new_req(Arc::clone(&app), "docscrate", "0.2.2");
::sign_in_as(&mut req, &user);
let mut response = ok_resp!(middle.call(&mut req));
let json: GoodCrate = ::json(&mut response);
assert_eq!(json.krate.documentation, None);
}

// Ensure latest version no longer has documentation
{
let mut req = ::req(Arc::clone(&app), Method::Get, "/api/v1/crates/docscrate");
let mut response = ok_resp!(middle.call(&mut req));
let json: CrateResponse = ::json(&mut response);
assert_eq!(json.krate.documentation, None);
}
}

#[test]
fn bad_keywords() {
let (_b, app, middle) = ::app();
Expand Down