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

Implement blame view for files #16873

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions src/api/app/assets/stylesheets/webui/application.scss
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
@import 'build-log';
@import 'content-selector-filters';
@import 'mixin';
@import 'blame';

html {
overflow-y: scroll !important;
Expand Down
45 changes: 45 additions & 0 deletions src/api/app/assets/stylesheets/webui/blame.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
.blame {
display: grid;
grid-template-columns: [time] max-content [user] 20px [commit] max-content [button] min-content [line] min-content [content] auto;
column-gap: 1rem;
.revision {
display: grid;
grid-template-columns: subgrid;
grid-column: time / span 6;
border-bottom: var(--bs-border-width) solid var(--bs-border-color);
.info {
display: grid;
grid-template-columns: subgrid;
grid-column: time / span 4;
.time {
grid-column-start: time;
}
.user {
grid-column-start: user;
}
.commit {
grid-column-start: commit;
max-width: 200px;
}
.button {
grid-column-start: button;
}
}
.code {
display: grid;
grid-template-columns: subgrid;
grid-column: line / span 2;
font-family: monospace;
white-space: pre;
text-wrap: wrap;
.number {
grid-column-start: line;
text-align: end;
user-select: none;
}
.content {
grid-column-start: content;
}
}
}
}
36 changes: 30 additions & 6 deletions src/api/app/controllers/webui/packages/files_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@
before_action :set_project
before_action :check_scmsync, only: :show
before_action :set_package
before_action :set_filename, only: %i[show update destroy]
before_action :ensure_existence, only: :show
before_action :ensure_viewable, only: :show
before_action :set_file, only: :show
before_action :set_filename, only: %i[show update destroy blame]
before_action :ensure_existence, only: %i[show blame]
before_action :ensure_viewable, only: %i[show blame]
before_action :set_file, only: %i[show blame]

after_action :verify_authorized, except: :show
after_action :verify_authorized, except: %i[show blame]

def show
@rev = params[:rev]
Expand Down Expand Up @@ -111,10 +111,34 @@
redirect_to package_show_path(project: @project, package: @package)
end

def blame
blame_file = Backend::Api::Sources::Package.blame(@project.name, @package_name, @filename, params.slice(:rev, :expand).permit!)

Check warning on line 115 in src/api/app/controllers/webui/packages/files_controller.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/controllers/webui/packages/files_controller.rb#L115

Added line #L115 was not covered by tests
# Regex to break apart the line into individual components
blame_parsed = blame_file.each_line.to_a.filter_map do |l|
match_line = /^\s*

Check warning on line 118 in src/api/app/controllers/webui/packages/files_controller.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/controllers/webui/packages/files_controller.rb#L117-L118

Added lines #L117 - L118 were not covered by tests
(?<file>\d*:)? # File prefix (optional)
(?<revision>\d*)\s # Revision number
\( # Metadata in parenthesis
(?<login>\S+)\s+ # User login
(?<date>[\d-]+)\s # Date, dash separated
(?<time>[\d:]+)\s+ # Time, colon separated
(?<line>\d+) # Line number
\)\s
(?<content>.*) # Content of the line
$/x
match_line.match(l)

Check warning on line 129 in src/api/app/controllers/webui/packages/files_controller.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/controllers/webui/packages/files_controller.rb#L129

Added line #L129 was not covered by tests
Copy link
Member

Choose a reason for hiding this comment

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

Guess this screams for a backend feature for structured blame data...

Copy link
Member

Choose a reason for hiding this comment

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

Also, you need to spec this parsing or it'll break left and right.

end
revision_numbers = blame_parsed.pluck('revision').uniq.compact_blank
@revisions = revision_numbers.index_with { |r| @package.commit(r) }
@blame_info = blame_parsed.slice_when { |a, b| a['revision'] != b['revision'] }.to_a
@rev = params[:rev] || revision_numbers.max
@expand = params[:expand]

Check warning on line 135 in src/api/app/controllers/webui/packages/files_controller.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/controllers/webui/packages/files_controller.rb#L131-L135

Added lines #L131 - L135 were not covered by tests
end

private

def set_filename
@filename = params[:filename]
@filename = params[:filename] || params[:file_filename]
end

def ensure_existence
Expand Down
6 changes: 6 additions & 0 deletions src/api/app/lib/backend/api/sources/package.rb
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,12 @@
http_get(['/source/:project/:package/:filename', project_name, package_name, file_name])
end

# Returns the content of the source file
# @return [String]
def self.blame(project_name, package_name, file_name, options = {})
http_get(['/source/:project/:package/:filename', project_name, package_name, file_name], defaults: { view: :blame }, params: options, accepted: %i[meta deleted expand rev view])

Check warning on line 146 in src/api/app/lib/backend/api/sources/package.rb

View check run for this annotation

Codecov / codecov/patch

src/api/app/lib/backend/api/sources/package.rb#L146

Added line #L146 was not covered by tests
end

# Writes the content of the source file
# @return [String]
def self.write_file(project_name, package_name, file_name, content = '', params = {})
Expand Down
7 changes: 5 additions & 2 deletions src/api/app/views/webui/package/_file.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,11 @@
/ limit download for anonymous user to avoid getting killed by crawlers
- unless nobody
%td.text-center<
- if viewable_file?(file['name'], file['size'].to_i)
= link_to(project_package_file_blame_path(project, package, file['name'], expand: expand), title: 'Blame file') do
%i.fas.fa-fw.fa-file-waveform
= link_to(file_url(project.name, package.name, file['name'], srcmd5), title: 'Download file', class: 'me-1') do
%i.fas.fa-download.text-secondary
%i.fas.fa-fw.fa-download.text-secondary
- if can_be_removed
= link_to('#', title: 'Delete file', data: { 'bs-toggle': 'modal',
'bs-target': '#delete-file-modal',
Expand All @@ -25,4 +28,4 @@
project_name: project,
package_name: package,
filename: file['name']) }) do
%i.fas.fa-times-circle.text-danger
%i.fas.fa-fw.fa-times-circle.text-danger
43 changes: 43 additions & 0 deletions src/api/app/views/webui/packages/files/blame.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
- @pagetitle = "Blame #{truncate(@filename, length: 50)} of Package #{truncate(@package.name, length: 50)}"

