Skip to content

Commit

Permalink
Show the diff against the superseded request
Browse files Browse the repository at this point in the history
  • Loading branch information
danidoni committed Feb 13, 2024
1 parent e8a80d9 commit b395729
Show file tree
Hide file tree
Showing 8 changed files with 130 additions and 13 deletions.
38 changes: 38 additions & 0 deletions src/api/app/components/sourcediff_tab_beta_component.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
.row
- if @action[:diff_not_cached]
.col-lg-12
.clearfix.mb-2.text-center
.btn.btn-outline-primary.cache-refresh{ title: 'Refresh results', onclick: "reloadRequestAction(#{@index})" }
Crunching the latest data. Refresh again in a few seconds
%i.fas.fa-sync-alt{ id: "cache#{@index}-reload" }
- else
- (@action[:sourcediff] || []).each do |sourcediff|
.clearfix.mb-2
.btn-group.float-end
%button.btn.btn-outline-secondary.expand-diffs{ data: { package: @action[:spkg] } }
Expand all
%button.btn.btn-outline-secondary.collapse-diffs{ data: { package: @action[:spkg] } }
Collapse all
- if sourcediff[:error]
%p
%i.error
= sourcediff[:error]
- else
- if @action[:sourcediff].length > 1
%h4
#{diff_label(sourcediff['new'])}#{diff_label(sourcediff['old'])}
- if sourcediff['filenames'].present?
- sourcediff['filenames'].each_with_index do |filename, file_index|
.mb-2
= render partial: 'webui/package/revision_diff_detail', locals: { file_view_path: file_view_path(filename, sourcediff),
filename: filename,
file: sourcediff['files'][filename],
index: file_index,
last: sourcediff['filenames'].last == filename,
package: @action[:spkg],
linkinfo: nil }
- else
.mb-2
%p.lead
No source changes.
36 changes: 36 additions & 0 deletions src/api/app/components/sourcediff_tab_beta_component.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
# frozen_string_literal: true

class SourcediffTabBetaComponent < ApplicationComponent
attr_accessor :bs_request, :action, :active, :index

delegate :valid_xml_id, to: :helpers
delegate :request_action_header, to: :helpers
delegate :diff_label, to: :helpers
delegate :diff_data, to: :helpers

def initialize(bs_request:, action:, active:, index:)
super

@bs_request = bs_request
@action = action
@active = active
@index = index
end

def file_view_path(filename, sourcediff)
return if sourcediff['files'][filename]['state'] == 'deleted'

diff_params = diff_data(@action[:type], sourcediff)
package_view_file_path(diff_params.merge(filename: filename))
end

def release_info
@action[:type] == :maintenance_incident && @action[:releaseproject]
end

def active_class
return if @active != @action[:name]

'active'
end
end
3 changes: 1 addition & 2 deletions src/api/app/components/sourcediff_tab_component.rb
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# frozen_string_literal: true

# This component renders the diff between a request and its superseded request
class SourcediffTabComponent < ApplicationComponent
attr_accessor :bs_request, :action, :active, :index

Expand Down
33 changes: 22 additions & 11 deletions src/api/app/controllers/webui/request_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,27 +7,28 @@ class Webui::RequestController < Webui::WebuiController
before_action :lockout_spiders
before_action :require_request,
only: [:changerequest, :show, :request_action, :request_action_changes, :inline_comment, :build_results, :rpm_lint,
:changes, :mentioned_issues, :chart_build_results, :complete_build_results]
before_action :set_actions, only: [:inline_comment, :show, :build_results, :rpm_lint, :changes, :mentioned_issues, :chart_build_results, :complete_build_results],
:changes, :supersede_diff, :mentioned_issues, :chart_build_results, :complete_build_results]
before_action :set_actions, only: [:inline_comment, :show, :build_results, :rpm_lint, :changes, :supersede_diff, :mentioned_issues, :chart_build_results, :complete_build_results],
if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
before_action :build_results_data, only: [:show], if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
before_action :set_supported_actions, only: [:inline_comment, :show, :build_results, :rpm_lint, :changes, :mentioned_issues],
before_action :set_supported_actions, only: [:inline_comment, :show, :build_results, :rpm_lint, :changes, :supersede_diff, :mentioned_issues],
if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
before_action :set_action_id, only: [:inline_comment, :show, :build_results, :rpm_lint, :changes, :mentioned_issues],
before_action :set_action_id, only: [:inline_comment, :show, :build_results, :rpm_lint, :changes, :supersede_diff, :mentioned_issues],
if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
before_action :set_bs_request_action, only: [:show, :build_results, :rpm_lint, :changes, :mentioned_issues],
before_action :set_bs_request_action, only: [:show, :build_results, :rpm_lint, :changes, :supersede_diff, :mentioned_issues],
if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
before_action :set_influxdb_data_request_actions, only: [:show, :build_results, :rpm_lint, :changes, :mentioned_issues],
before_action :set_influxdb_data_request_actions, only: [:show, :build_results, :rpm_lint, :changes, :supersede_diff, :mentioned_issues],
if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
before_action :set_active_action, only: [:inline_comment, :show, :build_results, :rpm_lint, :changes, :mentioned_issues],
before_action :set_active_action, only: [:inline_comment, :show, :build_results, :rpm_lint, :changes, :supersede_diff, :mentioned_issues],
if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
before_action :set_superseded_request, only: [:show, :request_action, :request_action_changes, :build_results, :rpm_lint, :changes, :mentioned_issues]
before_action :set_superseded_request, only: [:show, :request_action, :request_action_changes, :build_results, :rpm_lint, :changes, :supersede_diff, :mentioned_issues]
before_action :check_ajax, only: :sourcediff
before_action :prepare_request_data, only: [:show, :build_results, :rpm_lint, :changes, :mentioned_issues],
before_action :force_diff_to_superseded, only: [:supersede_diff]
before_action :prepare_request_data, only: [:show, :build_results, :rpm_lint, :changes, :supersede_diff, :mentioned_issues],
if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
before_action :cache_diff_data, only: [:show, :build_results, :rpm_lint, :changes, :mentioned_issues],
before_action :cache_diff_data, only: [:show, :build_results, :rpm_lint, :changes, :supersede_diff, :mentioned_issues],
if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
before_action :check_beta_user_redirect, only: [:build_results, :rpm_lint, :changes, :mentioned_issues]
before_action :check_beta_user_redirect, only: [:build_results, :rpm_lint, :changes, :supersede_diff, :mentioned_issues]

