Skip to content
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

frontend: Show feature flags in topbar and separate page #1144

Merged
merged 4 commits into from
Nov 13, 2020
Merged
Show file tree
Hide file tree
Changes from 2 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
6 changes: 5 additions & 1 deletion src/db/types.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use postgres_types::{FromSql, ToSql};
use serde::Serialize;

#[derive(Debug, Clone, PartialEq, Serialize, FromSql, ToSql)]
#[derive(Debug, Clone, PartialEq, Eq, Serialize, FromSql, ToSql)]
#[postgres(name = "feature")]
pub struct Feature {
name: String,
Expand All @@ -12,4 +12,8 @@ impl Feature {
pub fn new(name: String, subfeatures: Vec<String>) -> Self {
Feature { name, subfeatures }
}

pub fn is_private(&self) -> bool {
self.name.starts_with('_')
}
}
5 changes: 5 additions & 0 deletions src/test/fakes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,11 @@ impl<'a> FakeRelease<'a> {
self
}

pub(crate) fn features(mut self, features: HashMap<String, Vec<String>>) -> Self {
self.package.features = features;
self
}

/// Returns the release_id
pub(crate) fn create(self) -> Result<i32, Error> {
use std::fs;
Expand Down
100 changes: 100 additions & 0 deletions src/web/crate_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ impl CrateDetails {
releases.license,
releases.documentation_url,
releases.default_target,
releases.features,
doc_coverage.total_items,
doc_coverage.documented_items,
doc_coverage.total_items_needing_examples,
Expand Down Expand Up @@ -148,6 +149,7 @@ impl CrateDetails {
default_target: krate.get("default_target"),
doc_targets: MetaData::parse_doc_targets(krate.get("doc_targets")),
yanked: krate.get("yanked"),
features: MetaData::parse_features(krate.get("features")),
};

let documented_items: Option<i32> = krate.get("documented_items");
Expand Down Expand Up @@ -325,6 +327,7 @@ mod tests {
use crate::test::{wrapper, TestDatabase};
use failure::Error;
use kuchiki::traits::TendrilSink;
use std::collections::HashMap;

fn assert_last_successful_build_equals(
db: &TestDatabase,
Expand Down Expand Up @@ -741,4 +744,101 @@ mod tests {
Ok(())
});
}

#[test]
fn feature_flags_report_empty() {
wrapper(|env| {
env.fake_release()
.name("library")
.version("0.1.0")
.features(HashMap::new())
.create()?;

let page = kuchiki::parse_html().one(
env.frontend()
.get("/crate/library/0.1.0/features")
.send()?
.text()?,
);
assert!(page.select_first(r#"p[data-id="empty-features"]"#).is_ok());
Ok(())
});
}

#[test]
fn feature_private_feature_flags_are_hidden() {
wrapper(|env| {
let features = [("_private".into(), Vec::new())]
.iter()
.cloned()
.collect::<HashMap<String, Vec<String>>>();
env.fake_release()
.name("library")
.version("0.1.0")
.features(features)
.create()?;

let page = kuchiki::parse_html().one(
env.frontend()
.get("/crate/library/0.1.0/features")
.send()?
.text()?,
);
assert!(page.select_first(r#"p[data-id="empty-features"]"#).is_ok());
Ok(())
});
}

#[test]
fn feature_flags_without_default() {
wrapper(|env| {
let features = [("feature1".into(), Vec::new())]
.iter()
.cloned()
.collect::<HashMap<String, Vec<String>>>();
env.fake_release()
.name("library")
.version("0.1.0")
.features(features)
.create()?;

let page = kuchiki::parse_html().one(
env.frontend()
.get("/crate/library/0.1.0/features")
.send()?
.text()?,
);
assert!(page.select_first(r#"p[data-id="empty-features"]"#).is_err());
let def_len = page
.select_first(r#"b[data-id="default-feature-len"]"#)
.unwrap();
assert_eq!(def_len.text_contents(), "0");
Ok(())
});
}

#[test]
fn feature_flags_report_null() {
wrapper(|env| {
let id = env
.fake_release()
.name("library")
.version("0.1.0")
.features(HashMap::new())
almusil marked this conversation as resolved.
Show resolved Hide resolved
.create()?;

env.db()
.conn()
.query("UPDATE releases SET features = NULL WHERE id = $1", &[&id])?;

let page = kuchiki::parse_html().one(
env.frontend()
.get("/crate/library/0.1.0/features")
.send()?
.text()?,
);
assert!(page.select_first(r#"p[data-id="null-features"]"#).is_ok());
Ok(())
});
}
}
30 changes: 30 additions & 0 deletions src/web/features.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
use crate::{
db::Pool,
impl_webpage,
web::{page::WebPage, MetaData},
};
use iron::{IronResult, Request, Response};
use router::Router;
use serde::Serialize;

#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
struct FeaturesPage {
metadata: MetaData,
}

impl_webpage! {
FeaturesPage = "crate/features.html",
}

pub fn build_features_handler(req: &mut Request) -> IronResult<Response> {
let router = extension!(req, Router);
let name = cexpect!(req, router.find("name"));
let version = cexpect!(req, router.find("version"));

let mut conn = extension!(req, Pool).get()?;

FeaturesPage {
metadata: cexpect!(req, MetaData::from_crate(&mut conn, &name, &version)),
}
.into_response(req)
}
19 changes: 18 additions & 1 deletion src/web/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ mod builds;
mod crate_details;
mod error;
mod extensions;
mod features;
mod file;
pub(crate) mod metrics;
mod releases;
Expand All @@ -90,6 +91,7 @@ mod sitemap;
mod source;
mod statics;

use crate::db::types::Feature;
use crate::{impl_webpage, Context};
use chrono::{DateTime, Utc};
use error::Nope;
Expand Down Expand Up @@ -520,6 +522,7 @@ pub(crate) struct MetaData {
pub(crate) default_target: String,
pub(crate) doc_targets: Vec<String>,
pub(crate) yanked: bool,
pub(crate) features: Option<Vec<Feature>>,
}

impl MetaData {
Expand All @@ -533,7 +536,8 @@ impl MetaData {
releases.rustdoc_status,
releases.default_target,
releases.doc_targets,
releases.yanked
releases.yanked,
releases.features
FROM releases
INNER JOIN crates ON crates.id = releases.crate_id
WHERE crates.name = $1 AND releases.version = $2",
Expand All @@ -552,6 +556,7 @@ impl MetaData {
default_target: row.get(5),
doc_targets: MetaData::parse_doc_targets(row.get(6)),
yanked: row.get(7),
features: MetaData::parse_features(row.get(8)),
})
}

Expand All @@ -566,6 +571,14 @@ impl MetaData {
})
.unwrap_or_else(Vec::new)
}

pub(crate) fn parse_features(features: Option<Vec<Feature>>) -> Option<Vec<Feature>> {
features.map(|vec| {
vec.into_iter()
.filter(|feature| !feature.is_private())
.collect()
})
}
}

#[derive(Debug, Clone, PartialEq, Serialize)]
Expand Down Expand Up @@ -844,6 +857,7 @@ mod test {
"arm64-unknown-linux-gnu".to_string(),
],
yanked: false,
features: None,
};

let correct_json = json!({
Expand All @@ -858,6 +872,7 @@ mod test {
"arm64-unknown-linux-gnu",
],
"yanked": false,
"features": null
});

assert_eq!(correct_json, serde_json::to_value(&metadata).unwrap());
Expand All @@ -875,6 +890,7 @@ mod test {
"arm64-unknown-linux-gnu",
],
"yanked": false,
"features": null,
});

assert_eq!(correct_json, serde_json::to_value(&metadata).unwrap());
Expand All @@ -892,6 +908,7 @@ mod test {
"arm64-unknown-linux-gnu",
],
"yanked": false,
"features": null,
});

assert_eq!(correct_json, serde_json::to_value(&metadata).unwrap());
Expand Down
4 changes: 4 additions & 0 deletions src/web/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,10 @@ pub(super) fn build_routes() -> Routes {
"/crate/:name/:version/builds/:id",
super::builds::build_list_handler,
);
routes.internal_page(
"/crate/:name/:version/features",
super::features::build_features_handler,
);
routes.internal_page(
"/crate/:name/:version/source",
SimpleRedirect::new(|url| url.set_path(&format!("{}/", url.path()))),
Expand Down
4 changes: 3 additions & 1 deletion src/web/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,8 @@ impl FileList {
releases.files,
releases.default_target,
releases.doc_targets,
releases.yanked
releases.yanked,
releases.features
FROM releases
LEFT OUTER JOIN crates ON crates.id = releases.crate_id
WHERE crates.name = $1 AND releases.version = $2",
Expand Down Expand Up @@ -137,6 +138,7 @@ impl FileList {
default_target: rows[0].get(6),
doc_targets: MetaData::parse_doc_targets(rows[0].get(7)),
yanked: rows[0].get(8),
features: MetaData::parse_features(rows[0].get(9)),
},
files: file_list,
})
Expand Down
82 changes: 82 additions & 0 deletions templates/crate/features.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,82 @@
{%- extends "base.html" -%}
{%- import "header/package_navigation.html" as navigation -%}

{%- block title -%}
{{ macros::doc_title(name=metadata.name, version=metadata.version) }}
{%- endblock title -%}

{%- block topbar -%}
{%- set latest_version = "" -%}
{%- set latest_path = "" -%}
{%- set target = "" -%}
{%- set inner_path = metadata.target_name ~ "/index.html" -%}
{%- set is_latest_version = true -%}
{%- set is_prerelease = false -%}
{%- include "rustdoc/topbar.html" -%}
{%- endblock topbar -%}

{%- block header -%}
{{ navigation::package_navigation(metadata=metadata, active_tab="features") }}
{%- endblock header -%}

{%- block body -%}
<div class="container package-page-container">
<div class="pure-g">
<div class="pure-u-1 pure-u-sm-7-24 pure-u-md-5-24">
<div class="pure-menu package-menu">
<ul class="pure-menu-list">
<li class="pure-menu-heading">Feature flags</li>
{%- if metadata.features -%}
{%- for feature in metadata.features -%}
<li class="pure-menu-item">
<a href="#{{ feature.name }}" class="pure-menu-link" style="text-align:center;">
{{ feature.name }}
</a>
</li>
{%- endfor -%}
{%- elif metadata.features is iterable -%}
<li class="pure-menu-item">
<span style="font-size: 13px;">This release does not have any feature flags.</span>
</li>
{%- else -%}
<li class="pure-menu-item">
<span style="font-size: 13px;">Feature flags data are not available for this release.</span>
</li>
{%- endif -%}
</ul>
</div>
</div>

<div class="pure-u-1 pure-u-sm-17-24 pure-u-md-19-24 package-details" id="main">
<h1>{{ metadata.name }}</h1>
{%- if metadata.features -%}
<p>This version has <b>{{ metadata.features | length }}</b> feature flags, <b data-id="default-feature-len">
{%- if metadata.features[0].name == 'default' -%}
{{ metadata.features[0].subfeatures | length }}
Copy link
Member

Choose a reason for hiding this comment

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

This count is wrong when one of the default features activates more features. In bs58 0.4.0 I have default = ["std"] and std = ["alloc"].

Also I think the previous count should be decreased by 1 (if the crate has a default feature), otherwise a crate which activates all its features by default will still say something like "This version has 5 feature flags, 4 of them enabled by default" because default is not counted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

async-compression 0.3.6

I was trying it locally and I can see both tokio-02 and tokio-03.

Copy link
Member

Choose a reason for hiding this comment

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

Since the renamed dependency issue is unrelated to this UI I opened #1175 for it.

{%- else -%}
0
{%- endif -%}
</b> of them enabled by <b>default</b>.</p>
{%- for feature in metadata.features -%}
<h3 id="{{ feature.name }}">{{ feature.name }}</h3>
<ul class="pure-menu-list">
{%- if feature.subfeatures -%}
{%- for subfeature in feature.subfeatures -%}
<li class="pure-menu-item">
<span>{{ subfeature }}</span>
</li>
{%- endfor -%}
{%- else -%}
<p>This feature flag does not enable additional features.</p>
{%- endif -%}
</ul>
{%- endfor -%}
{%- elif metadata.features is iterable -%}
<p data-id="empty-features">This release does not have any feature flags.</p>
{%- else -%}
<p data-id="null-features">Feature flags data are not available for this release.</p>
{%- endif -%}
</div>
</div>
</div>
{%- endblock body -%}
Loading