Merge pull request #1134 from dradis/qa-activity

QA: State change activity

CHANGELOG

@@ -1,6 +1,5 @@
 [v#.#.#] ([month] [YYYY])
-  - [entity]:
-    - [future tense verb] [feature]
+  - QA: Show state changes in activity feed
   - Upgraded gems:
     - [gem]
   - Bugs fixes:

app/controllers/concerns/activity_tracking.rb

@@ -30,4 +30,8 @@ def track_recovered(trackable, user: current_user, project: current_project)
   def track_updated(trackable, user: current_user, project: current_project)
     track_activity(trackable, :update, user, project)
   end
+
+  def track_state_change(trackable, user: current_user, project: current_project)
+    track_activity(trackable, :state_change, user, project)
+  end
 end

app/controllers/qa/issues_controller.rb

@@ -1,4 +1,5 @@
 class QA::IssuesController < AuthenticatedController
+  include ActivityTracking
   include LiquidEnabledResource
   include ProjectScoped
 
@@ -21,6 +22,7 @@ def edit
 
   def update
     if @issue.update(state: @state, updated_at: Time.now)
+      track_state_change(@issue)
       redirect_to project_qa_issues_path(current_project), notice: 'State updated successfully.'
     else
       render :show, alert: @issue.errors.full_messages.join('; ')
@@ -32,6 +34,11 @@ def multiple_update
 
     respond_to do |format|
       if @issues.update_all(state: @state, updated_at: Time.now)
+
+        @issues.each do |issue|
+          track_state_change(issue)
+        end
+
         format.html { redirect_to_target_or_default project_qa_issues_path(current_project), notice: 'State updated successfully.' }
         format.json { head :ok }
       else

app/models/activity.rb

@@ -17,14 +17,14 @@ def project=(new_project); end
 
   validates_presence_of :action, :trackable_id, :trackable_type, :user
 
-  VALID_ACTIONS = %w[create update destroy recover]
+  VALID_ACTIONS = %w[create destroy recover state_change update]
 
   validates_inclusion_of :action, in: VALID_ACTIONS
 
   # -- Scopes ---------------------------------------------------------------
 
   scope :latest, -> do
-    includes(:trackable).order("activities.created_at DESC").limit(20)
+    includes(:trackable).order('activities.created_at DESC').limit(20)
   end
 
   # -- Callbacks ------------------------------------------------------------
@@ -45,7 +45,7 @@ def project=(new_project); end
   # FIXME - ISSUE/NOTE INHERITANCE
   def trackable=(new_trackable)
     super
-    self.trackable_type = "Issue" if new_trackable.is_a?(Issue)
+    self.trackable_type = 'Issue' if new_trackable.is_a?(Issue)
     new_trackable
   end
 

app/presenters/activity_presenter.rb

@@ -41,22 +41,23 @@ def created_at_ago
 
   def icon
     icon_css = %w{activity-icon fa}
-    icon_css << case activity.trackable_type
-                when 'Board', 'List', 'Card'
-                  'fa-trello'
-                when 'Comment'
-                  'fa-comment'
-                when 'Evidence'
-                  'fa-flag'
-                when 'Issue'
-                  'fa-bug'
-                when 'Node'
-                  'fa-folder-o'
-                when 'Note'
-                  'fa-file-text-o'
-                else
-                  ''
-                end
+    icon_css <<
+      case activity.trackable_type
+      when 'Board', 'List', 'Card'
+        'fa-trello'
+      when 'Comment'
+        'fa-comment'
+      when 'Evidence'
+        'fa-flag'
+      when 'Issue'
+        'fa-bug'
+      when 'Node'
+        'fa-folder-o'
+      when 'Note'
+        'fa-file-text-o'
+      else
+        ''
+      end
     h.content_tag :span, nil, class: icon_css
   end
 
@@ -79,8 +80,11 @@ def render_title
   # but this may change if we add activities whose action is an irregular
   # verb.
   def verb
-    if activity.action == 'destroy'
+    case activity.action
+    when 'destroy'
       'deleted'
+    when 'state_change'
+      'updated'
     else
       activity.action.sub(/e?\z/, 'ed')
     end
@@ -111,7 +115,7 @@ def partial_paths
   end
 
   def render_partial
-    locals = {activity: activity, presenter: self}
+    locals = { activity: activity, presenter: self }
     locals[trackable_name] = activity.trackable
     render partial_path, locals
   end
@@ -124,10 +128,11 @@ def trackable_name
   end
 
   def trackable_title
-    @title ||= if activity.trackable.respond_to?(:title) && activity.trackable.title?
-                 activity.trackable.title
-               elsif activity.trackable.respond_to?(:label) && activity.trackable.label?
-                 activity.trackable.label
-               end
+    @title ||=
+    if activity.trackable.respond_to?(:title) && activity.trackable.title?
+      activity.trackable.title
+    elsif activity.trackable.respond_to?(:label) && activity.trackable.label?
+      activity.trackable.label
+    end
   end
 end

app/views/issues/activities/_issue.html.erb

@@ -1,6 +1,6 @@
 <% if issue %>
   <% if issue.title? %>
-    <%= presenter.verb %> the <%= link_to issue.title, [current_project, issue] %> issue.
+    <%= presenter.verb %> the <%= link_to issue.title, [current_project, issue] %> <%= activity.action == 'state_change' ? "issue's state to #{issue.state.humanize.downcase}" : 'issue' %>.
   <% else %>
     <%= presenter.verb %> <%= link_to "Issue ##{issue.id}", [current_project, issue] %>.
   <% end %>

spec/support/qa_shared_examples.rb

@@ -1,4 +1,6 @@
 shared_examples 'qa pages' do |item_type|
+  let(:model) { item_type.to_s.classify.constantize }
+  let(:states) { ['Draft', 'Published'] }
 
   describe 'index page', js: true do
     before do
@@ -48,45 +50,46 @@
       end
 
       it 'updates the list of records with the state' do
-        within '.dataTables_wrapper' do
+        states.each do |state|
+          record = model.where(state: 'ready_for_review').first
+          visit polymorphic_path([current_project, :qa, item_type.to_s.pluralize.to_sym])
+
           @original_row_count = page.all('tbody tr').count
           page.find('td.select-checkbox', match: :first).click
 
           click_button('State')
-          click_link('Published')
+          expect do
+            click_link state
+            # Wait for action to complete
+            page.find('.alert')
+          end.to have_enqueued_job(ActivityTrackingJob).with(job_params(record))
+
+          expect(current_path).to eq polymorphic_path([current_project, :qa, item_type.to_s.pluralize.to_sym])
+          expect(page.all('tbody tr').count).to eq(@original_row_count - 1)
+          expect(page).to have_selector('.alert-success', text: 'State updated successfully.')
+          expect(record.reload.state).to eq state.downcase.gsub(' ', '_')
         end
-
-        page.find('.alert')
-
-        expect(page.all('tbody tr').count).to eq(@original_row_count - 1)
-        expect(page).to have_selector('.alert-success', text: 'State updated successfully.')
       end
     end
   end
 
   describe 'show page' do
-    before do
-      visit polymorphic_path([current_project, :qa, records.first])
-    end
-
     it 'shows the record\'s content' do
+      visit polymorphic_path([current_project, :qa, records.first])
       expect(page).to have_content(records.first.title)
     end
 
-    it 'updates the state to draft' do
-      click_button 'Draft'
+    it 'updates the state' do
+      states.each do |state|
+        record = model.where(state: 'ready_for_review').first
+        visit polymorphic_path([current_project, :qa, record])
 
-      expect(current_path).to eq polymorphic_path([current_project, :qa, item_type.to_s.pluralize.to_sym])
-      expect(page).to have_selector('.alert-success', text: 'State updated successfully.')
-      expect(records.first.reload.draft?).to eq true
-    end
+        expect { click_button state }.to have_enqueued_job(ActivityTrackingJob).with(job_params(record))
 
-    it 'updates the state to published' do
-      click_button 'Published'
-
-      expect(current_path).to eq polymorphic_path([current_project, :qa, item_type.to_s.pluralize.to_sym])
-      expect(page).to have_selector('.alert-success', text: 'State updated successfully.')
-      expect(records.first.reload.published?).to eq true
+        expect(current_path).to eq polymorphic_path([current_project, :qa, item_type.to_s.pluralize.to_sym])
+        expect(page).to have_selector('.alert-success', text: 'State updated successfully.')
+        expect(record.reload.state).to eq state.downcase.gsub(' ', '_')
+      end
     end
   end
 
@@ -116,4 +119,14 @@
       expect(current_path).to eq polymorphic_path([current_project, :qa, records.first])
     end
   end
+
+  def job_params(record)
+    {
+      action: 'state_change',
+      project_id: current_project.id,
+      trackable_id: record.id,
+      trackable_type: record.class.to_s,
+      user_id: @logged_in_as.id
+    }
+  end
 end