diff --git a/.github/workflows/storybook.yml b/.github/workflows/docs.yml similarity index 69% rename from .github/workflows/storybook.yml rename to .github/workflows/docs.yml index 49222b758..94564d7fe 100644 --- a/.github/workflows/storybook.yml +++ b/.github/workflows/docs.yml @@ -1,4 +1,4 @@ -name: Build & Publish Storybook +name: Build & Publish Docs & Storybook on: push: @@ -7,13 +7,13 @@ on: - develop paths: - 'frontend/**' - - '.github/workflows/storybook.yml' + - '.github/workflows/docs.yml' - '.yarn/**' - '.storybook/**' pull_request: paths: - 'frontend/**' - - '.github/workflows/storybook.yml' + - '.github/workflows/docs.yml' - '.yarn/**' - '.storybook/**' @@ -27,23 +27,34 @@ jobs: runs-on: ubuntu-latest steps: - uses: actions/checkout@v4 + + - name: Build ER/MKDOCS site + run: | + mkdir -p docs-pages + sudo docker compose --env-file .env-github-actions run server bash -c "mkdocs build -d ./docs-pages-build" + cp -r ./backend/docs-pages-build/* ./docs-pages + - name: Install dependencies run: yarn working-directory: ./frontend + - name: Build Storybook - run: yarn storybook:build + run: yarn storybook:build --output-dir ../docs-pages/storybook working-directory: ./frontend + - name: Setup Github Pages uses: actions/configure-pages@v2 + - name: Upload artifact uses: actions/upload-pages-artifact@v1 with: - path: ./frontend/storybook-static + path: ./docs-pages + - name: Archive production artifacts uses: actions/upload-artifact@v3 with: name: build - path: ./frontend/storybook-static + path: ./docs-pages deploy-gh-pages: # only deploy on develop branch diff --git a/backend/experiment/actions/trial.py b/backend/experiment/actions/trial.py index 5e6b27627..edcc03c8c 100644 --- a/backend/experiment/actions/trial.py +++ b/backend/experiment/actions/trial.py @@ -15,27 +15,24 @@ class Trial(BaseAction): # pylint: disable=too-few-public-methods - playback: player(s) to be displayed in this view - feedback_form: array of form elements - title: page title - defaults to empty - - result_id: optional, a result_id with which the whole Trial is associated """ - ID = 'TRIAL_VIEW' + ID = "TRIAL_VIEW" def __init__( - self, - playback: Playback = None, - html: str = None, - feedback_form: Form = None, - title='', - config: dict = None, - result_id: int = None, - style: FrontendStyle = FrontendStyle() - ): - ''' + self, + playback: Playback = None, + html: str = None, + feedback_form: Form = None, + title="", + config: dict = None, + style: FrontendStyle = FrontendStyle(), + ): + """ - playback: Playback object (may be None) - html: HTML object (may be None) - feedback_form: Form object (may be None) - title: string setting title in header of experiment - - result_id: id of Result to handle (especially important if there's no feedback form) - config: dictionary with following settings - response_time: how long to wait until stopping the player / proceeding to the next view - auto_advance: proceed to next view after player has stopped @@ -47,18 +44,17 @@ def __init__( - neutral-inverted: first element is yellow, second is blue, third is teal - boolean: first element is green, second is red - boolean-negative-first: first element is red, second is green - ''' + """ self.playback = playback self.html = html self.feedback_form = feedback_form self.title = title - self.result_id = result_id self.config = { - 'response_time': 5, - 'auto_advance': False, - 'listen_first': False, - 'show_continue_button': True, - 'continue_label': _('Continue'), + "response_time": 5, + "auto_advance": False, + "listen_first": False, + "show_continue_button": True, + "continue_label": _("Continue"), } if config: self.config.update(config) @@ -71,18 +67,17 @@ def action(self): """ # Create action action = { - 'view': Trial.ID, - 'title': self.title, - 'config': self.config, - 'result_id': self.result_id, + "view": Trial.ID, + "title": self.title, + "config": self.config, } if self.style: - action['style'] = self.style.to_dict() + action["style"] = self.style.to_dict() if self.playback: - action['playback'] = self.playback.action() + action["playback"] = self.playback.action() if self.html: - action['html'] = self.html.action() + action["html"] = self.html.action() if self.feedback_form: - action['feedback_form'] = self.feedback_form.action() + action["feedback_form"] = self.feedback_form.action() return action diff --git a/backend/result/utils.py b/backend/result/utils.py index 8d47b2c60..bea304683 100644 --- a/backend/result/utils.py +++ b/backend/result/utils.py @@ -6,7 +6,7 @@ def get_result(session, data): - result_id = data.get('result_id') + result_id = data.get("result_id") try: result = Result.objects.get(pk=result_id, session=session) except Result.DoesNotExist: @@ -23,12 +23,7 @@ def handle_results(data, session): if the given_result is an array of results, retrieve and save results for all of them else, handle results at top level """ - try: - form = data.pop('form') - except KeyError: - # no form, handle results at top level - result = score_result(data, session) - return result + form = data.pop("form") for form_element in form: result = get_result(session, form_element) # save relevant data such as config and decision time (except for the popped form) @@ -39,26 +34,23 @@ def handle_results(data, session): def prepare_profile_result(question_key, participant, **kwargs): - ''' Create a Result object, and provide its id to be serialized + """Create a Result object, and provide its id to be serialized - question_key: the key of the question in the questionnaire dictionaries - participant: the participant on which the Result is going to be registered possible kwargs: - expected_response: optionally, provide the correct answer, used for scoring - comment: optionally, provide a comment to be saved in the database - scoring_rule: optionally, provide a scoring rule - ''' - scoring_rule = PROFILE_SCORING_RULES.get(question_key, '') + """ + scoring_rule = PROFILE_SCORING_RULES.get(question_key, "") result, created = Result.objects.get_or_create( - question_key=question_key, - participant=participant, - scoring_rule=scoring_rule, - **kwargs + question_key=question_key, participant=participant, scoring_rule=scoring_rule, **kwargs ) return result def prepare_result(question_key: str, session: Session, **kwargs) -> int: - ''' Create a Result object, and provide its id to be serialized + """Create a Result object, and provide its id to be serialized - question_key: the key of the question in the questionnaire dictionaries - session: the session on which the Result is going to be registered possible kwargs: @@ -67,12 +59,8 @@ def prepare_result(question_key: str, session: Session, **kwargs) -> int: - json_data: optionally, provide json data tied to this result - comment: optionally, provide a comment to be saved in the database, e.g. "training phase" - scoring_rule: optionally, provide a scoring rule - ''' - result = Result.objects.create( - question_key=question_key, - session=session, - **kwargs - ) + """ + result = Result.objects.create(question_key=question_key, session=session, **kwargs) return result.id @@ -90,7 +78,7 @@ def score_result(data, session): """ result = get_result(session, data) result.save_json_data(data) - result.given_response = data.get('value') + result.given_response = data.get("value") # Calculate score: by default, apply a scoring rule # Can be overridden by defining calculate_score in the rules file if result.session: diff --git a/backend/session/models.py b/backend/session/models.py index 08f31338f..27066c62c 100644 --- a/backend/session/models.py +++ b/backend/session/models.py @@ -7,18 +7,12 @@ class Session(models.Model): """Experiment session by a participant""" - block = models.ForeignKey( - "experiment.Block", on_delete=models.CASCADE, blank=True, null=True - ) + block = models.ForeignKey("experiment.Block", on_delete=models.CASCADE, blank=True, null=True) participant = models.ForeignKey("participant.Participant", on_delete=models.CASCADE) - playlist = models.ForeignKey( - "section.Playlist", on_delete=models.SET_NULL, blank=True, null=True - ) + playlist = models.ForeignKey("section.Playlist", on_delete=models.SET_NULL, blank=True, null=True) started_at = models.DateTimeField(db_index=True, default=timezone.now) - finished_at = models.DateTimeField( - db_index=True, default=None, null=True, blank=True - ) + finished_at = models.DateTimeField(db_index=True, default=None, null=True, blank=True) json_data = models.JSONField(default=dict, blank=True, null=True) final_score = models.FloatField(db_index=True, default=0.0) current_round = models.IntegerField(default=1) @@ -35,9 +29,7 @@ def result_count(self): def total_score(self): """Sum of all result scores""" score = self.result_set.aggregate(models.Sum("score")) - return self.block.bonus_points + ( - score["score__sum"] if score["score__sum"] else 0 - ) + return self.block.bonus_points + (score["score__sum"] if score["score__sum"] else 0) def last_score(self): """Get last score, or return 0 if no scores are set""" @@ -49,7 +41,9 @@ def last_score(self): def last_result(self): """Get last result""" - return self.result_set.last() + result = self.result_set.last() + + return Result.objects.get(pk=result.id) def last_song(self): """Return artist and name of previous song, @@ -122,8 +116,9 @@ def get_used_song_ids(self, exclude={}): def get_unused_song_ids(self, filter_by={}): """Get a list of unused song ids from this session's playlist""" # Get all song ids from the current playlist - song_ids = self.playlist.section_set.filter( - **filter_by).order_by('song').values_list('song_id', flat=True).distinct() + song_ids = ( + self.playlist.section_set.filter(**filter_by).order_by("song").values_list("song_id", flat=True).distinct() + ) # Get all song ids from results used_song_ids = self.get_used_song_ids() return list(set(song_ids) - set(used_song_ids)) @@ -162,4 +157,4 @@ def get_previous_result(self, question_keys: list = []) -> Result: results = self.result_set if question_keys: results = results.filter(question_key__in=question_keys) - return results.order_by('-created_at').first() + return results.order_by("-created_at").first() diff --git a/frontend/.storybook/main.js b/frontend/.storybook/main.js index 90fbf71bf..9da30e2da 100644 --- a/frontend/.storybook/main.js +++ b/frontend/.storybook/main.js @@ -30,6 +30,7 @@ const config = { viteFinal: (config) => { return mergeConfig(config, { + base: "/MUSCLE/storybook/", resolve: { alias: { '@/': '/src/', diff --git a/frontend/src/components/Block/Block.tsx b/frontend/src/components/Block/Block.tsx index da40b9a8e..b66a4bb15 100644 --- a/frontend/src/components/Block/Block.tsx +++ b/frontend/src/components/Block/Block.tsx @@ -169,8 +169,6 @@ const Block = () => { const onResult = useResultHandler({ session, participant, - onNext, - state, }); // Render block state diff --git a/frontend/src/components/FeedbackForm/FeedbackForm.tsx b/frontend/src/components/FeedbackForm/FeedbackForm.tsx index 35a8018b4..8dab41173 100644 --- a/frontend/src/components/FeedbackForm/FeedbackForm.tsx +++ b/frontend/src/components/FeedbackForm/FeedbackForm.tsx @@ -12,6 +12,7 @@ interface FeedbackFormProps { skipLabel: string; isSkippable: boolean; onResult: OnResultType + onNext: () => void; emphasizeTitle?: boolean; } @@ -23,6 +24,7 @@ const FeedbackForm = ({ skipLabel, isSkippable, onResult, + onNext, emphasizeTitle = false, }: FeedbackFormProps) => { const isSubmitted = useRef(false); @@ -31,7 +33,7 @@ const FeedbackForm = ({ const [formValid, setFormValid] = useState(false); - const onSubmit = () => { + const onSubmit = async () => { // Prevent double submit if (isSubmitted.current) { return; @@ -39,9 +41,11 @@ const FeedbackForm = ({ isSubmitted.current = true; // Callback onResult with question data - onResult({ + await onResult({ form, }); + + onNext(); }; const onChange = (value: string | number | boolean, question_index: number) => { diff --git a/frontend/src/components/Question/_ButtonArray.test.tsx b/frontend/src/components/Question/_ButtonArray.test.tsx index 8f22a6472..191a1bde8 100644 --- a/frontend/src/components/Question/_ButtonArray.test.tsx +++ b/frontend/src/components/Question/_ButtonArray.test.tsx @@ -10,7 +10,6 @@ const getProps = (overrides = {}) => ({ "view": QuestionViews.BUTTON_ARRAY, "explainer": "", "question": "1. Do you know this song?", - "result_id": 12345, "is_skippable": false, "submits": false, "style": "boolean", diff --git a/frontend/src/components/Trial/Trial.test.tsx b/frontend/src/components/Trial/Trial.test.tsx index 9db8e89e5..1c370c490 100644 --- a/frontend/src/components/Trial/Trial.test.tsx +++ b/frontend/src/components/Trial/Trial.test.tsx @@ -13,8 +13,8 @@ vi.mock("../Playback/Playback", () => ({ )), })); vi.mock("../FeedbackForm/FeedbackForm", () => ({ - default: vi.fn(({ onResult }) => ( -
onResult({ type: 'feedback' })}>Mock Feedback Form
+ default: vi.fn(({ onResult, onNext }) => ( +
{ onResult(); onNext(); }}>Mock Feedback Form
)), })); vi.mock("../HTML/HTML", () => ({ @@ -131,7 +131,7 @@ describe('Trial', () => { it("calls finishedPlaying when Playback component finishes", () => { const config = { ...defaultConfig, auto_advance: true }; render( { render(); fireEvent.click(screen.getByTestId('mock-playback')); await waitFor(() => { expect(mockOnResult).toHaveBeenCalled(); expect(mockOnResult).toHaveBeenCalledWith( - expect.objectContaining({ result: { type: 'time_passed' }, result_id: "123" }), + expect.objectContaining({ + decision_time: expect.any(Number), + form: expect.arrayContaining([ + expect.objectContaining({ + key: 'test_question', + value: 'TIMEOUT' + }) + ]) + }) ); }); }); @@ -197,28 +205,4 @@ describe('Trial', () => { expect(mockOnNext).toHaveBeenCalled(); }); }); - - it("calls only onResult when form is not defined and break_round_on is NOT met", async () => { - const formless_feedback_form = { - ...feedback_form, - form: undefined - }; - const config = { - ...defaultConfig, - break_round_on: { EQUALS: ['slow'] } - }; - render(); - fireEvent.click(screen.getByTestId('mock-feedback-form')); - - await waitFor(() => { - expect(mockOnResult).toHaveBeenCalled(); - }); - - expect(mockOnNext).not.toHaveBeenCalled(); - }); }); diff --git a/frontend/src/components/Trial/Trial.tsx b/frontend/src/components/Trial/Trial.tsx index 55cbc7d53..2ac79ef71 100644 --- a/frontend/src/components/Trial/Trial.tsx +++ b/frontend/src/components/Trial/Trial.tsx @@ -23,7 +23,6 @@ export interface TrialProps { html: { body: string | TrustedHTML }; feedback_form: IFeedbackForm; config: TrialConfig; - result_id: string; onNext: (breakRound?: boolean) => void; onResult: OnResultType; } @@ -41,7 +40,6 @@ const Trial = (props: TrialProps) => { html, feedback_form, config, - result_id, onNext, onResult, } = props; @@ -62,32 +60,22 @@ const Trial = (props: TrialProps) => { // Create result data const makeResult = useCallback( - async (result: { type: 'time_passed' }) => { + async (hasTimedOut: boolean) => { // Prevent multiple submissions if (submitted.current) { return; } - submitted.current = true; if (!feedback_form) { - - if (result_id) { - onResult({ - result, - result_id - }); - } else { - onNext(); - } - - return; + return onNext(); } + const { form = [] } = feedback_form; - if (result.type === "time_passed") { + if (hasTimedOut) { form.map((formElement) => (formElement.value = "TIMEOUT")); } @@ -102,20 +90,14 @@ const Trial = (props: TrialProps) => { { decision_time: getAndStoreDecisionTime(), form, - config + config, }, - false, - // if we break the round, we don't want to call `onNext` in `onResult` - // as it does not allow us to pass a `breakRound` flag - !shouldBreakRound ); - if (shouldBreakRound) { - onNext(true); - } + return onNext(shouldBreakRound); }, - [feedback_form, config, onNext, onResult, result_id] + [feedback_form, config, onNext, onResult] ); const checkBreakRound = ( @@ -141,7 +123,7 @@ const Trial = (props: TrialProps) => { const getAndStoreDecisionTime = () => { const decisionTime = getTimeSince(startTime.current); // keep decisionTime in sessionStorage to be used by subsequent renders - window.sessionStorage.setItem('decisionTime', decisionTime); + window.sessionStorage.setItem('decisionTime', decisionTime.toString()); return decisionTime; } @@ -149,7 +131,6 @@ const Trial = (props: TrialProps) => { if (config.auto_advance) { - // Create a time_passed result if (config.auto_advance_timer != null) { if (playback.view === 'BUTTON') { @@ -157,14 +138,10 @@ const Trial = (props: TrialProps) => { } setTimeout(() => { - makeResult({ type: "time_passed", }); + makeResult(true); }, config.auto_advance_timer); } else { - - makeResult({ - type: "time_passed", - }); - + makeResult(true); } } setFormActive(true); @@ -198,7 +175,8 @@ const Trial = (props: TrialProps) => { buttonLabel={feedback_form.submit_label} skipLabel={feedback_form.skip_label} isSkippable={feedback_form.is_skippable} - onResult={makeResult} + onResult={onResult} + onNext={onNext} // emphasizeTitle={feedback_form.is_profile} // TODO: if we want left-aligned text with a pink divider, // make this style option available again (used in Question.scss) diff --git a/frontend/src/hooks/useResultHandler.test.ts b/frontend/src/hooks/useResultHandler.test.ts index 368a65d0d..0abecbfc5 100644 --- a/frontend/src/hooks/useResultHandler.test.ts +++ b/frontend/src/hooks/useResultHandler.test.ts @@ -1,40 +1,75 @@ import { act } from "react"; import { renderHook, } from "@testing-library/react"; import useResultHandler from "./useResultHandler"; -import { vi } from 'vitest'; +import { vi, beforeEach, describe, it, expect, } from 'vitest'; -import * as API from '@/API'; +import { scoreResult } from '@/API'; +import Session from "@/types/Session"; +import Participant from "@/types/Participant"; +import { QuestionViews } from "@/types/Question"; vi.mock('@/API'); describe('useResultHandler', () => { - const mockOnNext = vi.fn(); - const initialState = { next_round: ['round2'] }; // Example initial state - const mockSession = 'session-id'; - const mockParticipant = 'participant-id'; + const mockSession: Session = { id: 42 }; + const mockParticipant: Participant = { id: 13, hash: 'hash', csrf_token: 'token', participant_id_url: 'url', country: 'country' }; - it('buffers results correctly', async () => { + + beforeEach(() => { + vi.clearAllMocks(); + }); + + it('should call scoreResult with correct data', async () => { const { result } = renderHook(() => - useResultHandler({ session: mockSession, participant: mockParticipant, onNext: mockOnNext, state: initialState }) + useResultHandler({ + session: mockSession, + participant: mockParticipant, + }) ); + const mockResult = { + form: [{ id: 'q1', type: 'text', key: 'test_question', value: 'test', question: 'What is the average speed of a Swallow?', view: QuestionViews.BUTTON_ARRAY, choices: { 'slow': '1 km/h', 'fast': '42 km/h' } }], + decision_time: 1000, + config: { trialType: 'A' }, + }; + await act(async () => { - await result.current({ testResult: 'result1' }, false); - await result.current({ testResult: 'result2' }, false); + await result.current(mockResult); + }); + + expect(scoreResult).toHaveBeenCalledWith({ + session: mockSession, + participant: mockParticipant, + result: mockResult, }); + }); + + it('should not include section in data if not provided', async () => { + const { result } = renderHook(() => + useResultHandler({ + session: mockSession, + participant: mockParticipant, + }) + ); - expect(API.scoreResult).not.toHaveBeenCalled(); + const mockResult = { + result: { type: 'failure' }, + }; await act(async () => { - await result.current({ testResult: 'result3' }, true); + await result.current(mockResult); }); - expect(API.scoreResult).toHaveBeenCalledWith({ - session: 'session-id', - participant: 'participant-id', - result: { testResult: 'result3' }, + expect(scoreResult).toHaveBeenCalledWith({ + session: mockSession, + participant: mockParticipant, + result: mockResult, }); + expect(scoreResult).not.toHaveBeenCalledWith( + expect.objectContaining({ section: expect.anything() }) + ); }); + }); diff --git a/frontend/src/hooks/useResultHandler.ts b/frontend/src/hooks/useResultHandler.ts index 18eecf35b..8b371e19a 100644 --- a/frontend/src/hooks/useResultHandler.ts +++ b/frontend/src/hooks/useResultHandler.ts @@ -1,97 +1,45 @@ -import { useRef, useCallback } from "react"; +import { useCallback } from "react"; import { scoreResult } from "@/API"; import Session from "@/types/Session"; import Participant from "@/types/Participant"; import Question from "@/types/Question"; import { TrialConfig } from "@/types/Trial"; -interface ResultData { - session: Session; - participant: Participant; - result: unknown; - section?: unknown; -} - interface UseResultHandlerParams { session: Session; participant: Participant; - onNext: () => void; - state: any; } interface OnResultParams { - // If feedback form is provided - form?: Question[]; + form: Question[]; decision_time?: number; config?: TrialConfig - - // If no feedback form is provided - result?: { - type: string; - } - result_id?: string; } /** * useResult provides a reusable function to handle block view data - * - collect results in a buffer * - handles advancing to next round * - finally submits the data to the API and loads the new state */ -const useResultHandler = ({ session, participant, onNext, state }: UseResultHandlerParams) => { - const resultBuffer = useRef([]); +const useResultHandler = ({ session, participant }: UseResultHandlerParams) => { const onResult = useCallback( async ( result: OnResultParams, - forceSubmit = false, - goToNextAction = true ) => { - // Add data to result buffer - resultBuffer.current.push(result || {}); - - // Check if there is another round data available - // can be forced by forceSubmit - const hasNextRound = state && state.next_round && state.next_round.length; - if (hasNextRound && !forceSubmit && goToNextAction) { - onNext(); - return; - } - - // Merge result data with data from resultBuffer - // NB: result data with same properties will be overwritten by later results - const mergedResults = Object.assign( - {}, - ...resultBuffer.current, - result - ); // Create result data - const data: ResultData = { + const data = { session, participant, - result: mergedResults, + result, }; - // Optionally add section to result data - if (mergedResults.section) { - data.section = mergedResults.section; - } - // Send data to API await scoreResult(data); - - // Clear resultBuffer - resultBuffer.current = []; - - // Advance to next round - if (goToNextAction) { - onNext(); - } - }, - [participant, session, onNext, state] + [participant, session] ); return onResult; diff --git a/frontend/src/stories/ButtonArray.stories.jsx b/frontend/src/stories/ButtonArray.stories.jsx index d95895b27..eb61ec6ff 100644 --- a/frontend/src/stories/ButtonArray.stories.jsx +++ b/frontend/src/stories/ButtonArray.stories.jsx @@ -16,7 +16,7 @@ const defaultArgs = { value: "", choices: ["Choice 1", "Choice 2", "Choice 3"], }, - onChange: () => {}, + onChange: () => { }, id: 0, active: true, style: {}, @@ -94,7 +94,6 @@ export const CategorizationWithHiddenTextDisabled = { view: "BUTTON_ARRAY", explainer: "", question: "", - result_id: 16549, is_skippable: false, submits: true, style: { diff --git a/frontend/src/stories/Trial.stories.jsx b/frontend/src/stories/Trial.stories.jsx index 3fc03b402..e395a5081 100644 --- a/frontend/src/stories/Trial.stories.jsx +++ b/frontend/src/stories/Trial.stories.jsx @@ -51,7 +51,6 @@ const getDefaultArgs = (overrides = {}) => ({ view: "BUTTON_ARRAY", explainer: "", question: "1. Do you know this song?", - result_id: 17242, is_skippable: false, submits: false, style: "boolean", @@ -67,7 +66,6 @@ const getDefaultArgs = (overrides = {}) => ({ view: "ICON_RANGE", explainer: "", question: "2. How much do you like this song?", - result_id: 17241, is_skippable: false, submits: false, style: "gradient-7", @@ -202,7 +200,6 @@ export const ToontjeHoger4Absolute = { view: "BUTTON_ARRAY", explainer: "", question: "Welk fragment heeft de juiste toonhoogte?", - result_id: 3, is_skippable: false, submits: true, style: "neutral-inverted", diff --git a/frontend/src/types/Question.ts b/frontend/src/types/Question.ts index 0738ce5ca..63b5e20ed 100644 --- a/frontend/src/types/Question.ts +++ b/frontend/src/types/Question.ts @@ -26,4 +26,5 @@ export default interface Question { max_value?: number; max_length?: number; input_type?: 'number' | 'text'; + result_id?: number; }