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.

Sign up for GitHub

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

Jump to bottom

Application timestamp logs assigning and unassigning workshops #26583

Merged
merged 3 commits into from Jan 16, 2019
Merged

Application timestamp logs assigning and unassigning workshops #26583

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
99fea93
log changes to summer ws and fit weekend
Jan 11, 2019
feb2ebb
add tests
Jan 11, 2019
0ddf1eb
add test and fix bug
Jan 15, 2019
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
1 change: 1 addition & 0 deletions dashboard/app/controllers/api/v1/pd/application/facilitator_applications_controller.rb
View file
Open in desktop
Expand Up @@ -13,6 +13,7 @@ def new_form

def on_successful_create
@application.queue_email :confirmation, deliver_now: true
@application.update_status_timestamp_change_log(current_user)
end
end
end
10 changes: 10 additions & 0 deletions dashboard/app/controllers/api/v1/pd/applications_controller.rb
View file
Open in desktop
Expand Up @@ -172,6 +172,14 @@ def update
status_changed = true
end

if application_data[:fit_workshop_id] != @application.try(:fit_workshop_id)
fit_workshop_changed = true
end

if application_data[:pd_workshop_id] != @application.pd_workshop_id
summer_workshop_changed = true
end

if application_data[:response_scores]
application_data[:response_scores] = JSON.parse(application_data[:response_scores]).transform_keys {|x| x.to_s.underscore}.to_json
end
Expand Down Expand Up @@ -209,6 +217,8 @@ def update
end

@application.update_status_timestamp_change_log(current_user) if status_changed
@application.log_fit_workshop_change(current_user) if fit_workshop_changed
@application.log_summer_workshop_change(current_user) if summer_workshop_changed

render json: @application, serializer: ApplicationSerializer
end
Expand Down
4 changes: 2 additions & 2 deletions dashboard/app/models/pd/application/application_base.rb
View file
Open in desktop
Expand Up @@ -393,9 +393,9 @@ def sanitize_status_timestamp_change_log
# Record when the status changes and who changed it
# Ideally we'd implement this as an after_save action, but since we want the current
# user to be included, this needs to be explicitly passed in in the controller
def update_status_timestamp_change_log(user)
def update_status_timestamp_change_log(user, title = status)
log_entry = {
title: status,
title: title,
changing_user_id: user.try(:id),
changing_user_name: user.try(:name) || user.try(:email),
time: Time.zone.now
Expand Down
4 changes: 4 additions & 0 deletions dashboard/app/models/pd/application/facilitator_application_base.rb
View file
Open in desktop
Expand Up @@ -96,6 +96,10 @@ def fit_workshop_date_and_location
fit_workshop.try(&:date_and_location_name)
end

def log_fit_workshop_change(user)
update_status_timestamp_change_log(user, "Fit Workshop: #{fit_workshop_id ? fit_workshop_date_and_location : 'Unassigned'}")
end

def registered_fit_workshop?
# inspect the cached fit_workshop.enrollments rather than querying the DB
fit_workshop.enrollments.any? {|e| e.user_id == user.id} if fit_workshop_id
Expand Down
4 changes: 4 additions & 0 deletions dashboard/app/models/pd/application/workshop_autoenrolled_application.rb
View file
Open in desktop
Expand Up @@ -95,6 +95,10 @@ def workshop_date_and_location
workshop.try(&:date_and_location_name)
end

def log_summer_workshop_change(user)
update_status_timestamp_change_log(user, "Summer Workshop: #{pd_workshop_id ? workshop_date_and_location : 'Unassigned'}")
end

# override
def lock!
return if locked?
Expand Down
80 changes: 77 additions & 3 deletions dashboard/test/controllers/api/v1/pd/applications_controller_test.rb
View file
Open in desktop
Expand Up @@ -39,7 +39,7 @@ class ApplicationsControllerTest < ::ActionController::TestCase
@csd_teacher_application = create TEACHER_APPLICATION_FACTORY, course: 'csd'
@csd_teacher_application_with_partner = create TEACHER_APPLICATION_FACTORY, course: 'csd', regional_partner: @regional_partner
@csp_teacher_application = create TEACHER_APPLICATION_FACTORY, course: 'csp'
@csp_facilitator_application = create FACILITATOR_APPLICATION_FACTORY, course: 'csp'
@csp_facilitator_application = create FACILITATOR_APPLICATION_FACTORY, course: 'csp', regional_partner: @regional_partner

@serializing_teacher = create(:teacher,
email: 'minerva@hogwarts.edu',
Expand All @@ -52,6 +52,9 @@ class ApplicationsControllerTest < ::ActionController::TestCase
)
)

@summer_workshop = create :pd_workshop, :local_summer_workshop, num_sessions: 5, sessions_from: Date.new(2019, 6, 1), processed_location: {city: 'Orchard Park', state: 'NY'}.to_json
@fit_workshop = create :pd_workshop, :fit, num_sessions: 3, sessions_from: Date.new(2019, 6, 1), processed_location: {city: 'Orchard Park', state: 'NY'}.to_json

@markdown = Redcarpet::Markdown.new(Redcarpet::Render::StripDown)
end

Expand Down Expand Up @@ -298,7 +301,7 @@ class ApplicationsControllerTest < ::ActionController::TestCase
assert_equal({regional_partner_name: 'Yes'}, application.response_scores_hash)
end

test 'update appends to the status changed log if status is changed' do
test 'update appends to the timestamp log if status is changed' do
sign_in @program_manager

