Skip to content

Fixes #962 Backend: add "most_recently_downloaded" to 'summary' endpoint #1101

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 4 commits into from
Oct 9, 2017
Merged

Conversation

erewok
Copy link
Contributor

@erewok erewok commented Oct 3, 2017

Description

  • Adds "most_recently_downloaded" property to JSON response returned from api/v1/summary

  • Adds tests for api/v1/summary endpoint

src/krate/mod.rs Outdated
@@ -756,6 +756,10 @@ pub fn index(req: &mut Request) -> CargoResult<Response> {
/// Handles the `GET /summary` route.
pub fn summary(req: &mut Request) -> CargoResult<Response> {
use schema::crates::dsl::*;
use diesel::expression::DayAndMonthIntervalDsl;
use diesel::types::{BigInt, Nullable};
use diesel::expression::functions::date_and_time::{date, now};
Copy link
Contributor

Choose a reason for hiding this comment

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

use diesel::expression::{date, now};

src/krate/mod.rs Outdated
use diesel::expression::DayAndMonthIntervalDsl;
use diesel::types::{BigInt, Nullable};
use diesel::expression::functions::date_and_time::{date, now};
use diesel::expression::sql_literal::sql;
Copy link
Contributor

Choose a reason for hiding this comment

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

use diesel::expression::sql;

src/krate/mod.rs Outdated
),
)
.group_by(id)
.order(recent_downloads.clone().desc().nulls_last())
Copy link
Contributor

Choose a reason for hiding this comment

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

Why are we cloning this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, good point. Bad copy pasta.

@@ -71,6 +82,7 @@ fn new_crate(name: &str) -> u::NewCrate {
}
}


Copy link
Contributor

Choose a reason for hiding this comment

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

✂️

let json: SummaryResponse = ::json(&mut response);

assert_eq!(json.num_crates, 4);
assert_eq!(json.num_downloads, 0); // need to add a record to metadata
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like you need to fix this?

);
assert_eq!(json.popular_keywords[0].keyword, "popular");
assert_eq!(json.popular_categories[0].category, "Category 1");
assert_eq!(json.just_updated.len(), 0); // update a couple before running this request...
Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds like you need to fix this?

use self::diesel::prelude::*;
use serde_json;
use semver;
use chrono::Utc;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you sort these

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure.

src/krate/mod.rs Outdated
use diesel::expression::DayAndMonthIntervalDsl;
use diesel::types::{BigInt, Nullable};
use diesel::expression::{date, now};
use diesel::expression::sql;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you sort these and squash all the expression imports into one

- add tests for total_downloads and just updated
- sort imports
@sgrif
Copy link
Contributor

sgrif commented Oct 9, 2017

bors: r+

bors-voyager bot added a commit that referenced this pull request Oct 9, 2017
1101: Fixes #962 Backend: add "most_recently_downloaded" to 'summary' endpoint r=sgrif

## Description

- Adds "most_recently_downloaded" property to JSON response returned from `api/v1/summary`

- Adds tests for `api/v1/summary` endpoint
@bors-voyager
Copy link
Contributor

bors-voyager bot commented Oct 9, 2017

Build succeeded

@bors-voyager bors-voyager bot merged commit 1f0c928 into rust-lang:master Oct 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants