From fa7afea87f5cd42b28a97049caa0df9448516bcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Abell=C3=A1?= Date: Tue, 6 Aug 2024 18:58:26 -0300 Subject: [PATCH 1/5] [MER-3565] Fix page link error on activity rendering --- .../components/activities/DeliveryElement.ts | 1 + .../activities/DeliveryElementProvider.tsx | 1 + assets/src/data/content/writers/context.ts | 1 + assets/src/data/content/writers/html.tsx | 5 +++- lib/oli/rendering/activity/html.ex | 3 +- lib/oli/rendering/context.ex | 3 +- lib/oli_web/live/delivery/student/utils.ex | 30 ++++++++++++++++++- 7 files changed, 40 insertions(+), 4 deletions(-) diff --git a/assets/src/components/activities/DeliveryElement.ts b/assets/src/components/activities/DeliveryElement.ts index b547e7e2e26..d8530eafaec 100644 --- a/assets/src/components/activities/DeliveryElement.ts +++ b/assets/src/components/activities/DeliveryElement.ts @@ -64,6 +64,7 @@ export interface ActivityContext { renderPointMarkers: boolean; isAnnotationLevel: boolean; variables: any; + pageLinkParams: any; } /** diff --git a/assets/src/components/activities/DeliveryElementProvider.tsx b/assets/src/components/activities/DeliveryElementProvider.tsx index 6da6a77bcdf..7ca388dd3a3 100644 --- a/assets/src/components/activities/DeliveryElementProvider.tsx +++ b/assets/src/components/activities/DeliveryElementProvider.tsx @@ -37,6 +37,7 @@ export const DeliveryElementProvider: React.FC> = (pro resourceAttemptGuid: props.context.pageAttemptGuid, renderPointMarkers: props.context.renderPointMarkers, isAnnotationLevel: props.context.isAnnotationLevel, + pageLinkParams: props.context.pageLinkParams, }); return ( diff --git a/assets/src/data/content/writers/context.ts b/assets/src/data/content/writers/context.ts index cadbd24e03f..77dc86bbda5 100644 --- a/assets/src/data/content/writers/context.ts +++ b/assets/src/data/content/writers/context.ts @@ -28,6 +28,7 @@ export interface WriterContext { }; renderPointMarkers?: boolean; isAnnotationLevel?: boolean; + pageLinkParams?: any; } export const defaultWriterContext = (params: Partial = {}): WriterContext => diff --git a/assets/src/data/content/writers/html.tsx b/assets/src/data/content/writers/html.tsx index 5ba9ca7a9b1..7d8c758cdd6 100644 --- a/assets/src/data/content/writers/html.tsx +++ b/assets/src/data/content/writers/html.tsx @@ -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 = '#'; } diff --git a/lib/oli/rendering/activity/html.ex b/lib/oli/rendering/activity/html.ex index 270ef673f0c..a3ec40098a4 100644 --- a/lib/oli/rendering/activity/html.ex +++ b/lib/oli/rendering/activity/html.ex @@ -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() diff --git a/lib/oli/rendering/context.ex b/lib/oli/rendering/context.ex index 1f197123e1e..f09c50c888e 100644 --- a/lib/oli/rendering/context.ex +++ b/lib/oli/rendering/context.ex @@ -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 diff --git a/lib/oli_web/live/delivery/student/utils.ex b/lib/oli_web/live/delivery/student/utils.ex index 5b742352453..bee32266a62 100644 --- a/lib/oli_web/live/delivery/student/utils.ex +++ b/lib/oli_web/live/delivery/student/utils.ex @@ -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) @@ -702,4 +709,25 @@ defmodule OliWeb.Delivery.Student.Utils do """ 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 From f7ce31c048af6a2def5be612571cdfa1920b9c9e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Abell=C3=A1?= Date: Tue, 6 Aug 2024 18:58:52 -0300 Subject: [PATCH 2/5] [MER-3565] Fix page link error on page rendering --- lib/oli/rendering/content/html.ex | 7 +++++-- test/oli/rendering/content/html_test.exs | 2 +- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/oli/rendering/content/html.ex b/lib/oli/rendering/content/html.ex index bbe46f3c5c1..cf0e19e4907 100644 --- a/lib/oli/rendering/content/html.ex +++ b/lib/oli/rendering/content/html.ex @@ -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 @@ -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 \\ [] @@ -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 diff --git a/test/oli/rendering/content/html_test.exs b/test/oli/rendering/content/html_test.exs index 63cc142fdd5..fea2761ec90 100644 --- a/test/oli/rendering/content/html_test.exs +++ b/test/oli/rendering/content/html_test.exs @@ -32,7 +32,7 @@ defmodule Oli.Content.Content.HtmlTest do "

The American colonials proclaimed "no taxation without representation" assert rendered_html_string =~ - "Page Two" + "Page Two" assert rendered_html_string =~ "Stamp Act Congress" From 037eb2927c33c08ea47283538ff6b4c731f96736 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Abell=C3=A1?= Date: Tue, 6 Aug 2024 19:24:38 -0300 Subject: [PATCH 3/5] [MER-3565] Add missing props --- .../discussion/MockDiscussionDeliveryProvider.tsx | 1 + assets/test/check_all_that_apply/cata_delivery_test.tsx | 1 + assets/test/multiple_choice/mc_delivery_test.tsx | 1 + assets/test/ordering/ordering_delivery_test.tsx | 1 + assets/test/short_answer/short_answer_delivery_test.tsx | 1 + assets/test/writer/writer_test.ts | 2 +- 6 files changed, 6 insertions(+), 1 deletion(-) diff --git a/assets/src/components/activities/directed-discussion/discussion/MockDiscussionDeliveryProvider.tsx b/assets/src/components/activities/directed-discussion/discussion/MockDiscussionDeliveryProvider.tsx index 4a59a28f44f..d9d9da73e5b 100644 --- a/assets/src/components/activities/directed-discussion/discussion/MockDiscussionDeliveryProvider.tsx +++ b/assets/src/components/activities/directed-discussion/discussion/MockDiscussionDeliveryProvider.tsx @@ -46,6 +46,7 @@ export const MockDiscussionDeliveryProvider: React.FC<{ renderPointMarkers: false, isAnnotationLevel: false, variables: {}, + pageLinkParams: {}, }} onSaveActivity={nullHandler} onSavePart={nullHandler} diff --git a/assets/test/check_all_that_apply/cata_delivery_test.tsx b/assets/test/check_all_that_apply/cata_delivery_test.tsx index f3068879b6e..2abd139915e 100644 --- a/assets/test/check_all_that_apply/cata_delivery_test.tsx +++ b/assets/test/check_all_that_apply/cata_delivery_test.tsx @@ -33,6 +33,7 @@ describe('check all that apply delivery', () => { renderPointMarkers: false, isAnnotationLevel: false, variables: {}, + pageLinkParams: {}, }, preview: false, }; diff --git a/assets/test/multiple_choice/mc_delivery_test.tsx b/assets/test/multiple_choice/mc_delivery_test.tsx index ca9cdf94c42..f8d14c345b8 100644 --- a/assets/test/multiple_choice/mc_delivery_test.tsx +++ b/assets/test/multiple_choice/mc_delivery_test.tsx @@ -34,6 +34,7 @@ describe('multiple choice delivery', () => { renderPointMarkers: false, isAnnotationLevel: false, variables: {}, + pageLinkParams: {}, }, graded: false, preview: false, diff --git a/assets/test/ordering/ordering_delivery_test.tsx b/assets/test/ordering/ordering_delivery_test.tsx index e08703f1866..5df348d0de3 100644 --- a/assets/test/ordering/ordering_delivery_test.tsx +++ b/assets/test/ordering/ordering_delivery_test.tsx @@ -33,6 +33,7 @@ describe('ordering delivery', () => { renderPointMarkers: false, isAnnotationLevel: false, variables: {}, + pageLinkParams: {}, }, preview: false, }; diff --git a/assets/test/short_answer/short_answer_delivery_test.tsx b/assets/test/short_answer/short_answer_delivery_test.tsx index 4f4a307fc17..e89b728ee25 100644 --- a/assets/test/short_answer/short_answer_delivery_test.tsx +++ b/assets/test/short_answer/short_answer_delivery_test.tsx @@ -33,6 +33,7 @@ describe('multiple choice delivery', () => { renderPointMarkers: false, isAnnotationLevel: false, variables: {}, + pageLinkParams: {}, }, preview: false, }; diff --git a/assets/test/writer/writer_test.ts b/assets/test/writer/writer_test.ts index 1e6961e3b9b..4aecf2e96a7 100644 --- a/assets/test/writer/writer_test.ts +++ b/assets/test/writer/writer_test.ts @@ -104,7 +104,7 @@ describe('parser', () => { 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' && content === 'Page Two' ); }), From 6835e13ec2b7e1dadb9376ff28232b5d090d0313 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Abell=C3=A1?= Date: Tue, 6 Aug 2024 21:02:13 -0300 Subject: [PATCH 4/5] [MER-3565] Fix typescript tests --- assets/test/writer/writer_test.ts | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/assets/test/writer/writer_test.ts b/assets/test/writer/writer_test.ts index 4aecf2e96a7..2fb96b7c237 100644 --- a/assets/test/writer/writer_test.ts +++ b/assets/test/writer/writer_test.ts @@ -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/lesson/page_two' && + element.getAttribute('href') === + '/sections/some_section/lesson/page_two?request_path=%2Fpath%2Fto%2Fprevious%2Fpage' && content === 'Page Two' ); }), From 258025000d1d0bf10b97cb739d3cde38e0ac4cbe Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gast=C3=B3n=20Abell=C3=A1?= Date: Wed, 7 Aug 2024 12:10:53 -0300 Subject: [PATCH 5/5] [MER-3565] Update docker compose command in CI test runner --- .github/workflows/build.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index 55605bc8726..5924a02917f 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -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