assert_equal [], @csd_teacher_application_with_partner.sanitize_status_timestamp_change_log
Expand All @@ -316,7 +319,7 @@ class ApplicationsControllerTest < ::ActionController::TestCase
], @csd_teacher_application_with_partner.sanitize_status_timestamp_change_log
end

test 'update does not append to the status changed log if status is unchanged' do
test 'update does not append to the timestamp log if status is unchanged' do
sign_in @program_manager
@csd_teacher_application_with_partner.update(status_timestamp_change_log: '[]')
@csd_teacher_application_with_partner.reload
Expand All @@ -329,6 +332,77 @@ class ApplicationsControllerTest < ::ActionController::TestCase
assert_equal [], @csd_teacher_application_with_partner.sanitize_status_timestamp_change_log
end

test 'update appends to the timestamp log if fit workshop is changed' do
sign_in @program_manager
@csp_facilitator_application.update(status_timestamp_change_log: '[]')
Copy link
Contributor

Choose a reason for hiding this comment

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

why we choose @csp_facilitator_application instead of @csd_facilitator_application, are they different?

Copy link
Author

Choose a reason for hiding this comment

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

I only created a CSP facilitator application. CSF, CSD, and CSP facilitator applications all get assigned to FiT Workshops, but only CSD and CSP applications are assigned to a 5-day summer workshop. The differences between CSD and CSP are that they have a different course and are shown slightly different questions (stored in the form_data

@csp_facilitator_application.reload

assert_equal [], @csp_facilitator_application.sanitize_status_timestamp_change_log

post :update, params: {id: @csp_facilitator_application.id, application: {fit_workshop_id: @fit_workshop.id, status: @csp_facilitator_application.status}}
@csp_facilitator_application.reload

assert_equal [
{
title: "Fit Workshop: #{@csp_facilitator_application.fit_workshop_date_and_location}",
Copy link
Contributor

Choose a reason for hiding this comment

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

Could the fit_workshop_date_and_location be "Unassigned"? Same for workshop_date_and_location

Copy link
Author

Choose a reason for hiding this comment

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

ooh, actually, when I log this, I should be logging "Unassigned" based on whether or not the workshop is assigned (if application.fit_workshop_id exists), not based on the value of application.fit_workshop_data_and_location, because if the city name is missing in the workshop, for example, that will be nil but the workshop is assigned.

But to answer your question, currently those methods don't return "Unassigned", they either return the date and location string or nil.

changing_user_id: @program_manager.id,
changing_user_name: @program_manager.name,
time: Time.zone.now
}
], @csp_facilitator_application.sanitize_status_timestamp_change_log
end

test 'update appends to the timestamp log if summer workshop is changed' do
sign_in @program_manager
@csp_facilitator_application.update(status_timestamp_change_log: '[]')
@csp_facilitator_application.reload

assert_equal [], @csp_facilitator_application.sanitize_status_timestamp_change_log

post :update, params: {id: @csp_facilitator_application.id, application: {pd_workshop_id: @summer_workshop.id, status: @csp_facilitator_application.status}}
@csp_facilitator_application.reload

assert_equal [
{
title: "Summer Workshop: #{@csp_facilitator_application.workshop_date_and_location}",
changing_user_id: @program_manager.id,
changing_user_name: @program_manager.name,
time: Time.zone.now
}
], @csp_facilitator_application.sanitize_status_timestamp_change_log
end

test 'update does not append to the timestamp log if fit and summer workshop are not changed' do
sign_in @program_manager
@csp_facilitator_application.update(status_timestamp_change_log: '[]')
@csp_facilitator_application.reload

assert_equal [], @csp_facilitator_application.sanitize_status_timestamp_change_log

post :update, params: {id: @csp_facilitator_application.id, application: {fit_workshop_id: @fit_workshop.id, pd_workshop_id: @summer_workshop.id, status: @csp_facilitator_application.status}}
@csp_facilitator_application.reload

expected_log = [
{
title: "Fit Workshop: #{@csp_facilitator_application.fit_workshop_date_and_location}",
changing_user_id: @program_manager.id,
changing_user_name: @program_manager.name,
time: Time.zone.now
}, {
title: "Summer Workshop: #{@csp_facilitator_application.workshop_date_and_location}",
changing_user_id: @program_manager.id,
changing_user_name: @program_manager.name,
time: Time.zone.now
}
]

assert_equal expected_log, @csp_facilitator_application.sanitize_status_timestamp_change_log

post :update, params: {id: @csp_facilitator_application.id, application: {fit_workshop_id: @fit_workshop.id, pd_workshop_id: @summer_workshop.id, status: @csp_facilitator_application.status}}

assert_equal expected_log, @csp_facilitator_application.sanitize_status_timestamp_change_log
end

test 'workshop admins can lock and unlock applications' do
sign_in @workshop_admin
put :update, params: {id: @csf_facilitator_application_no_partner, application: {status: 'accepted', locked: 'true'}}
Expand Down
1 change: 1 addition & 0 deletions dashboard/test/factories/pd_factories.rb
View file
Open in desktop
Expand Up @@ -5,6 +5,7 @@
association :organizer, factory: :workshop_organizer
funded false
on_map true
location_name 'Hogwarts School of Witchcraft and Wizardry'
course Pd::Workshop::COURSES.first
subject {Pd::Workshop::SUBJECTS[course].try(&:first)}
trait :teachercon do
Expand Down