Skip to content

Commit

Permalink
refactor: change extension granularity from weeks to days
Browse files Browse the repository at this point in the history
  • Loading branch information
ublefo committed Apr 30, 2024
1 parent fa701a9 commit 62b9594
Show file tree
Hide file tree
Showing 12 changed files with 59 additions and 60 deletions.
2 changes: 1 addition & 1 deletion app/api/entities/unit_entity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ def is_admin_staff?(my_role)
expose :enable_sync_timetable, unless: :summary_only, if: lambda { |unit, options| is_staff?(options[:my_role]) }
expose :draft_task_definition_id, unless: :summary_only, if: lambda { |unit, options| is_staff?(options[:my_role]) }
expose :allow_student_extension_requests, unless: :summary_only
expose :extension_weeks_on_resubmit_request, unless: :summary_only, if: lambda { |unit, options| is_staff?(options[:my_role]) }
expose :extension_days_on_resubmit_request, unless: :summary_only, if: lambda { |unit, options| is_staff?(options[:my_role]) }
expose :allow_student_change_tutorial, unless: :summary_only

expose :learning_outcomes, using: LearningOutcomeEntity, as: :ilos, unless: :summary_only
Expand Down
10 changes: 5 additions & 5 deletions app/api/extension_comments_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ class ExtensionCommentsApi < Grape::API
desc 'Request an extension for a task'
params do
requires :comment, type: String, desc: 'The details of the request'
requires :weeks_requested, type: Integer, desc: 'The details of the request'
requires :days_requested, type: Integer, desc: 'The details of the request'
end
post '/projects/:project_id/task_def_id/:task_definition_id/request_extension' do
project = Project.find(params[:project_id])
Expand All @@ -19,11 +19,11 @@ class ExtensionCommentsApi < Grape::API
error!({ error: 'Not authorised to request an extension for this task' }, 403)
end

error!({ error: 'Extension weeks can not be 0.' }, 403) if params[:weeks_requested] == 0
error!({ error: 'Extension days can not be 0.' }, 403) if params[:days_requested] == 0

max_duration = task.weeks_can_extend
duration = params[:weeks_requested]
duration = max_duration unless params[:weeks_requested] <= max_duration
max_duration = task.days_can_extend
duration = params[:days_requested]
duration = max_duration unless params[:days_requested] <= max_duration

error!({ error: 'Extensions cannot be granted beyond task deadline.' }, 403) if duration <= 0

Expand Down
8 changes: 4 additions & 4 deletions app/api/units_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class UnitsApi < Grape::API
optional :portfolio_auto_generation_date, type: Date, desc: 'Indicates a date where student portfolio will automatically compile'
optional :allow_student_extension_requests, type: Boolean, desc: 'Can turn on/off student extension requests'
optional :allow_student_change_tutorial, type: Boolean, desc: 'Can turn on/off student ability to change tutorials'
optional :extension_weeks_on_resubmit_request, type: Integer, desc: 'Determines the number of weeks extension on a resubmit request'
optional :extension_days_on_resubmit_request, type: Integer, desc: 'Determines the number of days extension on a resubmit request'
optional :overseer_image_id, type: Integer, desc: 'The id of the docker image used with '
optional :assessment_enabled, type: Boolean

Expand Down Expand Up @@ -114,7 +114,7 @@ class UnitsApi < Grape::API
:draft_task_definition_id,
:portfolio_auto_generation_date,
:allow_student_extension_requests,
:extension_weeks_on_resubmit_request,
:extension_days_on_resubmit_request,
:allow_student_change_tutorial,
:overseer_image_id,
:assessment_enabled)
Expand Down Expand Up @@ -157,7 +157,7 @@ class UnitsApi < Grape::API
optional :enable_sync_timetable, type: Boolean, desc: 'Sync to timetable automatically if supported by deployment', default: true
optional :enable_sync_enrolments, type: Boolean, desc: 'Sync student enrolments automatically if supported by deployment', default: true
optional :allow_student_extension_requests, type: Boolean, desc: 'Can turn on/off student extension requests', default: true
optional :extension_weeks_on_resubmit_request, type: Integer, desc: 'Determines the number of weeks extension on a resubmit request', default: 1
optional :extension_days_on_resubmit_request, type: Integer, desc: 'Determines the number of weeks extension on a resubmit request', default: 1
optional :portfolio_auto_generation_date, type: Date, desc: 'Indicates a date where student portfolio will automatically compile'
optional :allow_student_change_tutorial, type: Boolean, desc: 'Can turn on/off student ability to change tutorials', default: true

