Skip to content

Commit

Permalink
[FEATURE] [MER-3529] Hints should be functional on scored pages (#5315)
Browse files Browse the repository at this point in the history
* [MER-3529] Create allow_hints field

* [MER-3529] Add allow hints column to assessment settings table

* [MER-3529] Logic to show hints in pages

* [MER-3529] Test

* [MER-3529] Fix JS tests
  • Loading branch information
simonchoxx authored Dec 23, 2024
1 parent e2cccb0 commit a523a2b
Show file tree
Hide file tree
Showing 16 changed files with 112 additions and 28 deletions.
1 change: 1 addition & 0 deletions assets/src/components/activities/DeliveryElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ export interface ActivityContext {
isAnnotationLevel: boolean;
variables: any;
pageLinkParams: any;
allowHints: boolean;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,22 +24,22 @@ const shouldShow = (
graded: boolean,
surveyId: string | null,
shouldShow?: boolean,
allowHints?: boolean,
) => {
if (graded) return false;
if (!graded) return true;
if (surveyId !== null) return false;
if (shouldShow) return true;

return !isEvaluated(uiState) && !isSubmitted(uiState);
return !isEvaluated(uiState) && !isSubmitted(uiState) && allowHints;
};

const isRequestHintDisabled = (
uiState: ActivityDeliveryState,
hasMoreHints: boolean,
correct: boolean,
graded: boolean,
) => {
if (!hasMoreHints) return false;
if (isEvaluated(uiState) && graded) return true;
if (isEvaluated(uiState)) return true;
if (!correct) return false;

return isEvaluated(uiState) || isSubmitted(uiState);
Expand All @@ -57,14 +57,14 @@ const isRequestHintDisabled = (
export const HintsDeliveryConnected: React.FC<Props> = (props) => {
const { context, writerContext, onRequestHint, onResetActivity } =
useDeliveryElementContext<HasHints>();
const { graded, surveyId } = context;
const { graded, surveyId, allowHints } = context;
const uiState = useSelector((state: ActivityDeliveryState) => state);
const dispatch = useDispatch();

const correct = isCorrect(uiState.attemptState);
const shouldShowHint = shouldShow(uiState, graded, surveyId, props.shouldShow);
const shouldShowHint = shouldShow(uiState, graded, surveyId, props.shouldShow, allowHints);
const hasMoreHints = uiState.partState[props.partId]?.hasMoreHints || false;
const requestHintDisabled = isRequestHintDisabled(uiState, hasMoreHints, correct, graded);
const requestHintDisabled = isRequestHintDisabled(uiState, hasMoreHints, correct);

const onHint = () => {
if (isEvaluated(uiState)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ export const MockDiscussionDeliveryProvider: React.FC<{
isAnnotationLevel: false,
variables: {},
pageLinkParams: {},
allowHints: false,
}}
onSaveActivity={nullHandler}
onSavePart={nullHandler}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,6 @@ export const MultipleChoiceComponent: React.FC = () => {
<HintsDeliveryConnected
partId={castPartId(activityState.parts[0].partId)}
resetPartInputs={{ [activityState.parts[0].partId]: [] }}
shouldShow
/>
<EvaluationConnected />
</div>
Expand Down
1 change: 1 addition & 0 deletions assets/test/check_all_that_apply/cata_delivery_test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('check all that apply delivery', () => {
isAnnotationLevel: false,
variables: {},
pageLinkParams: {},
allowHints: false,
},
preview: false,
};
Expand Down
1 change: 1 addition & 0 deletions assets/test/multiple_choice/mc_delivery_test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ describe('multiple choice delivery', () => {
isAnnotationLevel: false,
variables: {},
pageLinkParams: {},
allowHints: false,
},
graded: false,
preview: false,
Expand Down
1 change: 1 addition & 0 deletions assets/test/ordering/ordering_delivery_test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('ordering delivery', () => {
isAnnotationLevel: false,
variables: {},
pageLinkParams: {},
allowHints: false,
},
preview: false,
};
Expand Down
1 change: 1 addition & 0 deletions assets/test/short_answer/short_answer_delivery_test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('multiple choice delivery', () => {
isAnnotationLevel: false,
variables: {},
pageLinkParams: {},
allowHints: false,
},
preview: false,
};
Expand Down
2 changes: 2 additions & 0 deletions lib/oli/delivery/sections/section_resource.ex
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ defmodule Oli.Delivery.Sections.SectionResource do
field :poster_image, :string, default: nil
field :objectives, :map, default: %{}
field :relates_to, {:array, :id}, default: []
field :allow_hints, :boolean, default: false
belongs_to :resource_type, Oli.Resources.ResourceType
belongs_to :revision, Oli.Activities.ActivityRegistration
belongs_to :activity_type, Oli.Resources.Revision
Expand Down Expand Up @@ -139,6 +140,7 @@ defmodule Oli.Delivery.Sections.SectionResource do
:poster_image,
:objectives,
:relates_to,
:allow_hints,
:resource_type_id,
:revision_id,
:activity_type_id
Expand Down
3 changes: 2 additions & 1 deletion lib/oli/delivery/settings.ex
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ defmodule Oli.Delivery.Settings do
feedback_scheduled_date:
combine_field(:feedback_scheduled_date, section_resource, student_exception),
collab_space_config: collab_space_config,
explanation_strategy: explanation_strategy
explanation_strategy: explanation_strategy,
allow_hints: section_resource.allow_hints
}
end

Expand Down
6 changes: 4 additions & 2 deletions lib/oli/delivery/settings/combined.ex
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ defmodule Oli.Delivery.Settings.Combined do
feedback_mode: :allow,
feedback_scheduled_date: nil,
collab_space_config: nil,
explanation_strategy: nil
explanation_strategy: nil,
allow_hints: false

@type t() :: %__MODULE__{
resource_id: integer(),
Expand All @@ -36,6 +37,7 @@ defmodule Oli.Delivery.Settings.Combined do
feedback_mode: :allow | :disallow | :scheduled,
feedback_scheduled_date: DateTime.t(),
collab_space_config: %Oli.Resources.Collaboration.CollabSpaceConfig{},
explanation_strategy: %Oli.Resources.ExplanationStrategy{}
explanation_strategy: %Oli.Resources.ExplanationStrategy{},
allow_hints: boolean()
}
end
3 changes: 2 additions & 1 deletion lib/oli/rendering/activity/html.ex
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,8 @@ defmodule Oli.Rendering.Activity.Html do
renderPointMarkers: render_opts.render_point_markers,
isAnnotationLevel: true,
variables: variables,
pageLinkParams: Enum.into(context.page_link_params, %{})
pageLinkParams: Enum.into(context.page_link_params, %{}),
allowHints: effective_settings && effective_settings.allow_hints
}
|> Poison.encode!()
|> HtmlEntities.encode()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -932,6 +932,9 @@ defmodule OliWeb.Sections.AssessmentSettings.SettingsTable do
{key, value} when key in ~w(start_date end_date) ->
FormatDateTime.datestring_to_utc_datetime(value, ctx)

{key, value} when key in ~w(allow_hints) ->
Utils.string_to_boolean(value)

{_, value} ->
value
end
Expand Down Expand Up @@ -968,7 +971,8 @@ defmodule OliWeb.Sections.AssessmentSettings.SettingsTable do
:feedback_mode,
:review_submission,
:exceptions_count,
:scoring_strategy_id
:scoring_strategy_id,
:allow_hints
],
@default_params.sort_by
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,102 +27,108 @@ defmodule OliWeb.Sections.AssessmentSettings.SettingsTableModel do
%ColumnSpec{
name: :name,
label: "ASSESSMENT",
render_fn: &__MODULE__.render_assessment_column/3,
render_fn: &render_assessment_column/3,
th_class: "!sticky left-20 bg-white z-10",
td_class: "sticky left-20 bg-white dark:bg-neutral-800 z-10 whitespace-nowrap",
tooltip: Tooltips.for(:name)
},
%ColumnSpec{
name: :available_date,
label: "AVAILABLE DATE",
render_fn: &__MODULE__.render_available_date_column/3,
render_fn: &render_available_date_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:available_date)
},
%ColumnSpec{
name: :due_date,
label: "DUE DATE",
render_fn: &__MODULE__.render_due_date_column/3,
render_fn: &render_due_date_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:due_date)
},
%ColumnSpec{
name: :max_attempts,
label: "# ATTEMPTS",
render_fn: &__MODULE__.render_attempts_column/3,
render_fn: &render_attempts_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:max_attempts)
},
%ColumnSpec{
name: :time_limit,
label: "TIME LIMIT",
render_fn: &__MODULE__.render_time_limit_column/3,
render_fn: &render_time_limit_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:time_limit)
},
%ColumnSpec{
name: :late_policy,
label: "LATE POLICY",
render_fn: &__MODULE__.render_late_policy_column/3,
render_fn: &render_late_policy_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:late_policy)
},
%ColumnSpec{
name: :scoring_strategy_id,
label: "SCORING",
render_fn: &__MODULE__.render_scoring_column/3,
render_fn: &render_scoring_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:scoring_strategy_id)
},
%ColumnSpec{
name: :grace_period,
label: "GRACE PERIOD",
render_fn: &__MODULE__.render_grace_period_column/3,
render_fn: &render_grace_period_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:grace_period)
},
%ColumnSpec{
name: :retake_mode,
label: "RETAKE MODE",
render_fn: &__MODULE__.render_retake_mode_column/3,
render_fn: &render_retake_mode_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:retake_mode)
},
%ColumnSpec{
name: :assessment_mode,
label: "PRESENTATION",
render_fn: &__MODULE__.render_assessment_mode_column/3,
render_fn: &render_assessment_mode_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:assessment_mode)
},
%ColumnSpec{
name: :feedback_mode,
label: "VIEW FEEDBACK",
render_fn: &__MODULE__.render_view_feedback_column/3,
render_fn: &render_view_feedback_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:feedback_mode)
},
%ColumnSpec{
name: :review_submission,
label: "VIEW ANSWERS",
render_fn: &__MODULE__.render_view_answers_column/3,
render_fn: &render_view_answers_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:review_submission)
},
%ColumnSpec{
name: :password,
label: "PASSWORD",
render_fn: &__MODULE__.render_password_column/3,
render_fn: &render_password_column/3,
th_class: "pt-3 whitespace-nowrap",
sortable: false,
tooltip: Tooltips.for(:password)
},
%ColumnSpec{
name: :exceptions_count,
label: "EXCEPTIONS",
render_fn: &__MODULE__.render_exceptions_column/3,
render_fn: &render_exceptions_column/3,
th_class: "whitespace-nowrap",
tooltip: Tooltips.for(:exceptions_count)
},
%ColumnSpec{
name: :allow_hints,
label: "ALLOW HINTS",
render_fn: &render_allow_hints_column/3,
th_class: "whitespace-nowrap"
}
]