.card.mb-3
= render partial: 'webui/package/tabs', locals: { project: @project, package: @package }
.card-body
%h3.text-word-break-all
Blame #{@filename} of Package #{@package}
- if @rev
(Revision #{@rev})
- if params[:rev]
%p
Currently displaying revision
%i= @rev
,
= link_to('Show latest', expand: @expand)
// Coderay doesn't support rpmspec, which would be prerequisite here
-# code = CodeRay.scan(@file, :ruby).html.each_line.to_a
.blame
- @blame_info.each do |revision|
.revision
.info
- info = revision.first
.time= render TimeComponent.new(time: "#[info['date']} #{info['time']}".to_datetime)
.user
- if (user = User.find_by_login(info['login']))
= link_to(user_path(user)) do
= render AvatarComponent.new(name: info['login'], email: user.email)
.commit.text-truncate
- rev = @revisions[info['revision']]
- if rev
= link_to(package_show_path(@project, @package_name, rev: info['revision'])) do
- if rev['comment']
= rev['comment']
- else
%i No commit message
.button
- if info['revision'].present? && info['revision'] != @rev
= link_to(project_package_file_blame_path(@project, @package_name, @filename, rev: info['revision'])) do
%i.fas.fa-eye
.code.CodeRay<>
- revision.each do |line|
.number= line['line']
.content= line['content']
4 changes: 3 additions & 1 deletion src/api/config/routes/webui.rb
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,9 @@
resources :deletions, controller: 'webui/requests/deletions', only: %i[new create], constraints: cons
resources :devel_project_changes, controller: 'webui/requests/devel_project_changes', only: %i[new create], constraints: cons
resources :submissions, controller: 'webui/requests/submissions', only: %i[new create], constraints: cons
resources :files, controller: 'webui/packages/files', only: %i[new create show update destroy], constraints: cons, param: :filename, format: false, defaults: { format: 'html' }
resources :files, controller: 'webui/packages/files', only: %i[new create show update destroy], constraints: cons, param: :filename, format: false, defaults: { format: 'html' } do
get :blame
end
put 'toggle_watched_item', controller: 'webui/watched_items', constraints: cons
resource :badge, controller: 'webui/packages/badge', only: [:show], constraints: cons.merge(format: :svg)
resources :repositories, only: [], param: :name do
Expand Down