Skip to content

Commit

Permalink
[ENHANCEMENT] [MER-3672] Page context is loaded twice (#5054)
Browse files Browse the repository at this point in the history
* move lesson, prologue and adative_lesson routes to a separate route scope to implement RedirectByAttemptState plug

* remove old RequireExplorationPages plug used in old page template (before NG23) => remove 2 unnecessary queries

* remove maybe_gated_resource plug from not pages scope. only necesary in prologue, lesson and adaptive_lesson scope

* remove 3 redirect livesesion plugs, now replaced by the new redirect_by_attempt_state plug

* update test

* fix bug on back arrow after finishing an adaptive lesson - nil case was
not considered on MER-3506

* update failing test

* fix warning

* create a slimmer context for prologue pages

* do not assign prologue assigns if we are not aiming the prologue page

* always track access on PageContext.create_for_visit since it will be called only once

* use span for popup react components when rendering on a liveview

* only init pages when socket is already connected

* lesson_live: calculate assigns when socket is connected + load scripts with a hook

* adapt student_delivery_lesson template to render content when socket is connected + scripts loaded

* skip the 'scripts loaded' step in the prologue liveview

* review_live: calculate assigns when socket is connected + load scripts with a hook

* handle the error case on activity transformer viariable substitution - only affects dev mode when beginning a new page attempt

* move adaptive lessons from LessonLive to it's own separate liveview

* move emit_page_viewed_event/1 and its helpers to student's Utils module

* remove unused imports

* handle garbage collection on new AdaptiveLessson liveview

* handle adaptive chromeless pages with phoenix controller instead of a liveview - this avoids rendering page context twice

* handle adaptive chromeless review pages with phoenix controller - this avoids rendering page context twice

* remove unused scripts - they are loaded async triggered from the client by a hook

* fix warnings

* update tests for review liveview

* update lesson live tests to ensure content is visibile

* update maybe gated resource test

* update tests considering now the adaptive reviews are handled by the page delivery controller and all other cases by the lesson liveview

* evaluate MathJax expressions after websocket is connected

* move another query inside connected?

* Auto format

---------

Co-authored-by: Darren Siegel <[email protected]>
Co-authored-by: darrensiegel <[email protected]>
  • Loading branch information
3 people authored Aug 30, 2024
1 parent 1f67d34 commit 4a026df
Show file tree
Hide file tree
Showing 28 changed files with 1,069 additions and 1,246 deletions.
10 changes: 10 additions & 0 deletions assets/src/hooks/evaluate_mathjax_expressions.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// this hook is used to evaluate MathJax expressions in the page
// just after the page is mounted and the websocket connection is established.

export const EvaluateMathJaxExpressions = {
mounted() {
const elements = document.querySelectorAll('.formula');

window.MathJax.typesetPromise(Array.from(elements) as HTMLElement[]);
},
};
2 changes: 2 additions & 0 deletions assets/src/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { DateTimeLocalInputListener } from './datetimelocal_input_listener';
import { DragSource, DropTarget } from './dragdrop';
import { EmailList } from './email_list';
import { EndDateTimer } from './end_date_timer';
import { EvaluateMathJaxExpressions } from './evaluate_mathjax_expressions';
import { GraphNavigation } from './graph';
import { HierarchySelector } from './hierarchy_selector';
import { InputAutoSelect } from './input_auto_select';
Expand Down Expand Up @@ -79,4 +80,5 @@ export const Hooks = {
Countdown,
CountdownTimer,
EndDateTimer,
EvaluateMathJaxExpressions,
};
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ defmodule Oli.Activities.Transformers.VariableSubstitution.RestImpl do

{:ok, %HTTPoison.Response{}} ->
{:error, "Error retrieving the payload"}

{:error, reason} ->
{:error, reason}
end
end
end
6 changes: 2 additions & 4 deletions lib/oli/delivery/page/page_context.ex
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,7 @@ defmodule Oli.Delivery.Page.PageContext do
%Section{slug: section_slug, id: section_id},
page_slug,
user,
datashop_session_id,
opts \\ [track_access: true]
datashop_session_id
) do
# resolve the page revision per section
page_revision =
Expand All @@ -190,8 +189,7 @@ defmodule Oli.Delivery.Page.PageContext do
effective_settings =
Oli.Delivery.Settings.get_combined_settings(page_revision, section_id, user.id)

if opts[:track_access],
do: Attempts.track_access(page_revision.resource_id, section_id, user.id)
Attempts.track_access(page_revision.resource_id, section_id, user.id)

activity_provider = &Oli.Delivery.ActivityProvider.provide/6

Expand Down
67 changes: 67 additions & 0 deletions lib/oli/delivery/page/prologue_context.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
defmodule Oli.Delivery.Page.PrologueContext do
@moduledoc """
Defines the context required to render the prologue page in delivery mode.
"""

defstruct [
:page,
:resource_attempts,
:historical_attempts,
:effective_settings
]

alias Oli.Publishing.DeliveryResolver
alias Oli.Delivery.Attempts.Core
alias Oli.Delivery.Sections.Section

