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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 4 commits into from
Nov 13, 2020
Merged

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

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2df55ef
frontend: Show feature flags
almusil Oct 27, 2020
902515f
frontend: Distinguish between empty and null features
almusil Nov 10, 2020
1937862
frontend: Order default features in tree structure
almusil Nov 11, 2020
6730aa2
frontend: Make features code more modular
almusil Nov 12, 2020
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
View file
Open in desktop
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('_')
}
}
9 changes: 9 additions & 0 deletions src/test/fakes.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,15 @@ impl<'a> FakeRelease<'a> {
self
}

pub(crate) fn features(mut self, opt_features: Option<HashMap<String, Vec<String>>>) -> Self {
if let Some(features) = opt_features {
self.package.features = features;
} else {
self.package.features = HashMap::new();
}
almusil marked this conversation as resolved.
Show resolved Hide resolved
self
}

/// Returns the release_id
pub(crate) fn create(self) -> Result<i32, Error> {
use std::fs;
Expand Down
78 changes: 78 additions & 0 deletions src/web/crate_details.rs
View file
Open in desktop
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,79 @@ mod tests {
Ok(())
});
}

#[test]
fn feature_flags_report_empty() {
wrapper(|env| {
env.fake_release()
.name("library")
.version("0.1.0")
.binary(false)
almusil marked this conversation as resolved.
Show resolved Hide resolved
.features(None)
.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")
.binary(false)
.features(Some(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")
.binary(false)
.features(Some(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(())
});
}
}
30 changes: 30 additions & 0 deletions src/web/features.rs
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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
76 changes: 76 additions & 0 deletions templates/crate/features.html
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
{%- 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 -%}
{%- else -%}
<li class="pure-menu-item">
<span style="font-size: 13px;">Feature flags are not available for this version.</span>
almusil marked this conversation as resolved.
Show resolved Hide resolved
</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 being enabled by <b>default</b>.</p>
almusil marked this conversation as resolved.
Show resolved Hide resolved
{%- 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 -%}
{%- else -%}
<p data-id="empty-features">Feature flags are not available for this release.</p>
{%- endif -%}
</div>
</div>
</div>
{%- endblock body -%}
10 changes: 10 additions & 0 deletions templates/header/package_navigation.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
* `crate`
* `source`
* `builds`
* `features`

Note: `false` here is acting as a pseudo-null value since you can't directly construct null values
and tera requires all parameters without defaults to be filled
Expand Down Expand Up @@ -85,6 +86,15 @@ <h1 id="crate-title">
<span class="title"> Builds</span>
</a>
</li>

{# The features tab #}
<li class="pure-menu-item">
<a href="/crate/{{ crate_path | safe }}/features"
almusil marked this conversation as resolved.
Show resolved Hide resolved
class="pure-menu-link{% if active_tab == 'features' %} pure-menu-active{% endif %}">
{{ "flag" | fas }}
<span class="title">Feature flags</span>
</a>
</li>
</ul>
</div>
</div>
Expand Down
Loading