Expand Down Expand Up @@ -185,7 +185,7 @@ class UnitsApi < Grape::API
:enable_sync_timetable,
:enable_sync_enrolments,
:allow_student_extension_requests,
:extension_weeks_on_resubmit_request,
:extension_days_on_resubmit_request,
:portfolio_auto_generation_date,
:allow_student_change_tutorial,
)
Expand Down
4 changes: 2 additions & 2 deletions app/models/comments/extension_comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ def serialize(user)
json[:granted] = extension_granted
json[:assessed] = date_extension_assessed.present?
json[:date_assessed] = date_extension_assessed
json[:weeks_requested] = extension_weeks
json[:days_requested] = extension_days
json[:extension_response] = extension_response
json[:task_status] = task.status
json
Expand Down Expand Up @@ -38,7 +38,7 @@ def assess_extension(user, granted, automatic = false)
self.extension_granted = granted && self.task.can_apply_for_extension?

if self.extension_granted
self.task.grant_extension(user, extension_weeks)
self.task.grant_extension(user, extension_days)
if automatic
self.extension_response = "Time extended to #{self.task.due_date.strftime('%a %b %e')}"
else
Expand Down
29 changes: 14 additions & 15 deletions app/models/task.rb
Original file line number Diff line number Diff line change
Expand Up @@ -219,9 +219,9 @@ def processing_pdf?
end
end

# Get the raw extension date - with extensions representing weeks
# Get the raw extension date - with extensions representing days
def raw_extension_date
target_date + extensions.weeks
target_date + extensions.days
end

# Get the adjusted extension date, which ensures it is never past the due date
Expand All @@ -243,22 +243,22 @@ def tutor
end

# Applying for an extension will create an extension comment
def apply_for_extension(user, text, weeks)
def apply_for_extension(user, text, days)
extension = ExtensionComment.create
extension.task = self
extension.extension_weeks = weeks
extension.extension_days = days
extension.user = user
extension.content_type = :extension
extension.comment = text
if weeks <= weeks_can_extend
if days <= days_can_extend
extension.recipient = tutor
else
extension.recipient = unit.main_convenor_user
end
extension.save!

# Check and apply either auto extensions, or those requested by staff
if (unit.auto_apply_extension_before_deadline && weeks <= weeks_can_extend) || role_for(user) == :tutor
if (unit.auto_apply_extension_before_deadline && days <= days_can_extend) || role_for(user) == :tutor
if role_for(user) == :tutor
extension.assess_extension user, true, true
else
Expand All @@ -269,20 +269,19 @@ def apply_for_extension(user, text, weeks)
extension
end

def weeks_can_extend
def days_can_extend
deadline = task_definition.due_date.to_date
current_due = raw_extension_date.to_date

diff = deadline - current_due
(diff.to_f / 7).ceil
end

# Add an extension to the task
def grant_extension(by_user, weeks)
weeks_to_extend = [weeks, weeks_can_extend].min
return false unless weeks_to_extend > 0
def grant_extension(by_user, days)
days_to_extend = [days, days_can_extend].min
return false unless days_to_extend > 0

if update(extensions: self.extensions + weeks_to_extend)
if update(extensions: self.extensions + days_to_extend)
# Was the task previously assessed as time exceeded? ... with the extension should this change?
if self.task_status == TaskStatus.time_exceeded && submitted_before_due?
update(task_status: TaskStatus.ready_for_feedback)
Expand Down Expand Up @@ -530,11 +529,11 @@ def assess(task_status, assessor, assess_date = Time.zone.now)
else
self.completion_date = nil

# Grant an extension on fix if due date is within 1 week
# Grant an extension on fix if due date is within 7 days
case task_status
when TaskStatus.fix_and_resubmit, TaskStatus.discuss, TaskStatus.demonstrate
if to_same_day_anywhere_on_earth(due_date) < Time.zone.now + 7.days && can_apply_for_extension? && unit.extension_weeks_on_resubmit_request > 0
grant_extension(assessor, unit.extension_weeks_on_resubmit_request)
if to_same_day_anywhere_on_earth(due_date) < Time.zone.now + 7.days && can_apply_for_extension? && unit.extension_days_on_resubmit_request > 0
grant_extension(assessor, unit.extension_days_on_resubmit_request)
end
end
end
Expand Down
2 changes: 1 addition & 1 deletion app/models/unit.rb
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ def role_for(user)
validates :end_date, presence: true

validates :code, uniqueness: { scope: :teaching_period, message: "%{value} already exists in this teaching period" }, if: :has_teaching_period?
validates :extension_weeks_on_resubmit_request, :numericality => { :greater_than_or_equal_to => 0 }
validates :extension_days_on_resubmit_request, :numericality => { :greater_than_or_equal_to => 0 }