@doc """
Creates the page context required to render a page for visiting a current or new
attempt.
"""
def create_for_visit(
%Section{slug: section_slug, id: section_id},
page_slug,
user
) do
# resolve the page revision per section
page_revision = DeliveryResolver.from_revision_slug(section_slug, page_slug)
Core.track_access(page_revision.resource_id, section_id, user.id)

effective_settings =
Oli.Delivery.Settings.get_combined_settings(page_revision, section_id, user.id)

attempts =
case Core.get_resource_attempt_history(
page_revision.resource_id,
section_slug,
user.id
) do
{_access, attempts} -> attempts
nil -> []
end

graded_attempts = Enum.filter(attempts, fn a -> a.revision.graded == true end)

%__MODULE__{
page: page_revision,
resource_attempts: graded_attempts,
historical_attempts: retrieve_historical_attempts(graded_attempts),
effective_settings: effective_settings
}
end

# For the given resource attempt, retrieve all historical activity attempt records,
# assembling them into a map of activity_id to a list of the activity attempts, including
# the latest attempt

defp retrieve_historical_attempts([]), do: %{}

defp retrieve_historical_attempts(resource_attempts) do
Core.get_all_activity_attempts(hd(resource_attempts).id)
|> Enum.sort(fn a1, a2 -> a1.attempt_number < a2.attempt_number end)
|> Enum.reduce(%{}, fn a, m ->
appended = Map.get(m, a.resource_id, []) ++ [a]
Map.put(m, a.resource_id, appended)
end)
end
end
4 changes: 3 additions & 1 deletion lib/oli/rendering/content/html.ex
Original file line number Diff line number Diff line change
Expand Up @@ -811,7 +811,9 @@ defmodule Oli.Rendering.Content.Html do
"element" => element,
"inline" => true
},
html_element: "span"
html_element: "span",
container_tag: "span",
id: "popup_#{UUID.uuid4()}"
)

rendered
Expand Down
221 changes: 115 additions & 106 deletions lib/oli_web/components/layouts/student_delivery_lesson.html.heex
Original file line number Diff line number Diff line change
@@ -1,119 +1,128 @@
<div id="live_flash_container" class="fixed top-14 w-full mx-auto z-50">
<%= if live_flash(@flash, :info) do %>
<div class="alert alert-info flex flex-row" role="alert">
<div class="flex-1">
<%= live_flash(@flash, :info) %>
</div>
<div id="eventIntercept" phx-hook="LoadSurveyScripts">
<div :if={Phoenix.LiveView.connected?(@socket) and assigns[:scripts_loaded]}>
<script>
window.userToken = "<%= @user_token %>";
</script>
<div id="live_flash_container" class="fixed top-14 w-full mx-auto z-50">
<%= if live_flash(@flash, :info) do %>
<div class="alert alert-info flex flex-row" role="alert">
<div class="flex-1">
<%= live_flash(@flash, :info) %>
</div>

<button
type="button"
class="close"
data-bs-dismiss="alert"
aria-label="Close"
phx-click="lv:clear-flash"
phx-value-key="info"
>
<i class="fa-solid fa-xmark fa-lg"></i>
</button>
</div>
<% end %>
<button
type="button"
class="close"
data-bs-dismiss="alert"
aria-label="Close"
phx-click="lv:clear-flash"
phx-value-key="info"
>
<i class="fa-solid fa-xmark fa-lg"></i>
</button>
</div>
<% end %>

<%= if live_flash(@flash, :error) do %>
<div class="alert alert-danger flex flex-row" role="alert">
<div class="flex-1">
<%= live_flash(@flash, :error) %>
</div>
<%= if live_flash(@flash, :error) do %>
<div class="alert alert-danger flex flex-row" role="alert">
<div class="flex-1">
<%= live_flash(@flash, :error) %>
</div>

<button
type="button"
class="close"
data-bs-dismiss="alert"
aria-label="Close"
phx-click="lv:clear-flash"
phx-value-key="error"
>
<i class="fa-solid fa-xmark fa-lg"></i>
</button>
<button
type="button"
class="close"
data-bs-dismiss="alert"
aria-label="Close"
phx-click="lv:clear-flash"
phx-value-key="error"
>
<i class="fa-solid fa-xmark fa-lg"></i>
</button>
</div>
<% end %>
</div>
<% end %>
</div>

<div class="h-screen flex flex-col overscroll-none overflow-hidden">
<Components.Delivery.Layouts.header
:if={@section}
ctx={@ctx}
is_system_admin={assigns[:is_system_admin] || false}
section={@section}
preview_mode={@preview_mode}
force_show_user_menu={true}
include_logo={true}
/>
<div class="h-screen flex flex-col overscroll-none overflow-hidden">
<Components.Delivery.Layouts.header
:if={@section}
ctx={@ctx}
is_system_admin={assigns[:is_system_admin] || false}
section={@section}
preview_mode={@preview_mode}
force_show_user_menu={true}
include_logo={true}
/>

