From 6156a186958b845da2194f06138af97edb3b5222 Mon Sep 17 00:00:00 2001 From: BeritJanssen Date: Tue, 27 Aug 2024 08:45:18 +0200 Subject: [PATCH 01/11] remove next_round dependency in block API call --- frontend/src/components/Block/Block.tsx | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/frontend/src/components/Block/Block.tsx b/frontend/src/components/Block/Block.tsx index ce59050b9..aa0b3569c 100644 --- a/frontend/src/components/Block/Block.tsx +++ b/frontend/src/components/Block/Block.tsx @@ -30,7 +30,6 @@ interface BlockState { view: BlockView; title?: string; url?: string; - next_round?: any[]; // Some views require additional data button_label?: string; @@ -148,10 +147,9 @@ const Block = () => { }; const continueToNextRound = async () => { - const thisSession = await checkSession(); // Try to get next_round data from server const round = await getNextRound({ - session: thisSession + session: session }); if (round) { updateActions(round.next_round); @@ -178,7 +176,6 @@ const Block = () => { if (!loadingBlock && participant) { // Loading succeeded if (block) { - setSession(null); // Set Helmet Head data setHeadData({ title: block.name, @@ -194,6 +191,8 @@ const Block = () => { if (block.session_id) { setSession({ id: block.session_id }); + } else if (!block.session_id && session) { + setSession(null); } // Set theme @@ -203,11 +202,7 @@ const Block = () => { setTheme(null); } - if (block.next_round.length) { - updateActions([...block.next_round]); - } else { - setError("The first_round array from the ruleset is empty") - } + } else { // Loading error setError("Could not load block"); From 363bed1dbab5ee683765014e45a20968363ec331 Mon Sep 17 00:00:00 2001 From: BeritJanssen Date: Tue, 27 Aug 2024 10:02:50 +0200 Subject: [PATCH 02/11] fix: pass session to Block.continueToNextRound function --- frontend/src/components/Block/Block.tsx | 26 ++++++------------------- 1 file changed, 6 insertions(+), 20 deletions(-) diff --git a/frontend/src/components/Block/Block.tsx b/frontend/src/components/Block/Block.tsx index aa0b3569c..1970d97d5 100644 --- a/frontend/src/components/Block/Block.tsx +++ b/frontend/src/components/Block/Block.tsx @@ -124,32 +124,17 @@ const Block = () => { setState({ ...state }); }, []); - 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 => { - if (session) { - return session; - } - - if (block?.session_id) { - const newSession = { id: block.session_id }; - setSession(newSession); - return newSession; - } - }; - - const continueToNextRound = async () => { + const continueToNextRound = async (activeSession: Session) => { // Try to get next_round data from server const round = await getNextRound({ - session: session + session: activeSession }); if (round) { updateActions(round.next_round); @@ -166,7 +151,7 @@ const Block = () => { if (!doBreak && actions.length) { updateActions(actions); } else { - continueToNextRound(); + continueToNextRound(session as Session); } }; @@ -192,7 +177,7 @@ const Block = () => { if (block.session_id) { setSession({ id: block.session_id }); } else if (!block.session_id && session) { - setSession(null); + setError('Session could not be created'); } // Set theme @@ -202,6 +187,7 @@ const Block = () => { setTheme(null); } + continueToNextRound({ id: block.session_id }); } else { // Loading error From 040dc4c048a00581b947a5094e6d489d62c3f21a Mon Sep 17 00:00:00 2001 From: BeritJanssen Date: Tue, 27 Aug 2024 10:03:26 +0200 Subject: [PATCH 03/11] refactor: remove next_round array from experiment.views.get_block --- backend/experiment/views.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/backend/experiment/views.py b/backend/experiment/views.py index 381fc8d8c..ba5f0010c 100644 --- a/backend/experiment/views.py +++ b/backend/experiment/views.py @@ -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, } From 34924940548ec5fd7bb8624db99f7233cffb972b Mon Sep 17 00:00:00 2001 From: BeritJanssen Date: Tue, 27 Aug 2024 13:44:33 +0200 Subject: [PATCH 04/11] remove Consent from Block component; remove PlaybackView type (always wrapped in Trial) --- frontend/src/components/Block/Block.tsx | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/frontend/src/components/Block/Block.tsx b/frontend/src/components/Block/Block.tsx index 98a1d256a..d526c3140 100644 --- a/frontend/src/components/Block/Block.tsx +++ b/frontend/src/components/Block/Block.tsx @@ -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"; @@ -19,15 +18,14 @@ 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; @@ -78,11 +76,6 @@ interface ActionProps { image: string; link: string; }; - - // Consent related - text?: string; - confirm?: string; - deny?: string; } @@ -254,8 +247,6 @@ const Block = () => { return ; case "LOADING": return ; - case "CONSENT": - return ; case "INFO": return ; case "REDIRECT": From 4c5a48e5b998b94b066603788dff7effa66a227e Mon Sep 17 00:00:00 2001 From: BeritJanssen Date: Tue, 27 Aug 2024 14:00:36 +0200 Subject: [PATCH 05/11] fix beat_alignment unit test --- .../experiment/rules/tests/test_beat_alignment.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/backend/experiment/rules/tests/test_beat_alignment.py b/backend/experiment/rules/tests/test_beat_alignment.py index b3b208098..d40b0548a 100644 --- a/backend/experiment/rules/tests/test_beat_alignment.py +++ b/backend/experiment/rules/tests/test_beat_alignment.py @@ -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) @@ -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'] From a23f1383f98d65545ac2a0a6df2eeabf3036032d Mon Sep 17 00:00:00 2001 From: BeritJanssen Date: Tue, 27 Aug 2024 16:09:13 +0200 Subject: [PATCH 06/11] fix Block unit test --- frontend/src/components/Block/Block.test.tsx | 42 ++++++++++---------- frontend/src/components/Block/Block.tsx | 6 +-- 2 files changed, 25 insertions(+), 23 deletions(-) diff --git a/frontend/src/components/Block/Block.test.tsx b/frontend/src/components/Block/Block.test.tsx index 7be132e69..7f5a2081a 100644 --- a/frontend/src/components/Block/Block.test.tsx +++ b/frontend/src/components/Block/Block.test.tsx @@ -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(); @@ -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 = { @@ -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(), @@ -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( ); - 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( @@ -85,9 +90,6 @@ describe('Block Component', () => { ); - const button = await screen.findByText('Continue'); - fireEvent.click(button); - mock.onGet().replyOnce(200, nextRoundObj); await waitFor(() => expect(spy).toHaveBeenCalled()); }); diff --git a/frontend/src/components/Block/Block.tsx b/frontend/src/components/Block/Block.tsx index d526c3140..e10ff9b58 100644 --- a/frontend/src/components/Block/Block.tsx +++ b/frontend/src/components/Block/Block.tsx @@ -275,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" > { ) : (
- +
)} From 2dc84222bd1edc47c707540372e23760704f575e Mon Sep 17 00:00:00 2001 From: BeritJanssen Date: Tue, 27 Aug 2024 16:09:27 +0200 Subject: [PATCH 07/11] adjust types --- frontend/src/API.ts | 13 +++++++------ frontend/src/types/Block.ts | 9 +++++++-- 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/frontend/src/API.ts b/frontend/src/API.ts index 80bce25a7..7e49e3195 100644 --- a/frontend/src/API.ts +++ b/frontend/src/API.ts @@ -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"; @@ -43,8 +44,8 @@ export const URLS = { } }; -export const useBlock = (slug: string): [ExtendedBlock | null, boolean] => - useGet(API_BASE_URL + URLS.block.get(slug)); +export const useBlock = (slug: string): [IBlock | null, boolean] => + useGet(API_BASE_URL + URLS.block.get(slug)); export const useExperiment = (slug: string) => { const data = useGet(API_BASE_URL + URLS.experiment.get(slug)); @@ -63,19 +64,19 @@ export const useConsent = (slug: string) => useGet(API_BASE_URL + URLS.result.get('consent_' + slug)); interface CreateConsentParams { - block: Block; + experiment: IExperiment; participant: Pick; } /** 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, } ), diff --git a/frontend/src/types/Block.ts b/frontend/src/types/Block.ts index 73554636c..1f79baaf9 100644 --- a/frontend/src/types/Block.ts +++ b/frontend/src/types/Block.ts @@ -8,6 +8,13 @@ export default interface Block { description: string; image?: IImage; bonus_points: number; + theme?: Theme; + class_name: string; + rounds: number; + playlists: Playlist[]; + feedback_info: FeedbackInfo; + session_id: number; + loading_text: string; } export interface Playlist { @@ -36,6 +43,4 @@ export interface ExtendedBlock extends Block { feedback_info: FeedbackInfo; session_id: number; loading_text: string; - timer?: number; - steps?: Step[]; } From 64d40ee9d61f0418e290376d76c619e0e4084d5e Mon Sep 17 00:00:00 2001 From: BeritJanssen Date: Tue, 27 Aug 2024 16:09:36 +0200 Subject: [PATCH 08/11] rename block->experiment in Experiment component --- frontend/src/components/Experiment/Experiment.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/components/Experiment/Experiment.tsx b/frontend/src/components/Experiment/Experiment.tsx index 9384f9805..c67d3892a 100644 --- a/frontend/src/components/Experiment/Experiment.tsx +++ b/frontend/src/components/Experiment/Experiment.tsx @@ -55,7 +55,7 @@ const Experiment = () => { const attrs = { participant, onNext, - block: experiment, + experiment: experiment, ...experiment.consent, } return ( From 24939cedb5140c26b38a29d8abf4c00739c7a5cf Mon Sep 17 00:00:00 2001 From: BeritJanssen Date: Tue, 27 Aug 2024 16:10:02 +0200 Subject: [PATCH 09/11] adjust Consent component and test to expect Experiment, not Block object --- .../src/components/Consent/Consent.test.tsx | 19 ++++++++++--------- frontend/src/components/Consent/Consent.tsx | 12 ++++++------ 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/frontend/src/components/Consent/Consent.test.tsx b/frontend/src/components/Consent/Consent.test.tsx index 153874fff..653a46ccb 100644 --- a/frontend/src/components/Consent/Consent.test.tsx +++ b/frontend/src/components/Consent/Consent.test.tsx @@ -20,15 +20,15 @@ vi.mock('../../API', () => ({ useConsent: vi.fn(), })); -const mockBlock = { +const mockExperiment = { slug: 'test-experiment', - loading_text: 'Loading...', + name: 'Test' }; const getConsentProps: (overrides?: Partial) => ConsentProps = (overrides) => ({ title: 'Consent', text: '

