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

Avoid database queries on sourcediff component #15799

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 12 additions & 18 deletions src/api/app/controllers/webui/request_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ class Webui::RequestController < Webui::WebuiController
before_action :cache_diff_data, only: %i[show build_results rpm_lint changes mentioned_issues],
if: -> { Flipper.enabled?(:request_show_redesign, User.session) }
before_action :check_beta_user_redirect, only: %i[build_results rpm_lint changes mentioned_issues]
before_action :set_webui_actions, only: %i[request_action request_action_changes]

after_action :verify_authorized, only: [:create]

Expand Down Expand Up @@ -157,15 +158,8 @@ def modify_review

# TODO: Remove this once request_show_redesign is rolled out
def request_action
@diff_limit = params[:full_diff] ? 0 : nil
@index = params[:index].to_i
@actions = @bs_request.webui_actions(filelimit: @diff_limit, tarlimit: @diff_limit, diff_to_superseded: @diff_to_superseded, diffs: true,
action_id: params['id'].to_i, cacheonly: 1)
@action = @actions.find { |action| action[:id] == params['id'].to_i }
@active = @action[:name]
@not_full_diff = BsRequest.truncated_diffs?(@actions)
@diff_to_superseded_id = params[:diff_to_superseded]

if @action[:diff_not_cached]
bs_request_action = BsRequestAction.find(@action[:id])
job = Delayed::Job.where('handler LIKE ?', "%job_class: BsRequestActionWebuiInfosJob%#{bs_request_action.to_global_id.uri}%").count
Expand All @@ -178,17 +172,6 @@ def request_action
end

def request_action_changes
# TODO: Change @diff_limit to a local variable
@diff_limit = params[:full_diff] ? 0 : nil
# TODO: Change @actions to a local variable
@actions = @bs_request.webui_actions(filelimit: @diff_limit, tarlimit: @diff_limit, diff_to_superseded: @diff_to_superseded, diffs: true,
action_id: params['id'].to_i, cacheonly: 1)
@action = @actions.find { |action| action[:id] == params['id'].to_i }
# TODO: Check if @not_full_diff is really needed
@not_full_diff = BsRequest.truncated_diffs?(@actions)
# TODO: Check if @diff_to_superseded_id is really needed
@diff_to_superseded_id = params[:diff_to_superseded]

cache_diff_data

respond_to do |format|
Expand Down Expand Up @@ -578,4 +561,15 @@ def set_influxdb_data_request_actions
bs_request_action_type: @bs_request_action.class.name
}
end

def set_webui_actions
diff_limit = params[:full_diff] ? 0 : nil
@actions = @bs_request.webui_actions(filelimit: diff_limit,
tarlimit: diff_limit,
diff_to_superseded: @diff_to_superseded,
diffs: true,
action_id: params['id'].to_i,
cacheonly: 1)
@action = @actions.find { |action| action[:id] == params['id'].to_i }
end
end
5 changes: 2 additions & 3 deletions src/api/app/views/webui/request/_actions_details.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
.d-flex.flex-wrap.align-items-center
.input-group.w-auto
= link_to('Previous', next_prev_path(number: bs_request.number, request_action_id: prev_action&.id,
full_diff: diff_limit, diff_to_superseded: diff_to_superseded_id, page_name: page_name),
diff_to_superseded: diff_to_superseded_id, page_name: page_name),
class: "btn btn-primary btn-sm #{prev_action ? '' : 'disabled'}",
id: 'previous-action-button')
.dropdown#request-actions
Expand All @@ -25,12 +25,11 @@
= link_to((render partial: 'action_text', locals: { action: action_item, action_index: action_index }),
next_prev_path(number: bs_request.number,
request_action_id: action_item.id,
full_diff: diff_limit,
diff_to_superseded: diff_to_superseded_id, page_name: page_name),
class: "dropdown-item #{action_item.id == active_action.id ? 'active' : ''}",
'aria-current': 'true')
= link_to('Next', next_prev_path(number: bs_request.number, request_action_id: next_action&.id,
full_diff: diff_limit, diff_to_superseded: diff_to_superseded_id, page_name: page_name),
diff_to_superseded: diff_to_superseded_id, page_name: page_name),
class: "btn btn-primary btn-sm #{next_action ? '' : 'disabled'}",
id: 'next-action-button')

Expand Down
2 changes: 1 addition & 1 deletion src/api/app/views/webui/request/_request_header.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -77,5 +77,5 @@
= render partial: 'actions_details', locals: { bs_request: bs_request, action: action, active_action: active_action,
prev_action: prev_action, next_action: next_action,
actions: supported_actions, actions_count: supported_actions.count,
diff_to_superseded_id: diff_to_superseded_id, diff_limit: diff_limit,
diff_to_superseded_id: diff_to_superseded_id,
page_name: page_name }
2 changes: 1 addition & 1 deletion src/api/app/views/webui/request/build_results.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
= 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_build_results',
diff_to_superseded_id: @diff_to_superseded_id, page_name: 'request_build_results',
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,
Expand Down
4 changes: 0 additions & 4 deletions src/api/spec/controllers/webui/request_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -89,10 +89,6 @@
get :request_action, params: { number: bs_request.number, index: 0, id: bs_request.bs_request_actions.first.id, format: :js }, xhr: true
end

it 'shows a hint' do
expect(assigns(:not_full_diff)).to be_truthy
end

it 'shows the truncated diff' do
actions = assigns(:actions).select { |action| action[:type] == :submit && action[:sourcediff] }
diff_size = actions.first[:sourcediff].first['files'][file_name]['diff']['_content'].split.size
Expand Down