<div class="flex-1 flex flex-col mt-14 overflow-hidden">
<div
:if={assigns[:paywall_summary] && OliWeb.LayoutView.show_pay_early(@paywall_summary)}
id="pay_early_message"
class="absolute z-50 system-banner flex flex-row alert alert-warning ml-32 m-6"
role="alert"
>
<%= OliWeb.LayoutView.pay_early_message(@paywall_summary) %>
<div class="flex whitespace-nowrap items-center">
<.link class="ml-8" href={Routes.payment_path(@socket, :guard, @section.slug)}>
Pay Now
</.link>
<button
class="ml-10 stroke-gray-500 hover:stroke-gray-400"
phx-click={JS.hide(to: "#pay_early_message")}
<div class="flex-1 flex flex-col mt-14 overflow-hidden">
<div
:if={assigns[:paywall_summary] && OliWeb.LayoutView.show_pay_early(@paywall_summary)}
id="pay_early_message"
class="absolute z-50 system-banner flex flex-row alert alert-warning ml-32 m-6"
role="alert"
>
<OliWeb.Icons.close />
</button>
</div>
</div>
<%= OliWeb.LayoutView.pay_early_message(@paywall_summary) %>
<div class="flex whitespace-nowrap items-center">
<.link class="ml-8" href={Routes.payment_path(@socket, :guard, @section.slug)}>
Pay Now
</.link>
<button
class="ml-10 stroke-gray-500 hover:stroke-gray-400"
phx-click={JS.hide(to: "#pay_early_message")}
>
<OliWeb.Icons.close />
</button>
</div>
</div>

<div
:if={@section}
id="page-content"
class="flex-1 flex flex-col relative justify-center items-start overflow-hidden"
>
<div
:if={@view in [:graded_page, :practice_page] and @page_progress_state == :in_progress}
id="offline_detector"
>
<%= react_component("Components.OfflineDetector") %>
<div
:if={@section}
id="page-content"
phx-hook="EvaluateMathJaxExpressions"
class="flex-1 flex flex-col relative justify-center items-start overflow-hidden"
>
<div
:if={@view in [:graded_page, :practice_page] and @page_progress_state == :in_progress}
id="offline_detector"
>
<%= react_component("Components.OfflineDetector") %>
</div>
<.back_arrow
to={
if assigns[:request_path] in [nil, ""],
do:
~p"/sections/#{@section.slug}/learn?target_resource_id=#{@current_page["id"]}",
else: assigns[:request_path]
}
show_sidebar={assigns[:show_sidebar]}
view={assigns[:view]}
/>

<%= @inner_content %>
</div>

<Components.Delivery.Layouts.previous_next_nav
:if={assigns[:page_context]}
current_page={@current_page}
previous_page={@previous_page}
next_page={@next_page}
section_slug={@section.slug}
request_path={assigns[:request_path]}
selected_view={assigns[:selected_view]}
/>
</div>
<.back_arrow
to={
if assigns[:request_path] in [""],
do: ~p"/sections/#{@section.slug}/learn?target_resource_id=#{@current_page["id"]}",
else: assigns[:request_path]
}
show_sidebar={assigns[:show_sidebar]}
view={assigns[:view]}
/>

<%= @inner_content %>
<%= if @section do %>
<%= live_render(@socket, OliWeb.Dialogue.WindowLive,
session: %{
"section_slug" => @section.slug,
"resource_id" => @current_page["id"],
"revision_id" => @page_context.page.id,
"is_page" => true
},
id: "dialogue-window"
) %>
<% end %>
</div>

<Components.Delivery.Layouts.previous_next_nav
:if={assigns[:page_context]}
current_page={@current_page}
previous_page={@previous_page}
next_page={@next_page}
section_slug={@section.slug}
request_path={assigns[:request_path]}
selected_view={assigns[:selected_view]}
/>
</div>

<%= if @section do %>
<%= live_render(@socket, OliWeb.Dialogue.WindowLive,
session: %{
"section_slug" => @section.slug,
"resource_id" => @current_page["id"],
"revision_id" => @page_context.page.id,
"is_page" => true
},
id: "dialogue-window"
) %>
<% end %>
</div>
11 changes: 10 additions & 1 deletion lib/oli_web/controllers/page_delivery_controller.ex
Original file line number Diff line number Diff line change
Expand Up @@ -738,7 +738,7 @@ defmodule OliWeb.PageDeliveryController do
pageTitle: context.page.title,
pageSlug: context.page.slug,
graded: context.page.graded,
content: context.page.content,
content: build_page_content(context.page.content, conn.params["request_path"]),
resourceAttemptState: resource_attempt.state,
resourceAttemptGuid: resource_attempt.attempt_guid,
currentServerTime: DateTime.utc_now() |> to_epoch,
Expand Down Expand Up @@ -797,6 +797,15 @@ defmodule OliWeb.PageDeliveryController do
})
end

_docp = """
In case there is a request path we add that path in the page content as 'backUrl'.
This backUrl aims to return the student to the page they were on
before they accessed the page we are building (i.e. the "Learn", "Home" or "Schedule" page)
"""

defp build_page_content(content, nil), do: content
defp build_page_content(content, request_path), do: Map.put(content, "backUrl", request_path)

defp to_epoch(nil), do: nil

defp to_epoch(date_time) do
Expand Down
Loading

0 comments on commit 4a026df

Please sign in to comment.