after_action :verify_authorized, only: [:create]

Expand Down Expand Up @@ -313,6 +314,12 @@ def changes
@active_tab = 'changes'
end

def supersede_diff
redirect_to request_show_path(params[:number], params[:request_action_id]) unless @bs_request_action.tab_visibility.changes

@active_tab = 'supersede_diff'
end

def mentioned_issues
redirect_to request_show_path(params[:number], params[:request_action_id]) unless @bs_request_action.tab_visibility.issues

Expand Down Expand Up @@ -533,6 +540,10 @@ def handle_notification
authorize @current_notification, :update?, policy_class: NotificationPolicy
end

def force_diff_to_superseded
@diff_to_superseded = @bs_request.superseding.first
end

def prepare_request_data
@is_author = @bs_request.creator == User.possibly_nobody.login
@is_target_maintainer = @bs_request.is_target_maintainer?(User.session)
Expand Down
5 changes: 5 additions & 0 deletions src/api/app/helpers/webui/request_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -163,6 +163,11 @@ def request_action_header(action, creator)
# TODO: merge these extra conditions when request_show_redesign is rolled out.
if Flipper.enabled?(:request_show_redesign, User.session)
description = case action[:type]
when :submit
'Submit %{source_container} to %{target_container}' % {
source_container: project_or_package_link(source_project_hash),
target_container: project_or_package_link(project: action[:tprj], package: action[:tpkg])
}
when :change_devel
'Set %{source_container} to be devel project/package of %{target_container}' % {
source_container: project_or_package_link(source_project_hash),
Expand Down
9 changes: 9 additions & 0 deletions src/api/app/views/webui/request/_request_tabs.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,15 @@
%li.nav-item.scrollable-tab-link
= link_to('Changes', request_changes_path(bs_request.number, actions_count > 1 ? active_action : nil),
class: "nav-link text-nowrap #{active_tab == 'changes' ? 'active' : ''}")
- if bs_request.superseding.exists?
%li.nav-item.scrollable-tab-link
- if bs_request.superseding.present?
- for superseded in bs_request.superseding
= link_to("Diff To Superseded", request_supersede_diff_path(bs_request.number, actions_count > 1 ? active_action : nil, diff_to_superseded: superseded.number),
class: "nav-link text-nowrap #{active_tab == 'supersede_diff' ? 'active' : ''}")
- else
= link_to('Supersede Diff', request_supersede_diff_path(bs_request.number, actions_count > 1 ? active_action : nil),
class: "nav-link text-nowrap #{active_tab == 'supersede_diff' ? 'active' : ''}")
- if bs_request_action.tab_visibility.issues
%li.nav-item.scrollable-tab-link
= link_to(request_mentioned_issues_path(bs_request.number, actions_count > 1 ? active_action : nil),
Expand Down
18 changes: 18 additions & 0 deletions src/api/app/views/webui/request/supersede_diff.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
:ruby
@pagetitle = "Request #{@bs_request.number}: #{@action[:name]}"

= render partial: 'beta_alert', locals: { bs_request: @bs_request, action: @action }

.card
.card-body.p-0
= render partial: 'request_header',
locals: { bs_request: @bs_request, staging_status: @staging_status, action: @action, active_action: @active_action,
prev_action: @prev_action, next_action: @next_action, supported_actions: @supported_actions,
diff_to_superseded_id: @diff_to_superseded_id, diff_limit: @diff_limit, page_name: 'request_changes',
bs_requests: @watched_requests, packages: @watched_packages, projects: @watched_projects }
= render partial: 'request_tabs',
locals: { bs_request: @bs_request, bs_request_action: @bs_request_action, issues: @issues, active_action: @active_action,
actions_count: @supported_actions.count, active_tab: @active_tab }
.container.p-4
.tab-content.supersedediff
= render(SourcediffTabBetaComponent.new(bs_request: @bs_request, action: @action, active: @active_action, index: 0))
1 change: 1 addition & 0 deletions src/api/config/routes/webui_routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -311,6 +311,7 @@
get 'request/show/:number/(request_action/:request_action_id)/build_results' => :build_results, as: 'request_build_results', constraints: cons
get 'request/show/:number/(request_action/:request_action_id)/rpm_lint' => :rpm_lint, as: 'request_rpm_lint', constraints: cons
get 'request/show/:number/(request_action/:request_action_id)/changes' => :changes, as: 'request_changes', constraints: cons
get 'request/show/:number/(request_action/:request_action_id)/supersede_diff' => :supersede_diff, as: 'request_supersede_diff', constraints: cons
get 'request/show/:number/(request_action/:request_action_id)/mentioned_issues' => :mentioned_issues, as: 'request_mentioned_issues', constraints: cons
post 'request/sourcediff' => :sourcediff
post 'request/changerequest' => :changerequest
Expand Down

0 comments on commit b395729

Please sign in to comment.