Expand Down Expand Up @@ -437,6 +443,21 @@ defmodule OliWeb.Sections.AssessmentSettings.SettingsTableModel do
"""
end

def render_allow_hints_column(assigns, assessment, _) do
assigns =
Map.merge(assigns, %{
allow_hints: assessment.allow_hints,
id: assessment.resource_id
})

~H"""
<select class="torus-select pr-32" name={"allow_hints-#{@id}"}>
<option selected={@allow_hints} value="true">Allow</option>
<option selected={!@allow_hints} value="false">Disallow</option>
</select>
"""
end

defp value_from_datetime(nil, _ctx), do: nil

defp value_from_datetime(datetime, ctx) do
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
defmodule Oli.Repo.Migrations.AllowHintsScoredPages do
use Ecto.Migration

def change do
alter table(:section_resources) do
add :allow_hints, :boolean, default: false
end
end
end
Original file line number Diff line number Diff line change
Expand Up @@ -720,7 +720,7 @@ defmodule OliWeb.Sections.AssessmentSettings.SettingsLiveTest do
{:error, {:live_redirect, %{kind: :push, to: url}}} =
element(
view,
".instructor_dashboard_table tbody tr:first-of-type td:last-of-type a",
".instructor_dashboard_table tbody tr:first-of-type td:nth-last-of-type(2) a",
"1"
)
|> render_click()
Expand Down Expand Up @@ -1659,6 +1659,45 @@ defmodule OliWeb.Sections.AssessmentSettings.SettingsLiveTest do
## That is, 11 changes for each of the remaining resources (Page 1, Page 2, Page 4) and 1 done above.
assert length(changes) == 34
end

test "can change allow hints setting", %{
conn: conn,
section: section,
page_1: page_1
} do
{:ok, view, _html} = live(conn, live_view_overview_route(section.slug, "settings", "all"))

## Checks that there is no setting change for allow_hints
assert Settings.fetch_all_settings_changes() == []

## Changes the allow_hints setting to allow
view
|> form(~s{form[for="settings_table"]})
|> render_change(%{
"_target" => ["allow_hints-#{page_1.resource.id}"],
"allow_hints-#{page_1.resource.id}" => true
})

## Checks that there is a setting change for allow_hints
assert Settings.fetch_all_settings_changes() |> hd |> Map.get(:new_value) == "true"

## Changes the allow_hints setting back to disallow
view
|> form(~s{form[for="settings_table"]})
|> render_change(%{
"_target" => ["allow_hints-#{page_1.resource.id}"],
"allow_hints-#{page_1.resource.id}" => false
})

## Checks that now there are 2 setting changes for allow_hints (one for disallow and one for allow)
assert Settings.fetch_all_settings_changes() |> length() == 2

## Checks that the last setting change for allow_hints is disallow
assert Settings.fetch_all_settings_changes()
|> Enum.sort_by(& &1.id, :desc)
|> hd
|> Map.get(:new_value) == "false"
end
end

describe "student exceptions tab" do
Expand Down

0 comments on commit a523a2b

Please sign in to comment.