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

[CHORE] [MER-0000] Integrate v0.28.10 #5125

Merged
merged 16 commits into from
Sep 24, 2024
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
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import React, { createRef } from 'react';
import React, { createRef, useEffect, useState } from 'react';
import { MultiInputSize } from 'components/activities/multi_input/schema';
import { disableScrollWheelChange } from 'components/activities/short_answer/utils';
import { classNames } from 'utils/classNames';
import { isValidNumber } from 'utils/number';

interface Props {
value: string;
Expand All @@ -12,20 +13,35 @@ interface Props {
onBlur?: () => void;
onKeyUp: (e: React.KeyboardEvent<HTMLInputElement | HTMLTextAreaElement>) => void;
}

export const NumericInput: React.FC<Props> = (props) => {
const numericInputRef = createRef<HTMLInputElement>();
const [hasError, setHasError] = useState(false);

useEffect(() => {
if (props.value === '' || !isValidNumber(props.value)) {
setHasError(true);
} else {
setHasError(false);
}
}, [props.value]);

return (
<input
ref={numericInputRef}
placeholder={props.placeholder}
type="number"
type="text"
aria-label="answer submission textbox"
className={classNames(
'border-gray-300 rounded-md disabled:bg-gray-100 disabled:text-gray-600',
'rounded-md border-2 disabled:bg-gray-100 disabled:text-gray-600',
hasError ? 'input-error' : 'border-gray-300', // Use custom error class
'focus:outline-none', // Remove default focus outline
props.size && `input-size-${props.size}`,
)}
onChange={(e) => props.onChange(e.target.value)}
onChange={(e) => {
const value = e.target.value;
props.onChange(value);
}}
onBlur={props.onBlur}
onKeyUp={props.onKeyUp}
value={props.value}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,11 @@ import { LockResult } from 'data/persistence//lock';
import { DeferredPersistenceStrategy } from 'data/persistence/DeferredPersistenceStrategy';
import { PersistenceState, PersistenceStrategy } from 'data/persistence/PersistenceStrategy';

export function initializePersistence(): PersistenceStrategy {
const p = new DeferredPersistenceStrategy();
export function initializePersistence(
quietPeriodInMs = 2000,
maxDeferredTimeInMs = 5000,
): PersistenceStrategy {
const p = new DeferredPersistenceStrategy(quietPeriodInMs, maxDeferredTimeInMs);
const noOpLockAcquire = () => Promise.resolve({ type: 'acquired' } as LockResult);
const noOpLockRelease = () => Promise.resolve({ type: 'released' } as LockResult);
const noOpSuccess = () => {};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,7 +224,7 @@ export const MultiInputComponent: React.FC = () => {
}),
);

const fn = () =>
const doSave = () =>
onSaveActivity(uiState.attemptState.attemptGuid, [
{
attemptGuid: part.attemptGuid,
Expand All @@ -236,10 +236,10 @@ export const MultiInputComponent: React.FC = () => {
if ((uiState.model as MultiInputSchema).submitPerPart && !context.graded) {
handlePerPartSubmission(input.partId, value);
} else {
fn();
doSave();
}
} else {
deferredSaves.current[id].save(fn);
doSave();
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ export const ResponseMultiInputComponent: React.FC = () => {

const deferredSaves = useRef(
model.inputs.reduce((m: any, input: MultiInput) => {
const p = initializePersistence();
const p = initializePersistence(750, 1200);
m[input.id] = p;
return m;
}, {}),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@ export const ShortAnswerComponent: React.FC = () => {
const uiState = useSelector((state: ActivityDeliveryState) => state);
const { surveyId } = context;
const dispatch = useDispatch();
const deferredSave = useRef(initializePersistence());
const deferredSave = useRef(initializePersistence(750, 1200));

useEffect(() => {
listenForParentSurveySubmit(surveyId, dispatch, onSubmitActivity);
Expand Down Expand Up @@ -121,11 +121,16 @@ export const ShortAnswerComponent: React.FC = () => {
}),
);

deferredSave.current.save(() =>
const doSave = () =>
onSaveActivity(uiState.attemptState.attemptGuid, [
{ attemptGuid: uiState.attemptState.parts[0].attemptGuid, response: { input } },
]),
);
]);

if ((uiState.model as ShortAnswerModelSchema).inputType == 'textarea') {
deferredSave.current.save(doSave);
} else {
doSave();
}
};

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
} from 'data/activities/model/rules';
import { classNames } from 'utils/classNames';
import guid from 'utils/guid';
import { isValidNumber } from 'utils/number';
import { disableScrollWheelChange } from '../utils';

// here we defined a "editable number" variant data type that contains information
Expand Down Expand Up @@ -39,10 +40,6 @@ export type EditableNumber = InvalidNumber | ValidNumber;
const isVariable = (s: numberOrVar): boolean =>
typeof s === 'string' && s.match(/^@@\w+@@$/) !== null;

const isValidNumber = (s: string): boolean =>
// allowing .333 but not 33. Need disjunction to allow both
typeof s === 'string' && s.match(/^[+-]?\d*\.?\d+(?:[Ee][+-]?\d+)?$/) != null;

const parsedNumberOrEmptyString = (num: number | undefined) =>
num != undefined && !isNaN(num) ? num : '';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ export const YouTubeEditor = (props: YouTubeProps) => {
<Settings model={props.model} onEdit={onEdit} commandContext={props.commandContext} />
}
>
<YoutubePlayer video={model} authorMode={true} />
<YoutubePlayer video={model} authorMode={true} pageAttemptGuid="" />
</HoverContainer>

<CaptionEditor
Expand Down
14 changes: 6 additions & 8 deletions assets/src/components/youtube_player/YoutubePlayer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ export const YoutubePlayer: React.FC<{
video: ContentModel.YouTube;
children?: React.ReactNode;
context?: WriterContext;
pageAttemptGuid: string;
authorMode: boolean;
pointMarkerContext?: PointMarkerContext;
}> = ({ video, children, authorMode, context, pointMarkerContext }) => {
}> = ({ video, children, authorMode, context, pointMarkerContext, pageAttemptGuid }) => {
const stopInterval = useRef<number | undefined>();
const [videoTarget, setVideoTarget] = useState<Player | null>(null);
const segments = useRef<XAPI.PlayedSegment[]>([]);
Expand Down Expand Up @@ -101,15 +102,14 @@ export const YoutubePlayer: React.FC<{
}, [authorMode, video.endTime, video.startTime, videoTarget]);

const onStateChange = (e: any) => {
if (!videoTarget) return;
if (!videoTarget || pageAttemptGuid == '') return;

switch (e.data) {
case 0:
XAPI.emit_delivery(
{
type: 'page_video_key',
page_attempt_guid:
context?.resourceAttemptGuid !== undefined ? context?.resourceAttemptGuid : '',
page_attempt_guid: pageAttemptGuid,
},
{
type: 'video_completed',
Expand All @@ -132,8 +132,7 @@ export const YoutubePlayer: React.FC<{
XAPI.emit_delivery(
{
type: 'page_video_key',
page_attempt_guid:
context?.resourceAttemptGuid !== undefined ? context?.resourceAttemptGuid : '',
page_attempt_guid: pageAttemptGuid,
},
{
type: 'video_played',
Expand All @@ -155,8 +154,7 @@ export const YoutubePlayer: React.FC<{
XAPI.emit_delivery(
{
type: 'page_video_key',
page_attempt_guid:
context?.resourceAttemptGuid !== undefined ? context?.resourceAttemptGuid : '',
page_attempt_guid: pageAttemptGuid,
},
{
type: 'video_paused',
Expand Down
5 changes: 5 additions & 0 deletions assets/src/data/content/writers/html.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -365,11 +365,16 @@ export class HtmlParser implements WriterImpl {

youtube(context: WriterContext, next: Next, attrs: YouTube) {
if (!attrs.src) return <></>;
let guid = '';
if (context.resourceAttemptGuid !== undefined) {
guid = context.resourceAttemptGuid;
}
return (
<YoutubePlayer
video={attrs}
authorMode={false}
context={context}
pageAttemptGuid={guid}
pointMarkerContext={pointMarkerContextFrom(context, attrs)}
/>
);
Expand Down
32 changes: 32 additions & 0 deletions assets/src/hooks/delayed_submit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
export const DelayedSubmit = {
mounted() {
this.el.addEventListener('click', (event: any) => {
event.preventDefault(); // Prevent immediate click action

const inputs = document.querySelectorAll(
'input[type="text"], input[type="number"], textarea, select',
);

// Loop through each element and disable it. This prevents students from making any
// edits in activities while the submission is processing.
inputs.forEach((input: any) => {
input.disabled = true;
});

// Disable the button to prevent additional clicks
this.el.disabled = true;

// Change the button label and show the spinner
this.el.querySelector('.button-text').textContent = 'Submitting Answers...';
this.el.querySelector('.spinner').classList.remove('hidden');

// Delay the phx-click event by two full seconds
setTimeout(() => {
// Trigger the Phoenix event after the delay
this.pushEvent('finalize_attempt');

// Optionally, remove the spinner and reset button state if needed
}, 2000);
});
},
};
2 changes: 2 additions & 0 deletions assets/src/hooks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { Countdown } from './countdown';
import { CountdownTimer } from './countdown_timer';
import { CustomFocusWrap } from './custom_focus_wrap';
import { DateTimeLocalInputListener } from './datetimelocal_input_listener';
import { DelayedSubmit } from './delayed_submit';
import { DragSource, DropTarget } from './dragdrop';
import { EmailList } from './email_list';
import { EndDateTimer } from './end_date_timer';
Expand Down Expand Up @@ -39,6 +40,7 @@ import { VideoPlayer } from './video_player';
import { PauseOthersOnSelected, VideoPreview } from './video_preview';

export const Hooks = {
DelayedSubmit,
GraphNavigation,
DropTarget,
DragSource,
Expand Down
3 changes: 3 additions & 0 deletions assets/src/utils/number.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
// disjunction allows for both .300 and 300.
const regex = /^[+-]?((\d+\.?\d*)|(\.\d+))([eE][-+]?\d+)?$/;
export const isValidNumber = (value: string) => regex.test(value);
4 changes: 4 additions & 0 deletions assets/styles/delivery/activity.scss
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@
margin-top: 1em;
margin-bottom: 4em;

.input-error {
border-color: red !important; /* Override any other border color */
}

.activity-content {
display: flex;
flex-direction: column;
Expand Down
63 changes: 63 additions & 0 deletions assets/test/utils/number_test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
import { isValidNumber } from '../../src/utils/number';

describe('isValidNumber', () => {
// Valid cases
test('should return true for valid integers', () => {
expect(isValidNumber('0')).toBe(true);
expect(isValidNumber('3')).toBe(true);
expect(isValidNumber('-3')).toBe(true);
expect(isValidNumber('+3')).toBe(true);
expect(isValidNumber('123456')).toBe(true);
});

test('should return true for valid floating-point numbers', () => {
expect(isValidNumber('0.0')).toBe(true);
expect(isValidNumber('3.14')).toBe(true);
expect(isValidNumber('-3.14')).toBe(true);
expect(isValidNumber('123.456')).toBe(true);
expect(isValidNumber('.456')).toBe(true);
// not allowed in HTML standard but desired in significant figure contexts:
expect(isValidNumber('320.')).toBe(true);
});

test('should return true for valid scientific notation', () => {
expect(isValidNumber('3e10')).toBe(true);
expect(isValidNumber('-3e10')).toBe(true);
expect(isValidNumber('3.14e-2')).toBe(true);
expect(isValidNumber('2.5E+5')).toBe(true);
});

// Invalid cases
test('should return false for invalid numbers', () => {
expect(isValidNumber('3g')).toBe(false);
expect(isValidNumber('abc')).toBe(false);
expect(isValidNumber('3.14.15')).toBe(false);
expect(isValidNumber('++3')).toBe(false);
expect(isValidNumber('--3')).toBe(false);
expect(isValidNumber('3e+')).toBe(false);
expect(isValidNumber('e3')).toBe(false);
expect(isValidNumber('3e3e4')).toBe(false);
expect(isValidNumber('.')).toBe(false);
});

// Edge cases
test('should return false for empty strings', () => {
expect(isValidNumber('')).toBe(false);
});

test('should return false for whitespace', () => {
expect(isValidNumber(' ')).toBe(false);
expect(isValidNumber('\t')).toBe(false);
expect(isValidNumber('\n')).toBe(false);
});

test('should return false for only a decimal point', () => {
expect(isValidNumber('.')).toBe(false);
expect(isValidNumber('-')).toBe(false);
});

test('should return true for zero in different formats', () => {
expect(isValidNumber('0')).toBe(true);
expect(isValidNumber('0.0')).toBe(true);
});
});
2 changes: 2 additions & 0 deletions config/runtime.exs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ if config_env() == :prod do
database: System.get_env("DB_NAME", "oli"),
pool_size: String.to_integer(System.get_env("POOL_SIZE") || "10"),
timeout: db_timeout,
queue_target: String.to_integer(System.get_env("DB_QUEUE_TARGET") || "50"),
queue_interval: String.to_integer(System.get_env("DB_QUEUE_INTERVAL") || "1000"),
ownership_timeout: 600_000,
socket_options: maybe_ipv6

Expand Down
11 changes: 11 additions & 0 deletions lib/oli/accounts.ex
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,17 @@ defmodule Oli.Accounts do
"""
def get_user_by(clauses), do: Repo.get_by(User, clauses)

@doc """
Gets a single independent user by query parameter
## Examples
iex> get_independent_user_by(email: "student1@example.com")
%User{independent_learner: true, ...}
iex> get_independent_user_by(email: "student2@example.com")
nil
"""
def get_independent_user_by(clauses),
do: Repo.get_by(User, Enum.into([independent_learner: true], clauses))

@doc """
Gets a single user with platform roles and author preloaded
Returns `nil` if the User does not exist.
Expand Down
7 changes: 7 additions & 0 deletions lib/oli/rendering/content/html.ex
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,16 @@ defmodule Oli.Rendering.Content.Html do
end

def youtube(%Context{} = context, _, %{"src" => _} = attrs) do
attempt_guid =
case context.resource_attempt do
nil -> ""
attempt -> attempt.attempt_guid
end

{:safe, video_player} =
OliWeb.Common.React.component(context, "Components.YoutubePlayer", %{
"video" => attrs,
"pageAttemptGuid" => attempt_guid,
"pointMarkerContext" => %{
renderPointMarkers: context.render_opts.render_point_markers,
isAnnotationLevel: context.is_annotation_level
Expand Down
Loading
Loading