Merge pull request #15900 from krauselukas/show_review_reason

Show the reviewer why their review was requested

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)) {

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'

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

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

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

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

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

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

src/api/spec/components/bs_request_activity_timeline_component_spec.rb

@@ -4,11 +4,11 @@
   let!(:comment) { travel_to(1.day.ago) { create(:comment_request, commentable: bs_request) } }
 
   it 'shows the comment first, as it is an older timeline item' do
-    expect(render_inline(described_class.new(bs_request: bs_request))).to have_css('.timeline-item:first-child', text: comment.user.login)
-    expect(render_inline(described_class.new(bs_request: bs_request))).to have_css('.timeline-item:first-child', text: '1 day ago')
+    expect(render_inline(described_class.new(bs_request: bs_request, history_elements: [history_element]))).to have_css('.timeline-item:first-child', text: comment.user.login)
+    expect(render_inline(described_class.new(bs_request: bs_request, history_elements: [history_element]))).to have_css('.timeline-item:first-child', text: '1 day ago')
   end
 
   it 'shows the history element in the second position, as it is more recent' do
-    expect(render_inline(described_class.new(bs_request: bs_request))).to have_css('.timeline-item:last-child', text: 'accepted request')
+    expect(render_inline(described_class.new(bs_request: bs_request, history_elements: [history_element]))).to have_css('.timeline-item:last-child', text: 'accepted request')
   end
 end