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

QA: State change activity #1134

Merged
merged 14 commits into from
May 10, 2023
4 changes: 4 additions & 0 deletions app/controllers/concerns/activity_tracking.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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_updated_state(trackable, user: current_user, project: current_project)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't love the name here, and looking at the presenter further down, what'd be the point of splitting the methods, and the action if we're not going to differentiate the view?

It seems to me that what we care about it's not the updating on an attribute, but the change in the QA state. Naming convention around QA state, and displaying a related message would be more useful

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the name to track_state_change.
We are differentiating the view here

track_activity(trackable, :update_state, user, project)
end
end
6 changes: 5 additions & 1 deletion app/controllers/qa/issues_controller.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
class QA::IssuesController < AuthenticatedController
include ActivityTracking
include LiquidEnabledResource
include ProjectScoped

Expand All @@ -20,10 +21,13 @@ def edit
end

def update
@issues = current_project.issues.ready_for_review.where(id: params[:ids])
@issues = current_project.issues.where(id: params[:ids])

respond_to do |format|
if @issues.update_all(state: @state, updated_at: Time.now)
@issues.each do |issue|
track_updated_state(issue)
end
format.html { redirect_to project_qa_issues_path(current_project), notice: 'State updated successfully.' }
format.json { head :ok }
else
Expand Down
6 changes: 3 additions & 3 deletions app/models/activity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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 update update_state]

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 ------------------------------------------------------------
Expand All @@ -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

Expand Down
51 changes: 28 additions & 23 deletions app/presenters/activity_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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 'update_state'
'updated'
else
activity.action.sub(/e?\z/, 'ed')
end
Expand Down Expand Up @@ -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
Expand All @@ -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
2 changes: 1 addition & 1 deletion app/views/issues/activities/_issue.html.erb
Original file line number Diff line number Diff line change
@@ -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 == 'update_state' ? "issue's state to #{issue.state.humanize.downcase}" : 'issue' %>.
<% else %>
<%= presenter.verb %> <%= link_to "Issue ##{issue.id}", [current_project, issue] %>.
<% end %>
Expand Down
59 changes: 36 additions & 23 deletions spec/support/qa_shared_examples.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -116,4 +119,14 @@
expect(current_path).to eq polymorphic_path([current_project, :qa, records.first])
end
end

def job_params(record)
{
action: 'update_state',
project_id: current_project.id,
trackable_id: record.id,
trackable_type: record.class.to_s,
user_id: @logged_in_as.id
}
end
end