Consent Text

', - block: mockBlock, + experiment: mockExperiment, participant: { csrf_token: '42' }, onNext: vi.fn(), confirm: 'Agree', @@ -37,15 +37,16 @@ const getConsentProps: (overrides?: Partial) => ConsentProps = (ov }); describe('Consent', () => { - it('renders loading state correctly', () => { + it('renders circle while loading', () => { (useConsent as Mock).mockReturnValue([null, true]); // Mock loading state - const { getByText } = render(); - expect(document.body.contains(getByText('Loading...'))).toBe(true); + const { container } = render(); + + expect(document.body.contains(container.querySelector('.aha__circle'))).toBe(true); }); it('renders consent text when not loading', () => { (useConsent as Mock).mockReturnValue([null, false]); - const { getByText } = render(Consent Text

', block: { slug: 'test-experiment', loading_text: 'Loading...' } })} />); + const { getByText } = render(Consent Text

', experiment: mockExperiment })} />); expect(document.body.contains(getByText('Consent Text'))).toBe(true); }); @@ -53,7 +54,7 @@ describe('Consent', () => { it('calls onNext when Agree button is clicked', async () => { (useConsent as Mock).mockReturnValue([null, false]); const onNext = vi.fn(); - const { getByText } = render(); + const { getByText } = render(); fireEvent.click(getByText('Agree')); await waitFor(() => expect(onNext).toHaveBeenCalled()); @@ -70,7 +71,7 @@ describe('Consent', () => { it('auto advances if consent is already given', () => { (useConsent as Mock).mockReturnValue([true, false]); const onNext = vi.fn(); - render(); + render(); expect(onNext).toHaveBeenCalled(); }); diff --git a/frontend/src/components/Consent/Consent.tsx b/frontend/src/components/Consent/Consent.tsx index 8f0a77bb3..4b7071cfe 100644 --- a/frontend/src/components/Consent/Consent.tsx +++ b/frontend/src/components/Consent/Consent.tsx @@ -11,16 +11,16 @@ import Participant from "@/types/Participant"; export interface ConsentProps { title: string; text: string; - block: any; + experiment: any; participant: Pick; onNext: () => void; confirm: string; deny: string; } -/** Consent is an block view that shows the consent text, and handles agreement/stop actions */ -const Consent = ({ title, text, block, participant, onNext, confirm, deny }: ConsentProps) => { - const [consent, loadingConsent] = useConsent(block.slug); +/** Consent is an experiment view that shows the consent text, and handles agreement/stop actions */ +const Consent = ({ title, text, experiment, participant, onNext, confirm, deny }: ConsentProps) => { + const [consent, loadingConsent] = useConsent(experiment.slug); const urlQueryString = window.location.search; // Listen for consent, and auto advance if already given @@ -33,7 +33,7 @@ const Consent = ({ title, text, block, participant, onNext, confirm, deny }: Con // Click on agree button const onAgree = async () => { // Store consent - await createConsent({ block, participant }); + await createConsent({ experiment, participant }); // Next! onNext(); @@ -49,7 +49,7 @@ const Consent = ({ title, text, block, participant, onNext, confirm, deny }: Con // Loader in case consent is being loaded // or it was already given if (loadingConsent || consent) { - return ; + return ; } // Calculate height for consent text to prevent overlapping browser chrome From eef4cbf30c46e4dbacbd7a173386e637304e96b6 Mon Sep 17 00:00:00 2001 From: BeritJanssen Date: Mon, 2 Sep 2024 09:12:57 +0200 Subject: [PATCH 10/11] remove ExtendedBlock --- frontend/src/components/Playlist/Playlist.tsx | 4 ++-- frontend/src/types/Block.ts | 10 ---------- 2 files changed, 2 insertions(+), 12 deletions(-) diff --git a/frontend/src/components/Playlist/Playlist.tsx b/frontend/src/components/Playlist/Playlist.tsx index 5dab5120d..9a7142332 100644 --- a/frontend/src/components/Playlist/Playlist.tsx +++ b/frontend/src/components/Playlist/Playlist.tsx @@ -1,8 +1,8 @@ -import { ExtendedBlock } from "@/types/Block"; +import Block from "@/types/Block"; import { MutableRefObject, useEffect } from "react"; interface PlaylistProps { - block: ExtendedBlock; + block: Block; instruction: string; onNext: () => void; playlist: MutableRefObject; diff --git a/frontend/src/types/Block.ts b/frontend/src/types/Block.ts index 1f79baaf9..31d734343 100644 --- a/frontend/src/types/Block.ts +++ b/frontend/src/types/Block.ts @@ -34,13 +34,3 @@ export interface Step { id: number; description: string; } - -export interface ExtendedBlock extends Block { - theme?: Theme; - class_name: string; - rounds: number; - playlists: Playlist[]; - feedback_info: FeedbackInfo; - session_id: number; - loading_text: string; -} From a96347cc37c8cb3b793b044df492ff217a8263f3 Mon Sep 17 00:00:00 2001 From: Berit Date: Mon, 2 Sep 2024 15:21:32 +0200 Subject: [PATCH 11/11] Update frontend/src/components/Experiment/Experiment.tsx Co-authored-by: Drikus Roor --- frontend/src/components/Experiment/Experiment.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontend/src/components/Experiment/Experiment.tsx b/frontend/src/components/Experiment/Experiment.tsx index c67d3892a..e460f8029 100644 --- a/frontend/src/components/Experiment/Experiment.tsx +++ b/frontend/src/components/Experiment/Experiment.tsx @@ -55,7 +55,7 @@ const Experiment = () => { const attrs = { participant, onNext, - experiment: experiment, + experiment, ...experiment.consent, } return (