From ac107198b366fecd1e3df7cf8a58bcc468abea7e Mon Sep 17 00:00:00 2001 From: Graham Knop Date: Tue, 11 Jul 2017 14:14:55 +0200 Subject: [PATCH 1/8] add internal_error endpoint --- lib/MetaCPAN/Web/Controller/Root.pm | 8 ++++++++ root/internal_error.html | 13 +++++++++++++ 2 files changed, 21 insertions(+) create mode 100644 root/internal_error.html diff --git a/lib/MetaCPAN/Web/Controller/Root.pm b/lib/MetaCPAN/Web/Controller/Root.pm index bad022c4f3..aba9c3f20b 100644 --- a/lib/MetaCPAN/Web/Controller/Root.pm +++ b/lib/MetaCPAN/Web/Controller/Root.pm @@ -59,6 +59,14 @@ sub not_found : Private { $c->response->status(404); } +sub internal_error : Private { + my ( $self, $c ) = @_; + $c->cdn_never_cache(1); + + $c->stash( { template => 'internal_error.html' } ); + $c->response->status(500); +} + sub forbidden : Private { my ( $self, $c ) = @_; $c->cdn_never_cache(1); diff --git a/root/internal_error.html b/root/internal_error.html new file mode 100644 index 0000000000..3aeedf1d93 --- /dev/null +++ b/root/internal_error.html @@ -0,0 +1,13 @@ +<%- title = "Error 500 - Internal Error" %> +
+ <% INCLUDE left_sidebar.html %> +

Internal Error

+ +

+ <% IF message %> + <% message %> + <% ELSE %> + Internal Server Error + <% END %> +

+
From b8c55da6688d5203a5ac694b584ab132b26fb9a9 Mon Sep 17 00:00:00 2001 From: Graham Knop Date: Tue, 11 Jul 2017 14:16:16 +0200 Subject: [PATCH 2/8] handle Content-Encoding in API requests --- lib/MetaCPAN/Web/Model/API.pm | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/MetaCPAN/Web/Model/API.pm b/lib/MetaCPAN/Web/Model/API.pm index 37b72b8c09..b57a2e5b90 100644 --- a/lib/MetaCPAN/Web/Model/API.pm +++ b/lib/MetaCPAN/Web/Model/API.pm @@ -98,9 +98,9 @@ sub request { $self->client->do_request( request => $request )->transform( done => sub { - my $response = shift; + my $response = shift; + my $data = $response->decoded_content( charset => 'none' ); my $content_type = $response->header('content-type') || ''; - my $data = $response->content; if ( $content_type =~ /^application\/json/ ) { my $out; @@ -110,7 +110,7 @@ sub request { } # Response is raw data, e.g. text/plain - return $self->raw_api_response($data); + return $self->raw_api_response( $data, $response ); } ); } @@ -124,7 +124,7 @@ my $encode_check = ( Encode::FB_CROAK | Encode::LEAVE_SRC ); # Do raw files, git diffs, etc get converted? Any text that goes into ES? sub raw_api_response { - my ( $self, $data ) = @_; + my ( $self, $data, $response ) = @_; # we have to assume an encoding; doing nothing is like assuming latin1 # we'll probably have the least number of issues if we assume utf8 @@ -144,7 +144,7 @@ sub raw_api_response { warn $_[0]; }; - return +{ raw => $data }; + return +{ raw => $data, code => $response->code }; } __PACKAGE__->meta->make_immutable; From 5827234099b5fbaf410d0676a0df215e4770875e Mon Sep 17 00:00:00 2001 From: Graham Knop Date: Tue, 11 Jul 2017 14:16:46 +0200 Subject: [PATCH 3/8] properly strip W3C dates from note in changes parser --- lib/MetaCPAN/Web/Model/API/Changes/Parser.pm | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/MetaCPAN/Web/Model/API/Changes/Parser.pm b/lib/MetaCPAN/Web/Model/API/Changes/Parser.pm index 235f80e945..43850befbf 100644 --- a/lib/MetaCPAN/Web/Model/API/Changes/Parser.pm +++ b/lib/MetaCPAN/Web/Model/API/Changes/Parser.pm @@ -94,7 +94,7 @@ sub parse { } # start with W3CDTF, ignore rest - elsif ( $note =~ m{^($CPAN::Changes::W3CDTF_REGEX)} ) { + elsif ( $note =~ s{^($CPAN::Changes::W3CDTF_REGEX)}{} ) { $date = $1; $date =~ s{ }{T}; From b1b851556557256eccf1c4dece3612968c44a82b Mon Sep 17 00:00:00 2001 From: Graham Knop Date: Tue, 11 Jul 2017 14:17:17 +0200 Subject: [PATCH 4/8] remote extraneous

in not_found template --- root/not_found.html | 1 - 1 file changed, 1 deletion(-) diff --git a/root/not_found.html b/root/not_found.html index f991dae48e..06bdc3c436 100644 --- a/root/not_found.html +++ b/root/not_found.html @@ -26,5 +26,4 @@

Not Found

Search the CPAN for <% q %>

<% END %> -

From b66734b46e1bf34cc57db8f8d08bce658de1ba2a Mon Sep 17 00:00:00 2001 From: Graham Knop Date: Wed, 5 Jul 2017 16:23:21 +0200 Subject: [PATCH 5/8] move multiple author get into separate method --- .../Web/Controller/Recent/TopUploaders.pm | 3 ++- lib/MetaCPAN/Web/Model/API/Author.pm | 19 +++++++++++++++---- 2 files changed, 17 insertions(+), 5 deletions(-) diff --git a/lib/MetaCPAN/Web/Controller/Recent/TopUploaders.pm b/lib/MetaCPAN/Web/Controller/Recent/TopUploaders.pm index 62d1e1fb0d..b367d108f5 100644 --- a/lib/MetaCPAN/Web/Controller/Recent/TopUploaders.pm +++ b/lib/MetaCPAN/Web/Controller/Recent/TopUploaders.pm @@ -28,7 +28,8 @@ sub topuploaders : Private { my $data = $c->model('API::Release')->topuploaders($range)->get; my $authors - = $c->model('API::Author')->get( keys %{ $data->{counts} } )->get; + = $c->model('API::Author')->get_multiple( keys %{ $data->{counts} } ) + ->get; $c->stash( { diff --git a/lib/MetaCPAN/Web/Model/API/Author.pm b/lib/MetaCPAN/Web/Model/API/Author.pm index a0e71604bf..9150fccde0 100644 --- a/lib/MetaCPAN/Web/Model/API/Author.pm +++ b/lib/MetaCPAN/Web/Model/API/Author.pm @@ -27,12 +27,23 @@ it under the same terms as Perl itself. =cut sub get { - my ( $self, @author ) = @_; + my ( $self, $author ) = @_; - return $self->request( '/author/' . uc( $author[0] ) ) - if ( @author == 1 ); + return $self->request( '/author/' . uc($author) ); +} - return $self->request( '/author/by_ids', { id => \@author } ); +sub get_multiple { + my ( $self, @authors ) = @_; + return $self->request( '/author/by_ids', { id => [ map uc, @authors ] } ) + ->transform( + done => sub { + my $data = shift; + my %authors; + $authors{ $_->{pauseid} } = $_ for @{ $data->{authors} }; + $data->{authors} = [ @authors{@authors} ]; + $data; + } + ); } sub search { From 7b5da34fa054af35c3a46ff0a0f127bc7e82e1a7 Mon Sep 17 00:00:00 2001 From: Graham Knop Date: Fri, 7 Jul 2017 16:31:43 +0200 Subject: [PATCH 6/8] move release info collection into ReleaseInfo model This merges the ReleaseInfo role's logic into the ReleaseInfo model, and allows it to give all of the relevant release info at once. This removes the need to have goofy shared functions to massage the data, or insert it into the stash. It also allows the parallelism of the release info requests to be controlled easier. --- lib/MetaCPAN/Web/Controller/Pod.pm | 169 +++++++++++----------- lib/MetaCPAN/Web/Controller/Release.pm | 92 ++++-------- lib/MetaCPAN/Web/Model/ReleaseInfo.pm | 187 +++++++++++++++++++------ lib/MetaCPAN/Web/Role/ReleaseInfo.pm | 73 ---------- root/inc/breadcrumbs.html | 2 +- root/inc/favorite.html | 16 +-- root/release.html | 8 +- t/model/release-info.t | 22 +-- 8 files changed, 275 insertions(+), 294 deletions(-) delete mode 100644 lib/MetaCPAN/Web/Role/ReleaseInfo.pm diff --git a/lib/MetaCPAN/Web/Controller/Pod.pm b/lib/MetaCPAN/Web/Controller/Pod.pm index a46ae70bb2..4c7ef63f5a 100644 --- a/lib/MetaCPAN/Web/Controller/Pod.pm +++ b/lib/MetaCPAN/Web/Controller/Pod.pm @@ -10,10 +10,6 @@ use namespace::autoclean; BEGIN { extends 'MetaCPAN::Web::Controller' } -with qw( - MetaCPAN::Web::Role::ReleaseInfo -); - # /pod/$name sub find : Path : Args(1) { my ( $self, $c, @path ) = @_; @@ -21,7 +17,17 @@ sub find : Path : Args(1) { $c->browser_max_age('1h'); # TODO: Pass size param so we can disambiguate? - $c->stash->{pod_file} = $c->model('API::Module')->find(@path)->get; + my $pod_file = $c->stash->{pod_file} + = $c->model('API::Module')->find(@path)->get; + + $c->detach('/not_found') + if !$pod_file->{name}; + + my $release_info + = $c->model('ReleaseInfo') + ->get( $pod_file->{author}, $pod_file->{release} ) + ->else( sub { Future->done( {} ) } ); + $c->stash( $release_info->get ); # TODO: Disambiguate if there's more than once match. #176 @@ -41,8 +47,17 @@ sub release : Local : Args { $c->detach(); } - $c->stash->{pod_file} = $c->model('API::Module')->get(@path)->get; - $c->stash->{permalinks} = 1; + my $release_data = $c->model('ReleaseInfo')->get( @path[ 0, 1 ] ) + ->else( sub { Future->done( {} ) } ); + my $pod_file = $c->model('API::Module')->get(@path); + $c->stash( + { + pod_file => $pod_file->get, + %{ $release_data->get }, + permalinks => 1, + } + ); + $c->forward( 'view', [@path] ); } @@ -57,15 +72,18 @@ sub distribution : Local : Args { # Get latest "author/release" of dist so we can use it to find the file. # TODO: Pass size param so we can disambiguate? - my $release = try { - $c->model('API::Release')->find($dist)->get->{release}; + my $release_data = try { + $c->model('ReleaseInfo')->find($dist)->get; } or $c->detach('/not_found'); - # TODO: Disambiguate if there's more than once match. #176 + unshift @path, @{ $release_data->{release} }{qw( author name )}; - unshift @path, @$release{qw( author name )}; - - $c->stash->{pod_file} = $c->model('API::Module')->get(@path)->get; + $c->stash( + { + %$release_data, + pod_file => $c->model('API::Module')->get(@path)->get, + } + ); $c->forward( 'view', [@path] ); } @@ -81,7 +99,7 @@ sub view : Private { $c->detach; } - my ( $documentation, $pod, $documented_module ) + my ( $documentation, $assoc_pod, $documented_module ) = map { $_->{name}, $_->{associated_pod}, $_ } grep { @path > 1 || $path[0] eq $_->{name} } grep { @@ -89,35 +107,67 @@ sub view : Private { || $data->{documentation} eq $_->{name} } grep { $_->{associated_pod} } @{ $data->{module} || [] }; + $data->{documentation} = $documentation if $documentation; - if ( $pod && $pod ne "$data->{author}/$data->{release}/$data->{path}" ) { + + if ( $assoc_pod + && $assoc_pod ne "$data->{author}/$data->{release}/$data->{path}" ) + { $data->{pod_path} - = $pod =~ s{^\Q$data->{author}/$data->{release}/}{}r; + = $assoc_pod =~ s{^\Q$data->{author}/$data->{release}/}{}r; } $c->detach('/not_found') unless ( $data->{name} ); - my $pod_path = '/pod/' . ( $pod || join( q{/}, @path ) ); + my $pod_path = '/pod/' . ( $assoc_pod || join( q{/}, @path ) ); - my $reqs = $self->api_requests( - $c, + my $pod = $c->model('API')->request( + $pod_path, + undef, { - pod => $c->model('API')->request( - $pod_path, - undef, - { - show_errors => 1, - ( $permalinks ? ( permalinks => 1 ) : () ), - url_prefix => '/pod/', - } - ), - release => $c->model('API::Release') - ->get( @{$data}{qw(author release)} ), - }, - $data, + show_errors => 1, + ( $permalinks ? ( permalinks => 1 ) : () ), + url_prefix => '/pod/', + } )->get; - $self->stash_api_results( $c, $reqs, $data ); - $self->add_favorites_data( $data, $reqs->{favorites}, $data ); + + my $pod_html = $self->filter_html( $pod->{raw}, $data ); + + my $release = $c->stash->{release}; + + #<<< + my $canonical = ( $documented_module + && $documented_module->{authorized} + && $documented_module->{indexed} + ) ? "/pod/$documentation" + : join(q{/}, q{}, qw( pod distribution ), $release->{distribution}, + # Strip $author/$release from front of path. + @path[ 2 .. $#path ] + ); + #>>> + + # Store at fastly for a year - as we will purge! + $c->cdn_max_age('1y'); + $c->add_dist_key( $release->{distribution} ); + $c->add_author_key( $release->{author} ); + + $c->stash( + { + template => 'pod.html', + module => $data, + pod => $pod_html, + canonical => $canonical, + documented_module => $documented_module, + } + ); + + unless ( $pod->{raw} ) { + $c->stash( pod_error => $pod->{message}, ); + } +} + +sub filter_html { + my ( $self, $html, $data ) = @_; my $hr = HTML::Restrict->new( uri_schemes => @@ -188,7 +238,7 @@ sub view : Private { $base .= "$data->{author}/$data->{release}/"; } else { - $base .= $pod + $base .= $data->{associated_pod} || "$data->{author}/$data->{release}/$data->{path}"; } $val = URI->new_abs( $val, $base )->as_string; @@ -200,54 +250,7 @@ sub view : Private { return $tag; }, ); - - my $release = $reqs->{release}{release}; - - #<<< - my $canonical = ( $documented_module - && $documented_module->{authorized} - && $documented_module->{indexed} - ) ? "/pod/$documentation" - : join(q{/}, q{}, qw( pod distribution ), $release->{distribution}, - # Strip $author/$release from front of path. - @path[ 2 .. $#path ] - ); - #>>> - - my $dist = $release->{distribution}; - - # Store at fastly for a year - as we will purge! - $c->cdn_max_age('1y'); - $c->add_dist_key($dist); - $c->add_author_key( $release->{author} ); - - $c->stash( $c->model('API::Favorite')->find_plussers($dist)->get ); - - $c->stash( - $c->model( - 'ReleaseInfo', - { - author => $reqs->{author}, - distribution => $reqs->{distribution}, - release => $release, - } - )->summary_hash - ); - - $c->stash( - { - module => $data, - pod => $hr->process( $reqs->{pod}->{raw} ), - release => $release, - template => 'pod.html', - canonical => $canonical, - documented_module => $documented_module, - } - ); - - unless ( $reqs->{pod}->{raw} ) { - $c->stash( pod_error => $reqs->{pod}->{message}, ); - } + $hr->process($html); } __PACKAGE__->meta->make_immutable; diff --git a/lib/MetaCPAN/Web/Controller/Release.pm b/lib/MetaCPAN/Web/Controller/Release.pm index 9bed2140d4..0fc8a9a619 100644 --- a/lib/MetaCPAN/Web/Controller/Release.pm +++ b/lib/MetaCPAN/Web/Controller/Release.pm @@ -6,22 +6,17 @@ use namespace::autoclean; BEGIN { extends 'MetaCPAN::Web::Controller' } -with qw( - MetaCPAN::Web::Role::ReleaseInfo -); - sub root : Chained('/') PathPart('release') CaptureArgs(0) { my ( $self, $c ) = @_; - $c->stash->{model} = $c->model('API::Release'); + $c->stash->{current_model_instance} + = $c->model( 'ReleaseInfo', full_details => 1 ); } sub by_distribution : Chained('root') PathPart('') Args(1) { my ( $self, $c, $distribution ) = @_; - my $model = $c->stash->{model}; - $c->stash->{data} = $model->find($distribution); - $c->forward('view'); + $c->forward( 'view', [ $c->model->find($distribution) ] ); } sub index : Chained('/') PathPart('release') CaptureArgs(1) { @@ -37,8 +32,6 @@ sub plusser_display : Chained('index') PathPart('plussers') Args(0) { sub by_author_and_release : Chained('root') PathPart('') Args(2) { my ( $self, $c, $author, $release ) = @_; - my $model = $c->stash->{model}; - # force consistent casing in URLs if ( $author ne uc($author) ) { $c->res->redirect( @@ -52,66 +45,38 @@ sub by_author_and_release : Chained('root') PathPart('') Args(2) { } $c->stash->{permalinks} = 1; - $c->stash->{data} = $model->get( $author, $release ); - - $c->forward('view'); + $c->forward( 'view', [ $c->model->get( $author, $release ) ] ); } sub view : Private { - my ( $self, $c ) = @_; - - my $model = $c->stash->{model}; - my $data = delete $c->stash->{data}; - my $out = $data->get->{release}; - - $c->detach('/not_found') unless ($out); - - my ( $author, $release, $distribution ) - = ( $out->{author}, $out->{name}, $out->{distribution} ); - - my $reqs = $self->api_requests( - $c, - { - files => $model->interesting_files( $author, $release ), - modules => $model->modules( $author, $release ), - changes => $c->model('API::Changes')->get( $author, $release ), - }, - $out, + my ( $self, $c, $release_info ) = @_; + + my $data = $release_info->else( + sub { + my $error = shift; + return Future->fail($error) + if !ref $error; + $c->detach('/not_found') + if $error->{code} == 404; + $c->detach( '/internal_error', $error ); + } )->get; - $self->stash_api_results( $c, $reqs, $out ); - $self->add_favorites_data( $out, $reqs->{favorites}, $out ); - $c->res->last_modified( $out->{date} ); + my $release = $data->{release}; + + $c->res->last_modified( $release->{date} ); $c->cdn_max_age('1y'); - $c->add_dist_key($distribution); - $c->add_author_key($author); + $c->add_dist_key( $release->{distribution} ); + $c->add_author_key( $release->{author} ); - $c->stash( - $c->model( - 'ReleaseInfo', - { - author => $reqs->{author}, - distribution => $reqs->{distribution}, - release => $out - } - )->summary_hash - ); + my $categories = $self->_files_to_categories( map @$_, + $data->{files}, $data->{modules} ); $c->stash( - $c->model('API::Favorite')->find_plussers($distribution)->get ); - - my $categories = $self->_files_to_categories( map @{ $_->{files} }, - @{$reqs}{qw(files modules)} ); - - my $changes - = $c->model('API::Changes')->last_version( $reqs->{changes}, $out ); + %$data, + %$categories, - # TODO: make took more automatic (to include all) - $c->stash( template => 'release.html', - release => $out, - - %$categories, # TODO: Put this in a more general place. # Maybe make a hash for feature flags? @@ -119,15 +84,6 @@ sub view : Private { map { ( $_ => $c->config->{$_} ) } qw( mark_unauthorized_releases ) ), - - ( - @$changes - ? ( - last_version_changes => $changes->[0], - changelogs => $changes, - ) - : () - ) ); } diff --git a/lib/MetaCPAN/Web/Model/ReleaseInfo.pm b/lib/MetaCPAN/Web/Model/ReleaseInfo.pm index 3b93f0112f..4ab0eb1286 100644 --- a/lib/MetaCPAN/Web/Model/ReleaseInfo.pm +++ b/lib/MetaCPAN/Web/Model/ReleaseInfo.pm @@ -6,63 +6,167 @@ use MetaCPAN::Moose; extends 'Catalyst::Model'; -use List::Util qw( all ); -use MetaCPAN::Web::Types qw( HashRef Object ); +use List::Util qw( all max ); +use Ref::Util qw( is_hashref ); use URI; use URI::Escape qw(uri_escape uri_unescape); use URI::QueryParam; # Add methods to URI. +my %models = ( + _release => 'API::Release', + _author => 'API::Author', + _contributors => 'API::Contributors', + _changes => 'API::Changes', + _rating => 'API::Rating', + _favorite => 'API::Favorite', +); + +has [ keys %models ] => ( is => 'ro' ); +has full_details => ( is => 'ro' ); + sub ACCEPT_CONTEXT { - my ( $class, $c, $args ) = @_; - return $class->new( - { - c => $c, - %$args, - } - ); + my ( $class, $c, @args ) = @_; + @args = %{ $args[0] } + if @args == 1 and is_hashref( $args[0] ); + push @args, map +( $_ => $c->model( $models{$_} ) ), keys %models; + return $class->new(@args); } -# Setting these attributes to required will cause the app to exit when it tries -# to instantiate the model on startup. +sub find { + my ( $self, $dist ) = @_; + my $release = $self->_release->find($dist); + my %dist_data = $self->_dist_data($dist); + $release->then( + sub { + my $data = shift; + if ( !$data->{release} ) { + $_->cancel for values %dist_data; + return Future->fail( + { code => 404, message => 'Not found' } ); + } + $self->_wrap( + release => $data, + %dist_data, + $self->_release_data( + $data->{release}{author}, + $data->{release}{name} + ), + ); + } + )->then( $self->normalize ); +} -has author => ( - is => 'ro', - isa => HashRef, - required => 0, -); +sub get { + my ( $self, $author, $release_name ) = @_; + my $release = $self->_release->get( $author, $release_name ); + my %release_data = $self->_release_data( $author, $release_name ); + $release->then( + sub { + my $data = shift; + if ( !$data->{release} ) { + $_->cancel for values %release_data; + return Future->fail( + { code => 404, message => 'Not found' } ); + } + $self->_wrap( + release => $data, + %release_data, + $self->_dist_data( $data->{release}{distribution} ), + ); + } + )->then( $self->normalize ); +} -has c => ( - is => 'ro', - isa => Object, - required => 0, - documentation => 'Catlyst context object', -); +sub _wrap { + my ( $self, %data ) = @_; + my @keys = keys %data; + my @values = values %data; + Future->needs_all( map { Future->wrap($_)->else_done( {} ) } @values ) + ->transform( + done => sub { + my %out; + @out{@keys} = @_; + \%out; + } + ); +} -has distribution => ( - is => 'ro', - isa => HashRef, - required => 0, -); +sub _dist_data { + my ( $self, $dist ) = @_; + return ( + favorites => $self->_favorite->get( undef, $dist ), + plussers => $self->_favorite->find_plussers($dist), + rating => $self->_rating->get($dist), + versions => $self->_release->versions($dist), + distribution => $self->_release->distribution($dist), + ); +} -has release => ( - is => 'ro', - isa => HashRef, - required => 0, -); +sub _release_data { + my ( $self, $author, $release ) = @_; + return ( + author => $self->_author->get($author), + contributors => $self->_contributors->get( $author, $release ), + ( + $self->full_details + ? ( + files => + $self->_release->interesting_files( $author, $release ), + modules => $self->_release->modules( $author, $release ), + changes => $self->_changes->get( $author, $release ), + ) + : () + ), + ); +} -sub summary_hash { - my ($self) = @_; - return { - author => $self->author, - irc => $self->groom_irc, - issues => $self->normalize_issues, +sub normalize { + my $self = shift; + sub { + my $data = shift; + my $dist = $data->{release}{release}{distribution}; + Future->done( + { + took => max( + grep defined, + map $_->{took}, + grep is_hashref($_), + values %$data + ), + release => $data->{release}{release}, + favorites => $data->{favorites}{favorites}{$dist}, + rating => $data->{rating}{distributions}{$dist}, + versions => $data->{versions}{releases}, + distribution => $data->{distribution}, + author => $data->{author}, + contributors => $data->{contributors}, + irc => $self->groom_irc( $data->{release}{release} ), + issues => $self->normalize_issues( + $data->{release}{release}, + $data->{distribution} + ), + %{ $data->{plussers} }, + ( + $self->full_details + ? ( + files => $data->{files}{files}, + modules => $data->{modules}{files}, + changes => $self->_changes->last_version( + $data->{changes}, $data->{release}{release} + ), + ) + : () + ), + } + ); }; } sub groom_irc { - my ($self) = @_; + my ( $self, $release ) = @_; - my $irc = $self->release->{metadata}{resources}{x_IRC}; + my $irc = $release->{metadata}{resources}{x_IRC} + or return {}; my $irc_info = ref $irc ? {%$irc} : { url => $irc }; if ( !$irc_info->{web} && $irc_info->{url} ) { @@ -118,8 +222,7 @@ sub rt_url_prefix { } sub normalize_issues { - my ($self) = @_; - my ( $release, $distribution ) = ( $self->release, $self->distribution ); + my ( $self, $release, $distribution ) = @_; my $issues = {}; diff --git a/lib/MetaCPAN/Web/Role/ReleaseInfo.pm b/lib/MetaCPAN/Web/Role/ReleaseInfo.pm deleted file mode 100644 index 3b0359151a..0000000000 --- a/lib/MetaCPAN/Web/Role/ReleaseInfo.pm +++ /dev/null @@ -1,73 +0,0 @@ -package MetaCPAN::Web::Role::ReleaseInfo; - -use Moose::Role; -use Future; - -# TODO: are there other controllers that do (or should) include this? - -# TODO: should some of this be in a separate (instantiable) model -# so you don't have to keep passing $data? -# then wouldn't have to pass favorites back in. -# Role/API/Aggregator?, Model/APIAggregator/ReleaseInfo? - -# add favorites and myfavorite data into $main hash -sub add_favorites_data { - my ( $self, $main, $favorites, $data ) = @_; - $main->{myfavorite} - = $favorites->{myfavorites}->{ $data->{distribution} }; - $main->{favorites} = $favorites->{favorites}->{ $data->{distribution} }; - return; -} - -# TODO: should the api_requests be in the base controller role, -# and then the default extras be defined in other roles? - -# pass in any api request condvars and combine them with these defaults -sub api_requests { - my ( $self, $c, $reqs, $data ) = @_; - - my %reqs = ( - author => $c->model('API::Author')->get( $data->{author} ), - - favorites => $c->model('API::Favorite')->get( - $c->user_exists ? $c->user->id : undef, - $data->{distribution} - ), - - contributors => $c->model('API::Contributors') - ->get( $data->{author}, $data->{release} ), - - rating => $c->model('API::Rating')->get( $data->{distribution} ), - - versions => - $c->model('API::Release')->versions( $data->{distribution} ), - distribution => - $c->model('API::Release')->distribution( $data->{distribution} ), - %$reqs, - ); - my @names = keys %reqs; - my @futures = values %reqs; - return Future->needs_all(@futures)->transform( - done => sub { - my %results; - @results{@names} = @_; - return \%results; - } - ); -} - -# organize the api results into simple variables for the template -sub stash_api_results { - my ( $self, $c, $reqs, $data ) = @_; - $c->stash( - { - author => $reqs->{author}, - distribution => $reqs->{distribution}, - rating => $reqs->{rating}{distributions}{ $data->{distribution} }, - versions => $reqs->{versions}{releases}, - contributors => $reqs->{contributors}, - } - ); -} - -1; diff --git a/root/inc/breadcrumbs.html b/root/inc/breadcrumbs.html index 871d81e2d7..baa145ed4b 100644 --- a/root/inc/breadcrumbs.html +++ b/root/inc/breadcrumbs.html @@ -48,7 +48,7 @@ IF have_released; %> <%- END; END; %> -
<%- INCLUDE inc/favorite.html module = module || release %>
+
<%- INCLUDE inc/favorite.html %>
<%- IF module %>  / itemprop="name"<% END %> ><% module.documentation or module.module.0.name %> <%- END %> diff --git a/root/inc/favorite.html b/root/inc/favorite.html index c48c3a8f4e..fcdf93bf48 100644 --- a/root/inc/favorite.html +++ b/root/inc/favorite.html @@ -1,17 +1,17 @@ -
+
- - - - + + + +
- -<% module.favorites || release.favorites %> ++ + +<% favorites %> ++
diff --git a/root/release.html b/root/release.html index 8922af6598..6ff5cd5c30 100644 --- a/root/release.html +++ b/root/release.html @@ -47,14 +47,14 @@ <% END %> -<%- IF changelogs %> +<%- IF changes %>