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

Refactor block retrieval #1241

Merged
merged 15 commits into from
Sep 4, 2024
Merged
Show file tree
Hide file tree
Changes from 11 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
15 changes: 9 additions & 6 deletions backend/experiment/rules/tests/test_beat_alignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,17 +56,21 @@ def test_block(self):

block_json = self.load_json(block_response)
self.assertTrue({'id', 'slug', 'name', 'class_name', 'rounds',
'playlists', 'next_round', 'loading_text', 'session_id'} <= block_json.keys())
rounds = block_json.get('next_round')
'playlists', 'loading_text', 'session_id'} <= block_json.keys())
session_id = block_json['session_id']
response = self.client.post(
f'/session/{session_id}/next_round/')
rounds = self.load_json(response).get('next_round')

# check that we get the intro explainer, 3 practice rounds and another explainer
self.assertEqual(len(rounds), 5)
self.assertEqual(
block_json['next_round'][0]['view'], 'EXPLAINER')
rounds[0]['view'], 'EXPLAINER')
# check practice rounds
self.assertEqual(rounds[1].get('title'), 'Example 1')
self.assertEqual(rounds[3].get('title'), 'Example 3')
self.assertEqual(
block_json['next_round'][4]['view'], 'EXPLAINER')
rounds[4]['view'], 'EXPLAINER')

header = {'HTTP_USER_AGENT': "Test device with test browser"}
participant_response = self.client.get('/participant/', **header)
Expand All @@ -85,12 +89,11 @@ def test_block(self):
self.assertTrue(consent_json['status'], 'ok')

# test remaining rounds with request to `/session/{session_id}/next_round/`
session_id = block_json['session_id']
rounds_n = self.block.rounds # Default 10
views_exp = ['TRIAL_VIEW']*(rounds_n)
for i in range(len(views_exp)):
response = self.client.post(
'/session/{}/next_round/'.format(session_id))
f'/session/{session_id}/next_round/')
response_json = self.load_json(response)
result_id = response_json.get(
'next_round')[0]['feedback_form']['form'][0]['result_id']
Expand Down
7 changes: 0 additions & 7 deletions backend/experiment/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,6 @@ def get_block(request: HttpRequest, slug: str) -> JsonResponse:
"bonus_points": block.bonus_points,
"playlists": [{"id": playlist.id, "name": playlist.name} for playlist in block.playlists.all()],
"feedback_info": block.get_rules().feedback_info(),
# only call first round if the (deprecated) first_round method exists
# otherwise, call next_round
"next_round": (
serialize_actions(block.get_rules().first_round(block))
if hasattr(block.get_rules(), "first_round") and block.get_rules().first_round
else serialize_actions(block.get_rules().next_round(session))
),
"loading_text": _("Loading"),
"session_id": session.id,
}
Expand Down
13 changes: 7 additions & 6 deletions frontend/src/API.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ import { API_BASE_URL } from "@/config";
import useGet from "./util/useGet";
import axios from "axios";
import qs from "qs";
import Block, { ExtendedBlock } from "@/types/Block";
import IBlock from "@/types/Block";
import IExperiment from "@/types/Experiment";
import Participant, { ParticipantLink } from "./types/Participant";
import Session from "./types/Session";

Expand Down Expand Up @@ -43,8 +44,8 @@ export const URLS = {
}
};

export const useBlock = (slug: string): [ExtendedBlock | null, boolean] =>
useGet<ExtendedBlock>(API_BASE_URL + URLS.block.get(slug));
export const useBlock = (slug: string): [IBlock | null, boolean] =>
useGet<IBlock>(API_BASE_URL + URLS.block.get(slug));