validate :validate_end_date_after_start_date
validate :ensure_teaching_period_dates_match, if: :has_teaching_period?
Expand Down
34 changes: 17 additions & 17 deletions test/api/comments/extension_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ def test_extension_application
})
td.save!
data_to_post = {
weeks_requested: '1',
days_requested: '7',
comment: "I need a lot of help"
}

Expand All @@ -44,26 +44,26 @@ def test_extension_application
post_json "/api/projects/#{project.id}/task_def_id/#{td.id}/request_extension", data_to_post
response = last_response_body
assert_equal 201, last_response.status
assert response["weeks_requested"] == 1, "Error: Deadline less than a week, requested weeks should be 1, found #{response["weeks_requested"]}."
assert response["days_requested"] == 1, "Error: Requested days should be 1, found #{response["days_requested"]}."

# Request a 2 week extension on the day
td.due_date = Time.zone.now + 2.weeks
td.save!
data_to_post["weeks_requested"] = '2'
data_to_post["days_requested"] = '14'

# Add auth_token and username to header
add_auth_header_for(user: user)

post_json "/api/projects/#{project.id}/task_def_id/#{td.id}/request_extension", data_to_post
response = last_response_body
assert_equal 201, last_response.status
assert response["weeks_requested"] == 2, "Error: Weeks requested weeks should be 2, found #{response["weeks_requested"]}."
assert response["days_requested"] == 14, "Error: Days requested should be 14, found #{response["days_requested"]}."

# Add auth_token and username to header
add_auth_header_for(user: user)

# Ask for too long an extension
data_to_post["weeks_requested"] = '5'
data_to_post["days_requested"] = '35'
post_json "/api/projects/#{project.id}/task_def_id/#{td.id}/request_extension", data_to_post
response = last_response_body
assert_equal 403, last_response.status, "Error: Allowed too long of a request to be applied."
Expand All @@ -72,11 +72,11 @@ def test_extension_application
# Add auth_token and username to header
add_auth_header_for(user: user)

# Ask for 0 week extension
data_to_post["weeks_requested"] = '0'
# Ask for 0 day extension
data_to_post["days_requested"] = '0'
post_json "/api/projects/#{project.id}/task_def_id/#{td.id}/request_extension", data_to_post
response = last_response_body
assert_equal 403, last_response.status, "Error: Should not allow 0 week extension requests"
assert_equal 403, last_response.status, "Error: Should not allow 0 day extension requests"

td.destroy!
unit.destroy!
Expand Down Expand Up @@ -110,7 +110,7 @@ def test_extension_application

main_tutor = project.tutor_for(td)
data_to_post = {
weeks_requested: '1',
days_requested: '7',
comment: "I need a lot of help"
}

Expand All @@ -121,7 +121,7 @@ def test_extension_application
post_json "/api/projects/#{project.id}/task_def_id/#{td.id}/request_extension", data_to_post
response = last_response_body
assert_equal 201, last_response.status
assert response["weeks_requested"] == 1, "Error: Deadline less than a week, requested weeks should be 1, found #{response["weeks_requested"]}."
assert response["days_requested"] == 2, "Error: deadline is in 2 days, requested days should be 2, found #{response["days_requested"]}."

tc = TaskComment.find(response['id'])

Expand Down Expand Up @@ -168,7 +168,7 @@ def test_disallow_student_extensions

main_tutor = project.tutor_for(td)
data_to_post = {
weeks_requested: '1',
days_requested: '7',
comment: "I need a lot of help"
}

Expand All @@ -182,7 +182,7 @@ def test_disallow_student_extensions
post_json "/api/projects/#{project.id}/task_def_id/#{td.id}/request_extension", data_to_post
response = last_response_body
assert_equal 201, last_response.status
assert response["weeks_requested"] == 1, "Error: Deadline less than a week, requested weeks should be 1, found #{response["weeks_requested"]}."
assert response["days_requested"] == 2, "Error: Deadline less than 7 days, requested days should be 2, found #{response["days_requested"]}."

tc = ExtensionComment.find(response['id'])

Expand All @@ -195,7 +195,7 @@ def test_disallow_student_extensions
end

