Fixed: Fix play again URL rendering (#1051)

* fix: return play again url as a relative url instead of an absolute url

* fix: Render a Link or an anchor tag (<a>), depending on whether the url received from the Final action is relative or absolute

* fix: Render non-linkoneous button when button.link is nullish or an empty string

* fix: Fix conditional rendering logic & open absolute url in new tab

* story: Update stories for specific button/link types for Final component

* fix: Refactor Final component to use FinalButton for rendering buttons

* docs: Add comments to FinalButton component

backend/experiment/rules/base.py

@@ -5,12 +5,12 @@
 from django.conf import settings
 
 from experiment.actions import Final, Form, Trial
-from experiment.models import Experiment
 from section.models import Playlist
 from experiment.questions.demographics import DEMOGRAPHICS
 from experiment.questions.goldsmiths import MSI_OTHER
 from experiment.questions.utils import question_by_key, unanswered_questions
 from result.score import SCORING_RULES
+from session.models import Session
 
 from experiment.questions import get_questions_from_keys
 
@@ -52,7 +52,11 @@ def calculate_score(self, result, data):
         if scoring_rule:
             return scoring_rule(result, data)
         return None
-
+
+    def get_play_again_url(self, session: Session):
+        participant_id_url_param = f'?participant_id={session.participant.participant_id_url}' if session.participant.participant_id_url else ""
+        return f'/{session.experiment.slug}{participant_id_url_param}'
+
     def calculate_intermediate_score(self, session, result):
         """ process result data during a trial (i.e., between next_round calls) 
         return score

backend/experiment/rules/hooked.py

@@ -111,8 +111,10 @@ def next_round(self, session):
                     social=self.social_media_info(
                         session.experiment, total_score),
                     show_profile_link=True,
-                    button={'text': _('Play again'), 'link': '{}/{}'.format(
-                        settings.CORS_ORIGIN_WHITELIST[0], session.experiment.slug)}
+                    button={
+                        'text': _('Play again'),
+                        'link': self.get_play_again_url(session),
+                    }
                 )
             ]
 

backend/experiment/rules/matching_pairs.py

@@ -81,7 +81,7 @@ def next_round(self, session):
                 final_text='Can you score higher than your friends and family? Share and let them try!',
                 button={
                     'text': 'Play again',
-                    'link': f'/{session.experiment.slug}'
+                    'link': self.get_play_again_url(session)
                 },
                 rank=self.rank(session, exclude_unfinished=False),
                 social=social_info,

backend/experiment/rules/tests/test_base.py

@@ -1,6 +1,8 @@
 from django.test import TestCase
 from django.conf import settings
 from experiment.models import Experiment
+from session.models import Session
+from participant.models import Participant
 from section.models import Playlist
 from ..base import Base
 
@@ -29,6 +31,35 @@ def test_social_media_info(self):
         self.assertNotIn(social_media_info['url'], '//')
         self.assertEqual(social_media_info['hashtags'], ['music-lab', 'amsterdammusiclab', 'citizenscience'])
 
+    def test_get_play_again_url(self):
+        experiment = Experiment.objects.create(
+            name='Music Lab',
+            slug='music-lab',
+        )
+        session = Session.objects.create(
+            experiment=experiment,
+            participant=Participant.objects.create(),
+        )
+        base = Base()
+        play_again_url = base.get_play_again_url(session)
+        self.assertEqual(play_again_url, '/music-lab')
+
+    def test_get_play_again_url_with_participant_id(self):
+        experiment = Experiment.objects.create(
+            name='Music Lab',
+            slug='music-lab',
+        )
+        participant = Participant.objects.create(
+            participant_id_url='42',
+        )
+        session = Session.objects.create(
+            experiment=experiment,
+            participant=participant,
+        )
+        base = Base()
+        play_again_url = base.get_play_again_url(session)
+        self.assertEqual(play_again_url, '/music-lab?participant_id=42')
+
     def test_validate_playlist(self):
         base = Base()
         playlist = None

backend/experiment/rules/thats_my_song.py

@@ -80,8 +80,10 @@ def next_round(self, session):
                     rank=self.rank(session),
                     social=social_info,
                     show_profile_link=True,
-                    button={'text': _('Play again'), 'link': '{}/{}{}'.format(settings.CORS_ORIGIN_WHITELIST[0], session.experiment.slug,
-                        '?participant_id='+session.participant.participant_id_url if session.participant.participant_id_url else '')},
+                    button={
+                        'text': _('Play again'),
+                        'link': self.get_play_again_url(session)
+                    },
                     logo={'image': '/images/vumc_mcl_logo.png', 'link':'https://www.vumc.org/music-cognition-lab/welcome'}
                 )
             ]
@@ -153,4 +155,4 @@ def next_round(self, session):
                 actions.append(
                     self.next_heard_before_action(session))
 
-        return actions
+        return actions

backend/experiment/rules/visual_matching_pairs.py

@@ -55,7 +55,7 @@ def first_round(self, experiment):
             playlist,
             explainer
         ]
-    
+
     def next_round(self, session):
         if session.rounds_passed() < 1:       
             trials = self.get_questionnaire(session)
@@ -80,6 +80,7 @@ def next_round(self, session):
                 final_text='Can you score higher than your friends and family? Share and let them try!',
                 button={
                     'text': 'Play again',
+                    'link': self.get_play_again_url(session)
                 },
                 rank=self.rank(session, exclude_unfinished=False),
                 social=social_info,
@@ -88,7 +89,7 @@ def next_round(self, session):
             cont = self.get_visual_matching_pairs_trial(session)
 
             return [score, cont]
-    
+
     def get_visual_matching_pairs_trial(self, session):
 
         player_sections = list(session.playlist.section_set.filter(tag__contains='vmp'))
@@ -112,5 +113,5 @@ def calculate_score(self, result, data):
         for m in moves:
             m['filename'] = str(Section.objects.get(pk=m.get('selectedSection')).filename)
         score = data.get('result').get('score')
-        
-        return score
+
+        return score

frontend/src/components/Experiment/Experiment.jsx

@@ -93,7 +93,7 @@ const Experiment = ({ match }) => {
     };
 
     // trigger next action from next_round array, or call session/next_round
-    const onNext = async (doBreak) => {
+    const onNext = async (doBreak = false) => {
         if (!doBreak && actions.length) {
             updateActions(actions);
         } else {

frontend/src/components/Final/Final.jsx

@@ -1,5 +1,5 @@
 import React, { useState, useEffect, useRef } from "react";
-import { Link, withRouter } from "react-router-dom";
+import { withRouter } from "react-router-dom";
 
 import Rank from "../Rank/Rank";
 import Social from "../Social/Social";
@@ -9,6 +9,7 @@ import { finalizeSession } from "../../API";
 import useBoundStore from "../../util/stores";
 import ParticipantLink from "../ParticipantLink/ParticipantLink";
 import UserFeedback from "../UserFeedback/UserFeedback";
+import FinalButton from "./FinalButton";
 
 // Final is an experiment view that shows the final scores of the experiment
 // It can only be the last view of an experiment
@@ -65,9 +66,10 @@ const Final = ({ experiment, participant, score, final_text, action_texts, butto
             </div>
             {button && (
                 <div className="text-center pt-4">
-                    <Link className='btn btn-primary btn-lg' to={button.link} onClick={button.link ? undefined : onNext}>
-                        {button.text}
-                    </Link>
+                    <FinalButton
+                        button={button}
+                        onNext={onNext}
+                    />
                 </div>
             )}
             {logo && (

frontend/src/components/Final/Final.test.jsx

@@ -67,7 +67,7 @@ describe('Final Component', () => {
             </BrowserRouter>
         );
 
-        fireEvent.click(screen.getByText('Next'));
+        fireEvent.click(screen.getByTestId('button'));
         await waitFor(() => {
             expect(onNextMock).toHaveBeenCalled();
         });
@@ -134,4 +134,32 @@ session="session-id"
 
         expect(API.finalizeSession).toHaveBeenCalledWith({ session: 1, participant: 'participant-id' });
     });
+
+    it('Uses Link to navigate when button link is relative', () => {
+        render(
+            <BrowserRouter>
+                <Final
+                    button={{ text: 'Next', link: '/aml' }}
+                />
+            </BrowserRouter>
+        );
+
+        const el = screen.getByTestId('button-link');
+        expect(el).to.exist;
+        expect(el.getAttribute('href')).toBe('/aml');
+    });
+
+    it('Uses an anchor tag to navigate when button link is absolute', () => {
+        render(
+            <BrowserRouter>
+                <Final
+                    button={{ text: 'Next', link: 'https://example.com' }}
+                />
+            </BrowserRouter>
+        );
+
+        const el = screen.getByTestId('button-link');
+        expect(el).to.exist;
+        expect(el.getAttribute('href')).toBe('https://example.com');
+    });
 });

frontend/src/components/Final/FinalButton.tsx

@@ -0,0 +1,48 @@
+import React from "react";
+import { Link, withRouter } from "react-router-dom";
+
+interface FinalButtonProps {
+    button: {
+        text: string;
+        link: string;
+    };
+    onNext: (value: boolean) => void;
+}
+
+const isRelativeUrl = (url: string) => {
+    return url && url.startsWith("/");
+}
+
+const FinalButton: React.FC<FinalButtonProps> = ({ button, onNext }) => {
+
+    if (!button) {
+        return null;
+    }
+
+    // If the button does not have a link, it will call the onNext function using a button click event
+    if (!button.link) {
+        return (
+            <button data-testid="button" className='btn btn-primary btn-lg' onClick={() => onNext(false)}>
+                {button.text}
+            </button>
+        )
+    }
+
+    // If the button has a link, it will render a Link component if the link is a relative URL
+    if (isRelativeUrl(button.link)) {
+        return (
+            <Link data-testid="button-link" className='btn btn-primary btn-lg' to={button.link}>
+                {button.text}
+            </Link>
+        )
+    }
+
+    // If the button has a link, it will render an anchor tag if the link is an absolute URL
+    return (
+        <a data-testid="button-link" className='btn btn-primary btn-lg' href={button.link} target="_blank" rel="noopener noreferrer">
+            {button.text}
+        </a>
+    )
+}
+
+export default withRouter(FinalButton);

frontend/src/stories/Final.stories.jsx

@@ -10,8 +10,8 @@ export default {
     },
 };
 
-export const Default = {
-    args: {
+function getFinalData(overrides = {}) {
+    return {
         score: 100,
         rank: {
             text: "Rank",
@@ -21,15 +21,15 @@ export const Default = {
         points: "points",
         button: {
             text: "Button",
-            link: "https://www.google.com",
+            link: "https://www.example.com",
         },
         logo: {
             image: "https://via.placeholder.com/150",
-            link: "https://www.google.com",
+            link: "https://www.example.com",
         },
         social: {
             apps: ["facebook", "whatsapp", "twitter", "weibo", "share", "clipboard"],
-            url: "https://www.google.com",
+            url: "https://www.example.com",
             message: "Message",
             hashtags: ["hashtag"],
             text: "Text",
@@ -46,22 +46,61 @@ export const Default = {
             button: "Submit",
             thank_you: "Thank you for your feedback!",
             contact_body:
-                '<p>Please contact us at <a href="mailto:info@example.com">info@example.com</a> if you have any questions.</p>',
+                '<p>Please contact us at <a href="mailto:info@example.com">',
         },
         experiment: {
             slug: "test",
         },
         participant: "test",
-    },
-    decorators: [
-        (Story) => (
-            <div
-                style={{ width: "100%", height: "100%", backgroundColor: "#aaa", padding: "1rem" }}
-            >
-                <Router>
-                    <Story />
-                </Router>
-            </div>
-        ),
-    ],
+        onNext: () => { alert("Next"); },
+        ...overrides,
+    };
+}
+
+const getDecorator = (Story) => (
+    <div
+        style={{ width: "100%", height: "100%", backgroundColor: "#aaa", padding: "1rem" }}
+    >
+        <Router>
+            <Story />
+        </Router>
+    </div>
+);
+
+export const Default = {
+    args: getFinalData(),
+    decorators: [getDecorator],
+};
+
+// with relative button.link
+export const RelativeButtonLink = {
+    args: getFinalData({
+        button: {
+            text: "Play again",
+            link: "/profile",
+        },
+    }),
+    decorators: [getDecorator],
 };
+
+// with absolute button.link
+export const AbsoluteButtonLink = {
+    args: getFinalData({
+        button: {
+            text: "Button",
+            link: "https://www.example.com",
+        },
+    }),
+    decorators: [getDecorator],
+};
+
+// without button.link
+export const NoButtonLink = {
+    args: getFinalData({
+        button: {
+            text: "Button",
+            link: "",
+        },
+    }),
+    decorators: [getDecorator],
+};