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

[BUG FIX] [MER-3565] 500 error when launching assessments from a page link #5003

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/build.yml
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ jobs:
cp oli.example.env oli.env

- name: 🗄 Start test database
run: docker-compose up -d postgres
run: docker compose up -d postgres

- name: 💾 Restore the deps cache
id: mix-deps-cache
Expand Down
1 change: 1 addition & 0 deletions assets/src/components/activities/DeliveryElement.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ export interface ActivityContext {
renderPointMarkers: boolean;
isAnnotationLevel: boolean;
variables: any;
pageLinkParams: any;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ export const DeliveryElementProvider: React.FC<DeliveryElementProps<any>> = (pro
resourceAttemptGuid: props.context.pageAttemptGuid,
renderPointMarkers: props.context.renderPointMarkers,
isAnnotationLevel: props.context.isAnnotationLevel,
pageLinkParams: props.context.pageLinkParams,
});

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ export const MockDiscussionDeliveryProvider: React.FC<{
renderPointMarkers: false,
isAnnotationLevel: false,
variables: {},
pageLinkParams: {},
}}
onSaveActivity={nullHandler}
onSavePart={nullHandler}
Expand Down
1 change: 1 addition & 0 deletions assets/src/data/content/writers/context.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export interface WriterContext {
};
renderPointMarkers?: boolean;
isAnnotationLevel?: boolean;
pageLinkParams?: any;
}

export const defaultWriterContext = (params: Partial<WriterContext> = {}): WriterContext =>
Expand Down
5 changes: 4 additions & 1 deletion assets/src/data/content/writers/html.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,10 @@ export class HtmlParser implements WriterImpl {
let internalHref = href;
if (context.sectionSlug) {
const revisionSlug = href.replace(/^\/course\/link\//, '');
internalHref = `/sections/${context.sectionSlug}/page/${revisionSlug}`;
const params = new URLSearchParams(context.pageLinkParams);
const queryString = params.toString();

internalHref = `/sections/${context.sectionSlug}/lesson/${revisionSlug}?${queryString}`;
} else {
internalHref = '#';
}
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 @@ -33,6 +33,7 @@ describe('check all that apply delivery', () => {
renderPointMarkers: false,
isAnnotationLevel: false,
variables: {},
pageLinkParams: {},
},
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 @@ -34,6 +34,7 @@ describe('multiple choice delivery', () => {
renderPointMarkers: false,
isAnnotationLevel: false,
variables: {},
pageLinkParams: {},
},
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 @@ -33,6 +33,7 @@ describe('ordering delivery', () => {
renderPointMarkers: false,
isAnnotationLevel: false,
variables: {},
pageLinkParams: {},
},
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 @@ -33,6 +33,7 @@ describe('multiple choice delivery', () => {
renderPointMarkers: false,
isAnnotationLevel: false,
variables: {},
pageLinkParams: {},
},
preview: false,
};
Expand Down
15 changes: 13 additions & 2 deletions assets/test/writer/writer_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,12 +99,23 @@ describe('parser', () => {
});

it('renders internal link with context', () => {
render(parse(exampleContent, defaultWriterContext({ sectionSlug: 'some_section' })));
render(
parse(
exampleContent,
defaultWriterContext({
sectionSlug: 'some_section',
pageLinkParams: {
request_path: '/path/to/previous/page',
},
}),
),
);
expect(
screen.getByText((content, element) => {
return (
element?.tagName.toLowerCase() === 'a' &&
element.getAttribute('href') === '/sections/some_section/page/page_two' &&
element.getAttribute('href') ===
'/sections/some_section/lesson/page_two?request_path=%2Fpath%2Fto%2Fprevious%2Fpage' &&
content === 'Page Two'
);
}),
Expand Down
3 changes: 2 additions & 1 deletion lib/oli/rendering/activity/html.ex
Original file line number Diff line number Diff line change
Expand Up @@ -194,7 +194,8 @@ defmodule Oli.Rendering.Activity.Html do
end,
renderPointMarkers: render_opts.render_point_markers,
isAnnotationLevel: true,
variables: variables
variables: variables,
pageLinkParams: Enum.into(context.page_link_params, %{})
}
|> Poison.encode!()
|> HtmlEntities.encode()
Expand Down
7 changes: 5 additions & 2 deletions lib/oli/rendering/content/html.ex
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,9 @@ defmodule Oli.Rendering.Content.Html do

Important: any changes to this file must be replicated in writers/html.ts for activity rendering.
"""

use OliWeb, :verified_routes

import Oli.Utils
import Oli.Rendering.Utils

Expand Down Expand Up @@ -710,7 +713,7 @@ defmodule Oli.Rendering.Content.Html do
end

defp internal_link(
%Context{section_slug: section_slug, mode: mode, project_slug: project_slug},
%Context{section_slug: section_slug, mode: mode, project_slug: project_slug} = context,
next,
href,
opts \\ []
Expand All @@ -733,7 +736,7 @@ defmodule Oli.Rendering.Content.Html do
"/sections/#{section_slug}/preview/page/#{revision_slug_from_course_link(href)}"

_ ->
"/sections/#{section_slug}/page/#{revision_slug_from_course_link(href)}"
~p"/sections/#{section_slug}/lesson/#{revision_slug_from_course_link(href)}?#{context.page_link_params}"
end
end

Expand Down
3 changes: 2 additions & 1 deletion lib/oli/rendering/context.ex
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,6 @@ defmodule Oli.Rendering.Context do
learning_language: nil,
effective_settings: nil,
is_liveview: false,
is_annotation_level: true
is_annotation_level: true,
page_link_params: []
end
30 changes: 29 additions & 1 deletion lib/oli_web/live/delivery/student/utils.ex
Original file line number Diff line number Diff line change
Expand Up @@ -429,7 +429,14 @@ defmodule OliWeb.Delivery.Student.Utils do
# to apparently not be used by the page template:
# project_slug: base_project_slug,
# submitted_surveys: submitted_surveys,
resource_attempt: hd(page_context.resource_attempts)
resource_attempt: hd(page_context.resource_attempts),
page_link_params:
build_page_link_params(
assigns.section.slug,
assigns.page_context.page,
assigns.request_path,
assigns.selected_view
)
}

attempt_content = get_attempt_content(page_context)
Expand Down Expand Up @@ -702,4 +709,25 @@ defmodule OliWeb.Delivery.Student.Utils do
</div>
"""
end

def coalesce(first, second) do
case {first, second} do
{nil, nil} -> nil
{nil, s} -> s
{f, _s} -> f
end
end

defp build_page_link_params(section_slug, page, request_path, selected_view) do
current_page_path =
lesson_live_path(section_slug, page.slug,
request_path: request_path,
selected_view: selected_view
)

[
request_path: current_page_path,
selected_view: selected_view
]
end
end
2 changes: 1 addition & 1 deletion test/oli/rendering/content/html_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ defmodule Oli.Content.Content.HtmlTest do
"<p data-point-marker=\"2652513352\">The American colonials proclaimed &quot;no taxation without representation"

assert rendered_html_string =~
"<a class=\"internal-link\" href=\"/sections/some_section/page/page_two\">Page Two</a>"
"<a class=\"internal-link\" href=\"/sections/some_section/lesson/page_two\">Page Two</a>"

assert rendered_html_string =~
"<a class=\"external-link\" href=\"https://en.wikipedia.org/wiki/Stamp_Act_Congress\" target=\"_blank\" rel=\"noreferrer\">Stamp Act Congress</a>"
Expand Down
Loading