From a491b9f9d4dc091205f9f2334cc2030a2ea6e0a4 Mon Sep 17 00:00:00 2001 From: Eduardo Navarro Date: Fri, 20 Jan 2023 12:04:38 +0100 Subject: [PATCH] Show the reviewer why their review was requested --- .../webui/request_show_redesign/add_review.js | 11 ++++++ ...add_review_collapsible_component.html.haml | 37 ++++++++++--------- .../add_review_dropdown_component.html.haml | 13 ++++--- .../add_review_dropdown_component.rb | 9 ++++- .../bs_request_activity_timeline_component.rb | 4 +- .../controllers/webui/request_controller.rb | 1 + .../views/webui/request/beta_show.html.haml | 6 ++- .../_conversation_aside.html.haml | 4 +- 8 files changed, 57 insertions(+), 28 deletions(-) diff --git a/src/api/app/assets/javascripts/webui/request_show_redesign/add_review.js b/src/api/app/assets/javascripts/webui/request_show_redesign/add_review.js index 6c524fe49bd..4fa85159fb2 100644 --- a/src/api/app/assets/javascripts/webui/request_show_redesign/add_review.js +++ b/src/api/app/assets/javascripts/webui/request_show_redesign/add_review.js @@ -4,6 +4,7 @@ function handleReviewerCollapsibleForm() { // jshint ignore:line $('#review-form-collapse h5 i').html(review.html()); $('#review_id').val(review.data('review')); + fillReviewReason(review.data('review-reason')); }); $('#add-review-dropdown-component').on('shown.bs.dropdown', function () { @@ -13,8 +14,18 @@ function handleReviewerCollapsibleForm() { // jshint ignore:line $('.toggle-review-form').on('click', function() { $('#review-form-collapse h5 i').html(this.dataset.reviewerIcon); $('#review_id').val(this.dataset.review); + fillReviewReason(this.dataset.reviewReason); }); + function fillReviewReason(text) { + if (text) { + $('#review-reason').removeClass('d-none').html(text); + } + else { + $('#review-reason').addClass('d-none'); + } + } + $(document).click(function(e) { var reviewCollapsible = document.getElementById('review-form-collapse'); if (!reviewCollapsible.contains(e.target)) { diff --git a/src/api/app/components/add_review_collapsible_component.html.haml b/src/api/app/components/add_review_collapsible_component.html.haml index 04cb0456255..0aa6823c437 100644 --- a/src/api/app/components/add_review_collapsible_component.html.haml +++ b/src/api/app/components/add_review_collapsible_component.html.haml @@ -3,20 +3,23 @@ %h5 Give a review for %i - = form_tag(request_modify_review_path) do - = hidden_field_tag('review_id') - .mb-3 - = text_area_tag('comment', '', rows: 4, class: 'w-100 form-control', placeholder: 'Please comment on your decision') - .mb-3 - .form-check - = radio_button_tag('new_state', 'Decline', true, class: 'form-check-input', aria: { describedby: 'decline-description' }) - = label_tag('Decline', 'Decline', for: 'new_state_Decline', class: 'form-check-label') - %small.form-text.text-muted#decline-description - Veto the review. This will also decline the request. - .form-check - = radio_button_tag('new_state', 'Approve', false, class: 'form-check-input', aria: { describedby: 'approve-description' }) - = label_tag('Approve', for: 'new_state_Approve', class: 'form-check-label') do - Approve - %small.form-text.text-muted#approve-description - Give your consent for the review. The request will move forward. - = submit_tag 'Submit review', title: 'Submit the review.', class: 'btn btn-primary' + %ul.list-group.list-group-flush + %li.list-group-item#review-reason + %li.list-group-item + = form_tag(request_modify_review_path) do + = hidden_field_tag('review_id') + .mb-3 + = text_area_tag('comment', '', rows: 4, class: 'w-100 form-control', placeholder: 'Please comment on your decision') + .mb-3 + .form-check + = radio_button_tag('new_state', 'Decline', true, class: 'form-check-input', aria: { describedby: 'decline-description' }) + = label_tag('Decline', 'Decline', for: 'new_state_Decline', class: 'form-check-label') + %small.form-text.text-muted#decline-description + Veto the review. This will also decline the request. + .form-check + = radio_button_tag('new_state', 'Approve', false, class: 'form-check-input', aria: { describedby: 'approve-description' }) + = label_tag('Approve', for: 'new_state_Approve', class: 'form-check-label') do + Approve + %small.form-text.text-muted#approve-description + Give your consent for the review. The request will move forward. + = submit_tag 'Submit review', title: 'Submit the review.', class: 'btn btn-primary' diff --git a/src/api/app/components/add_review_dropdown_component.html.haml b/src/api/app/components/add_review_dropdown_component.html.haml index 81d509ea2b7..6cfa57ed236 100644 --- a/src/api/app/components/add_review_dropdown_component.html.haml +++ b/src/api/app/components/add_review_dropdown_component.html.haml @@ -1,7 +1,9 @@ - if @my_open_reviews.count <= 1 - .btn.btn-success.toggle-review-form{ type: 'button', data: { 'bs-toggle': 'collapse', 'bs-target': '#review-form-collapse', - review: @my_open_reviews.first.id, reviewer_icon: reviewer_icon_and_text(review: @my_open_reviews.first) }, - aria: { expanded: 'false', controls: 'review-form-collapse' } } + - open_review = @my_open_reviews.first + .btn.btn-success.toggle-review-form{ type: 'button', aria: { expanded: 'false', controls: 'review-form-collapse' }, + data: { 'bs-toggle': 'collapse', 'bs-target': '#review-form-collapse', + review: open_review.id, reviewer_icon: reviewer_icon_and_text(review: open_review), + 'review-reason': helpers.render_as_markdown(reason_when_review_was_requested(review: open_review)) } } Review - else .dropdown#add-review-dropdown-component @@ -10,8 +12,9 @@ .dropdown-menu.dropdown-menu-start %h5.dropdown-header Give a review for... - @my_open_reviews.each do |review| - = button_tag(type: 'button', class: 'dropdown-item', data: { 'bs-toggle': 'collapse', 'bs-target': '#review-form-collapse', - review: review.id }, aria: { expanded: 'false', controls: 'review-form-collapse' }) do + = button_tag(type: 'button', class: 'dropdown-item', aria: { expanded: 'false', controls: 'review-form-collapse' }, + data: { 'bs-toggle': 'collapse', 'bs-target': '#review-form-collapse', review: review.id, + 'review-reason': helpers.render_as_markdown(reason_when_review_was_requested(review: review)) }) do = reviewer_icon_and_text(review: review) :javascript diff --git a/src/api/app/components/add_review_dropdown_component.rb b/src/api/app/components/add_review_dropdown_component.rb index a6814515118..b607e2830b8 100644 --- a/src/api/app/components/add_review_dropdown_component.rb +++ b/src/api/app/components/add_review_dropdown_component.rb @@ -1,10 +1,11 @@ class AddReviewDropdownComponent < ApplicationComponent - def initialize(bs_request:, user:, my_open_reviews:) + def initialize(bs_request:, user:, my_open_reviews:, history_elements:) super @bs_request = bs_request @user = user @my_open_reviews = my_open_reviews + @history_elements = history_elements end def render? @@ -23,4 +24,10 @@ def reviewer_icon_and_text(review:) tag.i(nil, class: 'fa fa-cubes me-2') + "#{review.by_project}" end end + + def reason_when_review_was_requested(review:) + reason = @history_elements.reverse.find { |history_element| history_element.type == 'HistoryElement::RequestReviewAdded' && history_element.description_extension == review.id.to_s }&.comment + + reason || '' + end end diff --git a/src/api/app/components/bs_request_activity_timeline_component.rb b/src/api/app/components/bs_request_activity_timeline_component.rb index bdbbbf561c6..663d938a55b 100644 --- a/src/api/app/components/bs_request_activity_timeline_component.rb +++ b/src/api/app/components/bs_request_activity_timeline_component.rb @@ -6,7 +6,7 @@ class BsRequestActivityTimelineComponent < ApplicationComponent attr_reader :bs_request, :creator, :timeline, :request_reviews_for_non_staging_projects - def initialize(bs_request:, request_reviews_for_non_staging_projects: []) + def initialize(bs_request:, history_elements:, request_reviews_for_non_staging_projects: []) super @bs_request = bs_request @creator = User.find_by_login(bs_request.creator) || User.nobody @@ -17,7 +17,7 @@ def initialize(bs_request:, request_reviews_for_non_staging_projects: []) @timeline = ( action_comments + @bs_request.comments.without_parent.includes(:user) + - @bs_request.history_elements.includes(:user) + history_elements ).compact.sort_by(&:created_at) @request_reviews_for_non_staging_projects = request_reviews_for_non_staging_projects end diff --git a/src/api/app/controllers/webui/request_controller.rb b/src/api/app/controllers/webui/request_controller.rb index 05fe27285f3..4e4782e9e0c 100644 --- a/src/api/app/controllers/webui/request_controller.rb +++ b/src/api/app/controllers/webui/request_controller.rb @@ -29,6 +29,7 @@ class Webui::RequestController < Webui::WebuiController def show # TODO: Remove this `if` condition, and the `else` clause once request_show_redesign is rolled out if Flipper.enabled?(:request_show_redesign, User.session) + @history_elements = @bs_request.history_elements.includes(:user) @active_tab = 'conversation' render :beta_show else diff --git a/src/api/app/views/webui/request/beta_show.html.haml b/src/api/app/views/webui/request/beta_show.html.haml index 5422669a825..c71f3746a38 100644 --- a/src/api/app/views/webui/request/beta_show.html.haml +++ b/src/api/app/views/webui/request/beta_show.html.haml @@ -25,7 +25,8 @@ request_reviews: @request_reviews, package_maintainers: @package_maintainers, project_maintainers: @project_maintainers, - show_project_maintainer_hint: @show_project_maintainer_hint } + show_project_maintainer_hint: @show_project_maintainer_hint, + history_elements: @history_elements } .col-md-8.order-md-1.order-sm-2 .row @@ -78,9 +79,10 @@ Collapse history from superseded request ##{superseding_request.number} .collapse.mb-4#collapse-superseding = render BsRequestActivityTimelineComponent.new(bs_request: superseding_request, + history_elements: @history_elements, request_reviews_for_non_staging_projects: @request_reviews) - = render BsRequestActivityTimelineComponent.new(bs_request: @bs_request, + history_elements: @history_elements, request_reviews_for_non_staging_projects: @request_reviews) .comment_new diff --git a/src/api/app/views/webui/request/beta_show_tabs/_conversation_aside.html.haml b/src/api/app/views/webui/request/beta_show_tabs/_conversation_aside.html.haml index 688a65d5f0a..2a0ed6bab34 100644 --- a/src/api/app/views/webui/request/beta_show_tabs/_conversation_aside.html.haml +++ b/src/api/app/views/webui/request/beta_show_tabs/_conversation_aside.html.haml @@ -2,7 +2,9 @@ -# REVIEWERS - if request_reviews.present? .text-end - = render AddReviewDropdownComponent.new(bs_request: bs_request, user: User.session, my_open_reviews: my_open_reviews) + = render AddReviewDropdownComponent.new(bs_request: bs_request, user: User.session, + my_open_reviews: my_open_reviews, + history_elements: history_elements) = render AddReviewCollapsibleComponent.new .mb-2 %h6