def test_extension_on_resubmit
unit = FactoryBot.create(:unit, extension_weeks_on_resubmit_request: 2)
unit = FactoryBot.create(:unit, extension_days_on_resubmit_request: 14)
td = TaskDefinition.new({
unit_id: unit.id,
tutorial_stream: unit.tutorial_streams.first,
Expand Down Expand Up @@ -231,14 +231,14 @@ def test_extension_on_resubmit
# Get the task... check it is ready for feedback
task = project.task_for_task_definition(td)
assert_equal TaskStatus.ready_for_feedback, task.task_status
assert_equal 3, task.weeks_can_extend
assert_equal 21, task.days_can_extend
assert task.can_apply_for_extension?

# Ask for resubmit
task.assess TaskStatus.fix_and_resubmit, tutor

# Now check that the 2 weeks was added
assert_equal 1, task.weeks_can_extend
assert_equal 7, task.days_can_extend

td.destroy
unit.destroy
Expand Down Expand Up @@ -272,7 +272,7 @@ def test_extension_in_inbox
td.save!

data_to_post = {
weeks_requested: '1',
days_requested: '7',
comment: "I need a lot of help"
}

Expand All @@ -284,7 +284,7 @@ def test_extension_in_inbox
post_json "/api/projects/#{project.id}/task_def_id/#{td.id}/request_extension", data_to_post
response = last_response_body
assert_equal 201, last_response.status
assert response["weeks_requested"] == 1, "Error: Deadline less than a week, requested weeks should be 1, found #{response["weeks_requested"]}."
assert response["days_requested"] == 7, "Error: requested weeks should be 1, found #{response["days_requested"]}."

unit.reload
task = unit.tasks.last
Expand Down
2 changes: 1 addition & 1 deletion test/api/groups_api_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ def test_group_submission_with_extensions

data_to_post = {
comment: 'I need more time',
weeks_requested: 2
days_requested: 14
}

project = group.projects.first
Expand Down
16 changes: 8 additions & 8 deletions test/api/tasks_api_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,13 +152,13 @@ def test_extension_reverts_time_exceeded
# Get the task... check it is now time exceeded
task = project.task_for_task_definition(td)
assert_equal TaskStatus.time_exceeded, task.task_status
assert_equal 2, task.weeks_can_extend
assert_equal 14, task.days_can_extend
assert task.can_apply_for_extension?
refute task.submitted_before_due?

data_to_post = {
comment: 'Help me!',
weeks_requested: 2
days_requested: 14
}

# Add username and auth_token to Header
Expand All @@ -181,9 +181,9 @@ def test_extension_reverts_time_exceeded

# After extension... no more extensions are possible
task.reload
assert_equal 0, task.weeks_can_extend
assert_equal 0, task.days_can_extend
refute task.can_apply_for_extension?
assert_equal 2, task.extensions
assert_equal 14, task.extensions
assert task.submitted_before_due?

assert_equal TaskStatus.ready_for_feedback, task.task_status
Expand Down Expand Up @@ -230,13 +230,13 @@ def test_extension_reverts_time_exceeded_auto_apply
# Get the task... check it is now time exceeded
task = project.task_for_task_definition(td)
assert_equal TaskStatus.time_exceeded, task.task_status
assert_equal 2, task.weeks_can_extend
assert_equal 14, task.days_can_extend
assert task.can_apply_for_extension?
refute task.submitted_before_due?

data_to_post = {
comment: 'Help me!',
weeks_requested: 2
days_requested: 14
}

# Add username and auth_token to Header
Expand All @@ -248,9 +248,9 @@ def test_extension_reverts_time_exceeded_auto_apply

# After extension... no more extensions are possible
task.reload
assert_equal 0, task.weeks_can_extend
assert_equal 0, task.days_can_extend
refute task.can_apply_for_extension?
assert_equal 2, task.extensions
assert_equal 14, task.extensions
assert task.submitted_before_due?

assert_equal TaskStatus.ready_for_feedback, task.task_status
Expand Down
2 changes: 1 addition & 1 deletion test/api/units_api_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ def test_unit_output()
assert_equal actual_unit['start_date'].to_date, expected_unit.start_date.to_date
assert_equal actual_unit['end_date'].to_date, expected_unit.end_date.to_date

keys = ["code", "id", "name", "main_convenor_id", "description", "active", "auto_apply_extension_before_deadline", "send_notifications", "enable_sync_enrolments", "enable_sync_timetable", "draft_task_definition_id", "allow_student_extension_requests", "extension_weeks_on_resubmit_request", "allow_student_change_tutorial"]
keys = ["code", "id", "name", "main_convenor_id", "description", "active", "auto_apply_extension_before_deadline", "send_notifications", "enable_sync_enrolments", "enable_sync_timetable", "draft_task_definition_id", "allow_student_extension_requests", "extension_days_on_resubmit_request", "allow_student_change_tutorial"]

assert actual_unit.key?("my_role"), actual_unit.inspect
assert_equal expected_unit.role_for(expected_unit.main_convenor_user).name, actual_unit["my_role"]
Expand Down
Loading

0 comments on commit 62b9594

Please sign in to comment.