export const useExperiment = (slug: string) => {
const data = useGet(API_BASE_URL + URLS.experiment.get(slug));
Expand All @@ -63,19 +64,19 @@ export const useConsent = (slug: string) =>
useGet<ConsentResponse>(API_BASE_URL + URLS.result.get('consent_' + slug));

interface CreateConsentParams {
block: Block;
experiment: IExperiment;
participant: Pick<Participant, 'csrf_token'>;
}

/** Create consent for given experiment */
export const createConsent = async ({ block, participant }: CreateConsentParams) => {
export const createConsent = async ({ experiment, participant }: CreateConsentParams) => {
try {
const response = await axios.post(
API_BASE_URL + URLS.result.consent,
qs.stringify({
json_data: JSON.stringify(
{
key: "consent_" + block.slug,
key: "consent_" + experiment.slug,
value: true,
}
),
Expand Down
42 changes: 22 additions & 20 deletions frontend/src/components/Block/Block.test.tsx
Original file line number Diff line number Diff line change
@@ -1,13 +1,9 @@
import { Route, MemoryRouter, Routes } from 'react-router-dom';
import { fireEvent, render, screen, waitFor } from '@testing-library/react';
import { beforeEach, vi } from 'vitest';
import MockAdapter from 'axios-mock-adapter';
import axios from 'axios';
import { render, screen, waitFor } from '@testing-library/react';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
import Block from './Block';
import * as API from '../../API';

let mock = new MockAdapter(axios);

vi.mock("../../util/stores");

let mockUseParams = vi.fn();
Expand All @@ -20,12 +16,14 @@ vi.mock('react-router-dom', async () => {
};
});

const experimentObj = {
id: 24, slug: 'test', name: 'Test', playlists: [{ id: 42, name: 'TestPlaylist' }],
next_round: [{ view: 'INFO', button_label: 'Continue' }]
const blockObj = {
id: 24, slug: 'test', name: 'Test',
playlists: [{ id: 42, name: 'TestPlaylist' }],
session_id: 42,
loadingText: 'Patience!'
};

const nextRoundObj = { next_round: [{ view: 'EXPLAINER', instruction: 'Instruction' }] };
const nextRoundObj = { next_round: [{ view: 'EXPLAINER', instruction: 'Instruction', title: 'Some title' }] };

const mockSessionStore = { id: 1 };
const mockParticipantStore = {
Expand All @@ -36,12 +34,19 @@ const mockParticipantStore = {
country: 'nl',
};

vi.mock('../../API', () => ({
useBlock: () => [Promise.resolve(blockObj), false],
getNextRound: () => Promise.resolve(nextRoundObj)
}));


vi.mock('../../util/stores', () => ({
__esModule: true,
default: (fn) => {
const state = {
session: mockSessionStore,
participant: mockParticipantStore,
setError: vi.fn(),
setSession: vi.fn(),
setHeadData: vi.fn(),
resetHeadData: vi.fn(),
Expand All @@ -60,23 +65,23 @@ describe('Block Component', () => {
});

afterEach(() => {
mock.reset();
vi.clearAllMocks();
});

// fix/remove this implementation after merging #810
test('renders with given props', async () => {
mock.onGet().replyOnce(200, experimentObj);
it('renders with given props', async () => {
// Mock the useParticipantLink hook

render(
<MemoryRouter>
<Block />
</MemoryRouter>
);
await screen.findByText('Continue');
await screen.findByText('Instruction');
});

test('calls onNext', async () => {
mock.onGet().replyOnce(200, experimentObj);
it('calls onNext', async () => {
const spy = vi.spyOn(API, 'getNextRound');
spy.mockImplementationOnce(() => Promise.resolve(nextRoundObj))

render(
<MemoryRouter initialEntries={['/block/test']}>
Expand All @@ -85,9 +90,6 @@ describe('Block Component', () => {
</Routes>
</MemoryRouter>
);
const button = await screen.findByText('Continue');
fireEvent.click(button);
mock.onGet().replyOnce(200, nextRoundObj);
await waitFor(() => expect(spy).toHaveBeenCalled());
});

Expand Down
54 changes: 13 additions & 41 deletions frontend/src/components/Block/Block.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import classNames from "classnames";

import useBoundStore from "@/util/stores";
import { getNextRound, useBlock } from "@/API";
import Consent from "@/components/Consent/Consent";
import DefaultPage from "@/components/Page/DefaultPage";
import Explainer from "@/components/Explainer/Explainer";
import Final from "@/components/Final/Final";
Expand All @@ -19,19 +18,17 @@ import UserFeedback from "@/components/UserFeedback/UserFeedback";
import FontLoader from "@/components/FontLoader/FontLoader";
import useResultHandler from "@/hooks/useResultHandler";
import Session from "@/types/Session";
import { PlaybackArgs, PlaybackView } from "@/types/Playback";
import { PlaybackArgs } from "@/types/Playback";
import { FeedbackInfo, Step } from "@/types/Block";
import { TrialConfig } from "@/types/Trial";
import Social from "@/types/Social";

type BlockView = PlaybackView | "TRIAL_VIEW" | "EXPLAINER" | "SCORE" | "FINAL" | "PLAYLIST" | "LOADING" | "CONSENT" | "INFO" | "REDIRECT";
type BlockView = "TRIAL_VIEW" | "EXPLAINER" | "SCORE" | "FINAL" | "PLAYLIST" | "LOADING" | "CONSENT" | "INFO" | "REDIRECT";

interface ActionProps {

view: BlockView;
title?: string;
url?: string;
next_round?: any[];

// Some views require additional data
button_label?: string;
Expand Down Expand Up @@ -79,11 +76,6 @@ interface ActionProps {
image: string;
link: string;
};

// Consent related
text?: string;
confirm?: string;
deny?: string;
}


Expand Down Expand Up @@ -128,33 +120,17 @@ const Block = () => {
setKey(Math.random());
}, []);

const updateActions = useCallback((currentActions) => {
const updateActions = useCallback((currentActions: []) => {
const newActions = currentActions;
setActions(newActions);
const newState = newActions.shift();
updateState(newState);
}, [updateState]);

/**
* @deprecated
*/
const checkSession = async (): Promise<Session | void> => {
if (session) {
return session;
}

if (block?.session_id) {
const newSession = { id: block.session_id };
setSession(newSession);
return newSession;
}
};

const continueToNextRound = async () => {
const thisSession = await checkSession();
const continueToNextRound = async (activeSession: Session) => {
// Try to get next_round data from server
const round = await getNextRound({
session: thisSession
session: activeSession
});
if (round) {
updateActions(round.next_round);
Expand All @@ -171,7 +147,7 @@ const Block = () => {
if (!doBreak && actions.length) {
updateActions(actions);
} else {
continueToNextRound();
continueToNextRound(session as Session);
}
};

Expand All @@ -181,7 +157,6 @@ const Block = () => {
if (!loadingBlock && participant) {
// Loading succeeded
if (block) {
setSession(null);
// Set Helmet Head data
setHeadData({
title: block.name,
Expand All @@ -197,6 +172,8 @@ const Block = () => {

if (block.session_id) {
setSession({ id: block.session_id });
} else if (!block.session_id && session) {
setError('Session could not be created');
}

// Set theme
Expand All @@ -206,11 +183,8 @@ const Block = () => {
setTheme(null);
}

if (block.next_round.length) {
updateActions([...block.next_round]);
} else {
setError("The first_round array from the ruleset is empty")
}
continueToNextRound({ id: block.session_id });

} else {
// Loading error
setError("Could not load block");
Expand Down Expand Up @@ -273,8 +247,6 @@ const Block = () => {
return <Playlist key={key} {...attrs} />;
case "LOADING":
return <Loading key={key} {...attrs} />;
case "CONSENT":
return <Consent key={key} {...attrs} />;
case "INFO":
return <Info key={key} {...attrs} />;
case "REDIRECT":
Expand Down Expand Up @@ -303,10 +275,10 @@ const Block = () => {
className={classNames(
"aha__block",
!loadingBlock && block
? "experiment-" + block.slug
? "block-" + block.slug
: ""
)}
data-testid="experiment-wrapper"
data-testid="block-wrapper"
>
<CSSTransition
timeout={{ enter: 300, exit: 0 }}
Expand Down Expand Up @@ -340,7 +312,7 @@ const Block = () => {
</DefaultPage>
) : (
<div className="loader-container">
<Loading />
<Loading loadingText={loadingText} />
</div>
)}

Expand Down
Loading
Loading