Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
URI-escape dist name in sidebar links
and other things used in querystrings.

refs #1577.
  • Loading branch information
rwstauner committed Aug 19, 2015
1 parent 1336952 commit d1079ed
Show file tree
Hide file tree
Showing 5 changed files with 51 additions and 17 deletions.
7 changes: 7 additions & 0 deletions lib/MetaCPAN/Web/View/HTML.pm
Expand Up @@ -217,6 +217,13 @@ Template::Alloy->define_vmethod(
},
);

Template::Alloy->define_vmethod(
'text',
quotemeta => sub {
return quotemeta( $_[0] );
},
);

__PACKAGE__->meta->make_immutable( inline_constructor => 0 );

1;
2 changes: 1 addition & 1 deletion root/inc/perlybook.html
Expand Up @@ -15,6 +15,6 @@
-%>
<%- FOREACH t IN targets -%>
<%- UNLESS loop.first %> | <% END -%>
<a href="http://perlybook.org/?source=<% IF module %><% module.documentation or module.module.0.name %><% ELSE %><% release.distribution %><% END %>&amp;book_selection=<% IF module %><% ELSE %>1<% END %>&amp;target=<% t.name %>" title="<% t.title %>"><% t.name.upper %></a>
<a href="http://perlybook.org/?source=<% IF module %><% module.documentation or module.module.0.name | uri %><% ELSE %><% release.distribution | uri %><% END %>&amp;book_selection=<% IF module %><% ELSE %>1<% END %>&amp;target=<% t.name | uri %>" title="<% t.title %>"><% t.name.upper %></a>
<%- END -%>
</li>
6 changes: 3 additions & 3 deletions root/inc/release-info.html
Expand Up @@ -29,9 +29,9 @@
<% END %>
<li>
<%
testers_version = release.name.remove('^' _ release.distribution _ '-?');
cpantesters_base = "http://www.cpantesters.org/distro/" _ release.distribution.chunk(1).0.uc _ "/" _ release.distribution _ ".html?oncpan=1&distmat=1&version=" _ testers_version %>
<a href="http://matrix.cpantesters.org/?dist=<% release.distribution %>+<% testers_version %>" title="Matrix"><i class="fa fa-fw fa-check-circle black"></i>Testers</a>
testers_version = release.name.remove('^' _ release.distribution.quotemeta _ '-?');
cpantesters_base = "http://www.cpantesters.org/distro/" _ release.distribution.chunk(1).0.uc _ "/" _ release.distribution _ ".html?oncpan=1&distmat=1&version=" _ (testers_version | uri) %>
<a href="http://matrix.cpantesters.org/?dist=<% release.distribution | uri %>+<% testers_version | uri %>" title="Matrix"><i class="fa fa-fw fa-check-circle black"></i>Testers</a>
<% IF release.tests.size %><span title="(pass / fail / na)">(<a href="<% cpantesters_base %>&amp;grade=2" style="color: #090"><% release.tests.pass %></a> / <a href="<% cpantesters_base %>&amp;grade=3" style="color: #900"><% release.tests.fail %></a> / <a href="<% cpantesters_base %>&amp;grade=4"><% release.tests.na %></a>)</span><% END %>
</li>
<li>
Expand Down
6 changes: 3 additions & 3 deletions root/inc/release-tools.html
Expand Up @@ -6,12 +6,12 @@
</a>
</li>
<li>
<a href="https://explorer.metacpan.org/?url=/<% module ? ['module', module.author, module.release, module.path].join("/") : ['release', release.author, release.name].join("/") %>">
<a href="https://explorer.metacpan.org/?url=<% ("/" _ ( module ? ['module', module.author, module.release, module.path].join("/") : ['release', release.author, release.name].join("/") ) ) | uri %>">
<i class="fa fa-list-alt fa-fw black"></i>MetaCPAN Explorer
</a>
</li>
<li>
<a href="<% 'http://cpanratings.perl.org/rate/?distribution=' _ release.distribution %>">
<a href="<% 'http://cpanratings.perl.org/rate/?distribution=' _ release.distribution | uri %>">
<i class="fa fa-star fa-fw black"></i>Rate this distribution
</a>
</li>
Expand All @@ -35,7 +35,7 @@
</select>
</li>
<li>
<select name="release" onchange="document.location.href='/diff/file/?target=<% [release.author, release.name, module.path].join("/") %>&amp;source=' + this.value" class="form-control tool-bar-form">
<select name="release" onchange="document.location.href='/diff/file/?target=<% [release.author, release.name, module.path].join("/") | uri %>&amp;source=' + encodeURIComponent(this.value)" class="form-control tool-bar-form">
<option>Diff with version</option>
<%- PROCESS version_options %>
</select>
Expand Down
47 changes: 37 additions & 10 deletions t/controller/shared/release-info.t
Expand Up @@ -34,6 +34,16 @@ test_psgi app, sub {
# has all optional tests
{ module => 'Dist::Zilla' },

# dist name that needs uri encoding
{
module => 'Text::Tabs',
release => 'Text-Tabs+Wrap',
qs_dist => 'Text-Tabs%2BWrap',
home_page => 0,
repository => 0,
reviews => 0,
},

# release name different than just s/::/-/g
{
module => 'LWP::UserAgent',
Expand All @@ -55,6 +65,8 @@ test_psgi app, sub {
foreach my $test (@tests) {
( $test->{release} = $test->{module} ) =~ s/::/-/g
if !$test->{release};
my $qs_dist = $test->{qs_dist} || $test->{release};
my $qs_module = $test->{module};

# turn tests on by default
exists( $test->{$_} ) or $test->{$_} = 1 for @optional;
Expand All @@ -75,7 +87,7 @@ test_psgi app, sub {

# these first tests are similar between the controllers only because of
# consistecy or coincidence and are not specifically related to release-info
$tx->like( '/html/head/title', qr/$name/,
$tx->like( '/html/head/title', qr/\Q$name\E/,
qq[title includes name "$name"] );

ok( $tx->find_value(qq<//a[\@href="$req_uri"]>),
Expand All @@ -90,13 +102,14 @@ test_psgi app, sub {
# A fragile and unsure way to get the version, but at least an 80% solution.
# TODO: Set up a fake cpan; We'll know what version to expect; we can test that this matches
ok(
my $version
= (
$this =~ m!(?:/pod)?/release/[^/]+/$release-([^/"]+)! )
[0],
my $version = (
$this =~ m!(?:/pod)?/release/[^/]+/\Q$release\E-([^/"]+)!
)[0],
'got version from "this" link'
);

my $qs_version = $version;

# TODO: latest version (should be where we already are)
# TODO: author

Expand All @@ -117,6 +130,18 @@ test_psgi app, sub {
# TODO: Download
# TODO: Changes

foreach my $booktype (qw(mobi epub)) {
my ( $source, $sel )
= $type eq 'module'
? ( $qs_module, '' )
: ( $qs_dist, 1 );
$tx->is(
"//a[text()=\"\U$booktype\E\"]/\@href",
"http://perlybook.org/?source=$source&book_selection=$sel&target=$booktype",
"link for perlybook $booktype"
);
}

optional_test home_page => sub {
ok( $tx->find_value('//a[text()="Homepage"]/@href'),
'link for resources.homepage' );
Expand All @@ -138,7 +163,7 @@ test_psgi app, sub {
# not all dists have reviews
optional_test reviews => sub {
my $rating
= qq{//a[\@href="http://cpanratings.perl.org/rate/?distribution=$release"]};
= qq!//a[\@href="http://cpanratings.perl.org/rate/?distribution=$qs_dist"]!;
$tx->like( "$rating/span/\@class", qr/^rating-\d+$/,
'has link to rate',
);
Expand All @@ -156,16 +181,17 @@ test_psgi app, sub {
ok(
$tx->find_value(
'//a[@href="http://cpanratings.perl.org/rate/?distribution='
. $release . '"]'
. $qs_dist . '"]'
),
'cpanratings link to rate this dist'
);

# TODO: cpantesters
# TODO: release.tests.size

$tx->is(
'//a[@title="Matrix"]/@href',
"http://matrix.cpantesters.org/?dist=$release+$version",
"http://matrix.cpantesters.org/?dist=$qs_dist+$qs_version",
'link to test matrix'
);

Expand All @@ -191,11 +217,12 @@ test_psgi app, sub {
'reverse deps link uses dist name'
);

my $slash = '%2F';
$tx->like(
'//a[starts-with(@href, "https://explorer.metacpan.org/?url")]/@href',
$type eq 'module'
? qr!\?url=/module/\w+/${release}-${version}/.+!
: qr!\?url=/release/\w+/${release}-${version}\z!,
? qr!\?url=${slash}module${slash}\w+${slash}${qs_dist}-${version}${slash}.+!
: qr!\?url=${slash}release${slash}\w+${slash}${qs_dist}-${version}\z!,
'explorer link points to module file or release',
);

Expand Down

0 comments on commit d1079ed

Please sign in to comment.