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

Support GitHub Actions Badges #1838

Closed
wants to merge 12 commits into from
4 changes: 4 additions & 0 deletions app/components/badge-github-actions.hbs
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
<a href="{{ this.url }}">
<img src="{{ this.imageUrl }}" alt="{{ this.text }}" title="{{ this.text }}">

</a>
56 changes: 56 additions & 0 deletions app/components/badge-github-actions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import Component from '@ember/component';
import { computed } from '@ember/object';
import { alias } from '@ember/object/computed';

export default Component.extend({
tagName: 'span',
repository: alias('badge.attributes.repository'),
imageUrl: computed('badge.attributes.{repository,workflow_enc,branch,event}', function() {
const url = new URL(`https://github.com/${this.repository}/workflows/${this.workflow_enc}/badge.svg`);

if (this.branch !== '') {
url.searchParams.set('branch', this.branch);
}

if (this.event !== '') {
url.searchParams.set('event', this.event);
}

return url.href;
}),
url: computed('badge.attributes.{repository,workflow_enc,branch,event}', function() {
const url = new URL(`https://github.com/${this.repository}/actions`);

if (this.workflow_enc !== '') {
url.searchParams.set('workflow', this.workflow_enc);

Choose a reason for hiding this comment

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

It looks like this’ll double-encode the workflow.

Suggested change
url.searchParams.set('workflow', this.workflow_enc);
url.searchParams.set('workflow', this.workflow);

(and the condition too for neatness even though it behaves the same)

Copy link
Member

@flip1995 flip1995 Jan 28, 2020

Choose a reason for hiding this comment

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

According to this doc url.searchParams is readOnly. Are you sure, that it can be used like this? (I'm a JS noob, so maybe my understanding is completely wrong)

Choose a reason for hiding this comment

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

The property is read-only:

> new URL('http://example.com/').searchParams = new URLSearchParams()
Uncaught:
TypeError: Cannot set property searchParams of [object URL] which has only a getter

but its value, the URLSearchParams, isn’t.

Copy link
Member

Choose a reason for hiding this comment

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

I just tested this and the format produced by the searchParams isn't accepted by GitHub. We need this format: query=workflow%3A"Say+hello%2Fwhatever"+branch%3Abadges-test+event%3Apush

}

if (this.branch !== '') {
url.searchParams.set('branch', this.branch);
}

if (this.event !== '') {
url.searchParams.set('event', this.event);
}

return url.href;
}),
workflow: computed('badge.attributes.workflow', function() {
return this.get('badge.attributes.workflow');
}),
workflow_enc: computed('badge.attributes.workflow', function() {
return this.get('badge.attributes.workflow')
.split('/')
.map(encodeURIComponent)
.join('/');
}),
branch: computed('badge.attributes.branch', function() {
return encodeURIComponent(this.get('badge.attributes.branch') || 'master');

Choose a reason for hiding this comment

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

Suggested change
return encodeURIComponent(this.get('badge.attributes.branch') || 'master');
return this.get('badge.attributes.branch') || 'master';

Copy link
Member

Choose a reason for hiding this comment

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

I think we want to encode branch names, for branches with / in them.

Copy link
Member

Choose a reason for hiding this comment

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

Ah wait, / works in queries. And we also don't want to display the branch name encoded in the (alt-)text.

Choose a reason for hiding this comment

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

Branch names do need to be encoded, but URLSearchParams takes care of that. (It’s the name that needs the explicit encodeURIComponent because it’s a path component instead of a part of the params.)

}),
event: computed('badge.attributes.event', function() {
return encodeURIComponent(this.get('badge.attributes.event') || '');

Choose a reason for hiding this comment

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

Suggested change
return encodeURIComponent(this.get('badge.attributes.event') || '');
return this.get('badge.attributes.event') || '';

}),
text: computed('badge', function() {
return `GitHub Actions workflow status for the ${this.workflow} workflow on the ${this.branch} branch`;
}),
});
7 changes: 7 additions & 0 deletions src/models/badge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ pub enum Badge {
pipeline: String,
build: Option<String>,
},
#[serde(rename = "github-actions")]
GitHubActions {
repository: String,
workflow: String,
branch: Option<String>,
event: Option<String>,
},
#[serde(rename = "gitlab")]
GitLab {
repository: String,
Expand Down
71 changes: 71 additions & 0 deletions src/tests/badge.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ struct BadgeRef {
appveyor_attributes: HashMap<String, String>,
travis_ci: Badge,
travis_ci_attributes: HashMap<String, String>,
github_actions: Badge,
github_actions_attributes: HashMap<String, String>,
gitlab: Badge,
gitlab_attributes: HashMap<String, String>,
azure_devops: Badge,
Expand Down Expand Up @@ -78,6 +80,19 @@ fn set_up() -> (BadgeTestCrate, BadgeRef) {
badge_attributes_travis_ci.insert(String::from("branch"), String::from("beta"));
badge_attributes_travis_ci.insert(String::from("repository"), String::from("rust-lang/rust"));

let github_actions = Badge::GitHubActions {
repository: String::from("rust-lang/rust"),
workflow: String::from("build"),
branch: Some(String::from("beta")),
event: Some(String::from("push")),
};
let mut badge_attributes_github_actions = HashMap::new();
badge_attributes_github_actions
.insert(String::from("repository"), String::from("rust-lang/rust"));
badge_attributes_github_actions.insert(String::from("workflow"), String::from("build"));
badge_attributes_github_actions.insert(String::from("branch"), String::from("beta"));
badge_attributes_github_actions.insert(String::from("event"), String::from("push"));

let gitlab = Badge::GitLab {
branch: Some(String::from("beta")),
repository: String::from("rust-lang/rust"),
Expand Down Expand Up @@ -169,6 +184,8 @@ fn set_up() -> (BadgeTestCrate, BadgeRef) {
appveyor_attributes: badge_attributes_appveyor,
travis_ci,
travis_ci_attributes: badge_attributes_travis_ci,
github_actions,
github_actions_attributes: badge_attributes_github_actions,
gitlab,
gitlab_attributes: badge_attributes_gitlab,
azure_devops,
Expand Down Expand Up @@ -226,6 +243,20 @@ fn update_add_travis_ci() {
assert_eq!(krate.badges(), vec![test_badges.travis_ci]);
}

#[test]
fn update_add_github_actions() {
// Add a github actions badge
let (krate, test_badges) = set_up();

let mut badges = HashMap::new();
badges.insert(
String::from("github-actions"),
test_badges.github_actions_attributes,
);
krate.update(&badges);
assert_eq!(krate.badges(), vec![test_badges.github_actions]);
}

#[test]
fn update_add_gitlab() {
// Add a gitlab badge
Expand Down Expand Up @@ -459,6 +490,46 @@ fn travis_ci_required_keys() {
assert_eq!(krate.badges(), vec![]);
}

#[test]
fn github_actions_required_key_repository() {
// Add a GitHub Actions badge missing the required repository field
let (krate, mut test_badges) = set_up();

let mut badges = HashMap::new();

// Repository is a required key
test_badges.github_actions_attributes.remove("repository");
badges.insert(
String::from("github-actions"),
test_badges.github_actions_attributes,
);

let invalid_badges = krate.update(&badges);
assert_eq!(invalid_badges.len(), 1);
assert_eq!(invalid_badges.first().unwrap(), "github-actions");
assert_eq!(krate.badges(), vec![]);
}

#[test]
fn github_actions_required_key_workflow() {
// Add a GitHub Actions badge missing the required workflow field
let (krate, mut test_badges) = set_up();

let mut badges = HashMap::new();

// Workflow is a required key
test_badges.github_actions_attributes.remove("workflow");
badges.insert(
String::from("github-actions"),
test_badges.github_actions_attributes,
);

let invalid_badges = krate.update(&badges);
assert_eq!(invalid_badges.len(), 1);
assert_eq!(invalid_badges.first().unwrap(), "github-actions");
assert_eq!(krate.badges(), vec![]);
}

#[test]
fn gitlab_required_keys() {
// Add a gitlab badge missing a required field
Expand Down