Skip to content

Commit

Permalink
refactor and improve file sorting on release page
Browse files Browse the repository at this point in the history 
Move all file sorting into one routine.  Only link one one of a matching
pod/pm pair.  Allow for other interesting files that aren't in the root
directory.
  • Loading branch information
@haarg
haarg committed Jul 9, 2017
1 parent 8ccb288 commit 84f1880
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 99 deletions.
143 changes: 53 additions & 90 deletions lib/MetaCPAN/Web/Controller/Release.pm
View file
Expand Up @@ -5,8 +5,6 @@ use Moose;
use namespace::autoclean;

BEGIN { extends 'MetaCPAN::Web::Controller' }
use List::Util ();
use Ref::Util qw( is_arrayref );

with qw(
MetaCPAN::Web::Role::ReleaseInfo
Expand Down Expand Up @@ -83,22 +81,6 @@ sub view : Private {
$self->stash_api_results( $c, $reqs, $out );
$self->add_favorites_data( $out, $reqs->{favorites}, $out );

# shortcuts
my ( $files, $modules ) = @{$reqs}{qw(files modules)};

my @root_files = (
sort { $a->{name} cmp $b->{name} }
grep { $_->{path} !~ m{/} } @{ $files->{files} }
);

my @examples = (
sort { $a->{path} cmp $b->{path} }
grep {
$_->{path} =~ m{\b(?:eg|ex|examples?|samples?)\b}i
and not $_->{path} =~ m{^x?t/}
} @{ $files->{files} }
);

$c->res->last_modified( $out->{date} );
$c->cdn_max_age('1y');
$c->add_dist_key($distribution);
Expand All @@ -118,10 +100,8 @@ sub view : Private {
$c->stash(
$c->model('API::Favorite')->find_plussers($distribution)->get );

# Simplify the file data we pass to the template.
my $view_files = $modules->{files};

my $categories = $self->_files_to_categories( $out, $view_files );
my $categories = $self->_files_to_categories( map @{ $_->{files} },
@{$reqs}{qw(files modules)} );

my $changes
= $c->model('API::Changes')->last_version( $reqs->{changes}, $out );
Expand All @@ -130,18 +110,8 @@ sub view : Private {
$c->stash(
template => 'release.html',
release => $out,
total => $modules->{total},
took => List::Util::max(
$modules->{took}, $files->{took}, $reqs->{versions}{took}
),
root => \@root_files,
examples => \@examples,
files => $view_files,

documentation => $categories->{documentation},
documentation_raw => $categories->{documentation_raw},
provides => $categories->{provides},
modules => $categories->{modules},
%$categories,

# TODO: Put this in a more general place.
# Maybe make a hash for feature flags?
Expand All @@ -162,84 +132,77 @@ sub view : Private {
}

sub _files_to_categories {
my ( $self, $release, $files ) = @_;
my $self = shift;
my %files = map +( $_->{path} => $_ ), @_;

my $ret = +{
provides => [],
documentation => [],
documentation_raw => [],
modules => [],
provides => [],
documentation => [],
modules => [],
other => [],
examples => [],
};

my %skip;
for my $path ( sort keys %files ) {
my $f = $files{$path};
next
if $f->{skip};
my $path = $f->{path};
my @modules = @{ $f->{module} || [] };

for my $f (@$files) {
next if $f->{documentation};
my @modules
= is_arrayref( $f->{module} ) ? @{ $f->{module} } : $f->{module};
for my $module ( grep {defined} @modules ) {
my $assoc = $module->{associated_pod} or next;
for my $module (@modules) {
my $assoc = $module->{associated_pod}
or next;
$assoc =~ s{^\Q$f->{author}/$f->{release}/}{};
if ( $assoc ne $f->{path}
&& $assoc eq $f->{path} =~ s{\.pm$}{\.pod}r )
{
my ($assoc_file) = grep $_->{path} eq $assoc, @$files;
$f->{$_} ||= $assoc_file->{$_} for qw(
abstract
documentation
);
$skip{$assoc}++;
}
next
if $assoc eq $f->{path}
|| $assoc ne $f->{path} =~ s{\.pm$}{\.pod}r;

my $assoc_file = $files{$assoc}
or next;

$f->{$_} ||= $assoc_file->{$_} for qw(
abstract
documentation
);
$assoc_file->{skip}++;
}
}

for my $f ( @{$files} ) {
next
if $skip{ $f->{path} };
my %info = (
status => $f->{status},
path => $f->{path},
release => $f->{release},
author => $f->{author},
);

my @modules
= is_arrayref( $f->{module} ) ? @{ $f->{module} } : $f->{module};
if (@modules) {
my %s;
if ( $f->{documentation} ) {
push @{ $ret->{modules} }, $f;
$s{ $f->{documentation} }++;
}

if ( $f->{documentation} and @modules ) {
push @{ $ret->{modules} }, $f;
push @{ $ret->{provides} },
map +{
%info,
package => $_->{name},
authorized => $_->{authorized}
},
grep {
defined $_->{name}
and $_->{name} ne $f->{documentation}
and $_->{indexed}
and $_->{authorized}
} @modules;
grep !$s{ $_->{name} }++,
map +{ %$f, %$_, }, @modules;
}
elsif (@modules) {
push @{ $ret->{provides} },
map +{
%info,
package => $_->{name},
authorized => $_->{authorized}
}, @modules;
elsif ( $f->{documentation} && $path =~ m/\.pm$/ ) {
push @{ $ret->{modules} }, $f;
}
elsif ( $f->{documentation} ) {
push @{ $ret->{documentation} }, $f;
}
elsif ( $path =~ m{^(?:eg|ex|examples?|samples?)\b}i ) {
push @{ $ret->{examples} }, $f;
}
elsif ( $path =~ m/\.pod$/ ) {
push @{ $ret->{documentation} }, $f;
}
else {
push @{ $ret->{documentation_raw} }, $f;
push @{ $ret->{other} }, $f;
}
}

$ret->{provides}
= [ sort { $a->{name} cmp $b->{name} || $a->{path} cmp $b->{path} }
@{ $ret->{provides} } ];

return $ret;
}

__PACKAGE__->meta->make_immutable;

1;

9 changes: 4 additions & 5 deletions root/preprocess.html
View file
Expand Up @@ -85,13 +85,12 @@
END;

MACRO link_to_source(file, linktext) BLOCK;
# NOTE: The 'package' attribute is fabricated in the release template.
# NOTE: The 'name' attribute is fabricated in the release template.
path = ['source', file.author, file.release, file.path].join('/');
# Append "#P${package}" if we have a package name.
IF file.package;
path = path _ '#P' _ file.package;
IF file.name;
path = path _ '#P' _ file.name;
END
%><a href="/<% path %>"><% linktext || file.package || file.path %></a><%
%><a href="/<% path %>"><% linktext || file.name || file.path %></a><%
END;

# protocols = {
Expand Down
8 changes: 4 additions & 4 deletions root/release.html
View file
Expand Up @@ -94,7 +94,7 @@ <h2 id="modules">Modules</h2>
<div class="file-group release-provides">
<h2 id="provides">Provides</h2>
<ul>
<%- FOREACH module IN provides.sort('package') %>
<%- FOREACH module IN provides %>
<li>
<% link_to_source(module) | none %> in <% module.path %>
<% IF module.indexed AND NOT module.authorized %><em class="unauthorized">UNAUTHORIZED</em><% END %>
Expand All @@ -108,7 +108,7 @@ <h2 id="provides">Provides</h2>
<div class="file-group release-examples">
<h2 id="examples">Examples</h2>
<ul>
<%- FOREACH file IN examples.sort('path') %>
<%- FOREACH file IN examples %>
<li>
<%- IF file.path.match('\.pod$') %>
<% link_to_file(file, file.path) | none %>
Expand All @@ -124,11 +124,11 @@ <h2 id="examples">Examples</h2>
</div>
<%- END %>

<%- IF root.size %>
<%- IF other.size %>
<div class="file-group release-other-files">
<h2 id="other">Other files</h2>
<ul>
<%- FOREACH file IN root %>
<%- FOREACH file IN other %>
<li><% link_to_file(file, file.name) | none %></li>
<%- END %>
</ul>
Expand Down

0 comments on commit 84f1880

Please sign in to comment.