Skip to content

Commit

Permalink
Refactor: Refactor block and block feedback URLs (#1184)
Browse files Browse the repository at this point in the history
* refactor: Prefix block & block feedback url with /block and remove collection prefix from experiment (fma experiment collection) urls

* fix: Fix admin shortcuts to blocks

* refactor: Update redirect URL in App component

* Refactor: Update experiment references to block in README_TOONTJEHOGER.md

* fix(test): Update route path in Block tests

* fix(test): Fix block urls in e2e tests

* fix: Update getBlockHref function to use "/block" prefix for block URLs

* refactor: Remove snapshot test & artifact from Question tests

* chore: Fix typo

Co-authored-by: Berit <berit.janssen@gmail.com>

---------

Co-authored-by: Berit <berit.janssen@gmail.com>
  • Loading branch information
drikusroor and BeritJanssen authored Jul 12, 2024
1 parent a59b2d1 commit efd6595
Show file tree
Hide file tree
Showing 25 changed files with 72 additions and 88 deletions.
30 changes: 15 additions & 15 deletions README_TOONTJEHOGER.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ Set the following experiment values in the admin:
| ---------- | ------------ | ------ |
| home | toontjehoger | 0 |

## Experiments
## Experiment blocks

The ToontjeHoger experiment rules can be found in:
The ToontjeHoger experiment block rules can be found in:

```
backend/experiment/rules/toontjehoger_1_mozart.py
Expand All @@ -19,13 +19,13 @@ backend/experiment/rules/toontjehoger_5_tempo.py
backend/experiment/rules/toontjehoger_6_relative.py
```

- These rules contain all textual content that is shown during the experiment
- These rules contain all textual content that is shown during the block

**Per experiment config**
**Per block config**

Set the following per-experiment values in the admin:
Set the following per-block values in the admin:

| Experiment | Slug | Rounds |
| Block | Slug | Rounds |
| ---------- | ------------------------ | ------ |
| 1 | toontjehoger_1_mozart | 2 |
| 2 | toontjehoger_2_preverbal | 2 |
Expand All @@ -34,15 +34,15 @@ Set the following per-experiment values in the admin:
| 5 | toontjehoger_5_tempo | 5 |
| 6 | toontjehoger_6_relative | 2 |

### Experiment images
### Block images

Images for the ToontjeHoger experiments can be found in:

```
frontend/public/images/experiments/toontjehoger/*
```

### Experiment information pages
### Block information pages

The information pages are shown after each experiment can be found in:

Expand All @@ -57,9 +57,9 @@ backend/templates/info/toontjehoger/experiment6.html

## Playlists

Each experiment uses a playlist with additional information in the tag/group field.
Each block uses a playlist with additional information in the tag/group field.

### Experiment 1
### Block 1

The group field contains the round.

Expand All @@ -68,7 +68,7 @@ The group field contains the round.
"Wolfgang Amadeus Mozart","Sonate voor twee pianos in D groot, KV 448 (uitgevoerd door Lucas en Arthur Jussen)",0,28.0,"/toontjehoger/mozart/fragment_b.mp3",0,0,2
```

### Experiment 2
### Block 2

The group field contains the round.

Expand All @@ -80,7 +80,7 @@ AML,Franse baby,0,1,/toontjehoger/preverbal/5_franse_baby.mp3,0,a,2
AML,Duitse baby,0,1,/toontjehoger/preverbal/4_duitse_baby.mp3,0,b,2
```

### Experiment 3
### Block 3

The group field contains a semi-colon separated list of time-period and emotion, e.g. 70s;vrolijk

Expand Down Expand Up @@ -112,7 +112,7 @@ David Bowie,Heroes,0,1,toontjehoger/plink/2021-024.mp3,0,0,70s;vrolijk
Dire Straits,Sultans Of Swing,0,1,toontjehoger/plink/2021-025.mp3,0,0,70s;vrolijk
```

### Experiment 4
### Block 4

The tag field contains indicates if the section pitch is original (a) or it it changed (b,c).
The group field contains a song identifier.
Expand Down Expand Up @@ -159,7 +159,7 @@ AML,Breaking Bad,0,1,/toontjehoger/absolute/4_Toonhoogte_Item13_b.mp3,0,b,13
AML,Breaking Bad,0,1,/toontjehoger/absolute/4_Toonhoogte_Item13_c.mp3,0,c,13
```

### Experiment 5
### Block 5

The tag field contains an identifier. The group field shows if it is an original (or) or changed (ch) track.

Expand Down Expand Up @@ -226,7 +226,7 @@ The Stooges,Now I Wanna Be Your Dog,0,1,/toontjehoger/tempo/R5_P2_CH_155.mp3,0,R
Iggy Pop,Now I Wanna Be Your Dog,0,1,/toontjehoger/tempo/R5_P2_OR_155.mp3,0,R5_P2_OR,or
```

### Experiment 6
### Block 6

The tag field indicates if the section in a, b or c.

Expand Down
4 changes: 2 additions & 2 deletions backend/experiment/actions/tests/test_action_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,9 @@ def setUp(self) -> None:
def test_collection_url(self):
self.assertEqual(get_current_collection_url(self.session), None)
self.session.save_json_data({COLLECTION_KEY: 'superdupercollection'})
self.assertEqual(get_current_collection_url(self.session), '/collection/superdupercollection')
self.assertEqual(get_current_collection_url(self.session), '/superdupercollection')
self.participant.participant_id_url = 'participant42'
self.assertEqual(get_current_collection_url(self.session), '/collection/superdupercollection?participant_id=participant42')
self.assertEqual(get_current_collection_url(self.session), '/superdupercollection?participant_id=participant42')

def test_randomize_playhead(self):
min_jitter = 5
Expand Down
4 changes: 2 additions & 2 deletions backend/experiment/actions/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,9 @@ def get_current_collection_url(session: Session) -> str:

if session.participant.participant_id_url:
participant_id_url = session.participant.participant_id_url
return f'/collection/{collection_slug}?participant_id={participant_id_url}'
return f'/{collection_slug}?participant_id={participant_id_url}'
else:
return f'/collection/{collection_slug}'
return f'/{collection_slug}'


def final_action_with_optional_button(session, final_text='', title=_('End'), button_text=_('Continue')):
Expand Down
4 changes: 2 additions & 2 deletions backend/experiment/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ def block_name_link(self, obj):

def block_slug_link(self, obj):
dev_mode = settings.DEBUG is True
url = f"http://localhost:3000/{obj.slug}" if dev_mode else f"/{obj.slug}"
url = f"http://localhost:3000/block/{obj.slug}" if dev_mode else f"/block/{obj.slug}"

return format_html(
f'<a href="{url}" target="_blank" rel="noopener noreferrer" title="Open {obj.slug} block in new tab" >{obj.slug}&nbsp;<small>&#8599;</small></a>')
Expand Down Expand Up @@ -225,7 +225,7 @@ class ExperimentCollectionAdmin(

def slug_link(self, obj):
dev_mode = settings.DEBUG is True
url = f"http://localhost:3000/collection/{obj.slug}" if dev_mode else f"/collection/{obj.slug}"
url = f"http://localhost:3000/{obj.slug}" if dev_mode else f"/{obj.slug}"

return format_html(
f'<a href="{url}" target="_blank" rel="noopener noreferrer" title="Open {obj.slug} experiment group in new tab" >{obj.slug}&nbsp;<small>&#8599;</small></a>')
Expand Down
2 changes: 1 addition & 1 deletion backend/experiment/rules/tests/test_beat_alignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ def load_json(self, response):
return json.loads(response.content)

def test_block(self):
response = self.client.get('/experiment/ba/')
response = self.client.get('/experiment/block/ba/')
response_json = self.load_json(response)
self.assertTrue({'id', 'slug', 'name', 'class_name', 'rounds',
'playlists', 'next_round', 'loading_text'} <= response_json.keys())
Expand Down
11 changes: 10 additions & 1 deletion backend/experiment/tests/test_admin_experiment.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class MockRequest:
request = MockRequest()


class TestAdminExperiment(TestCase):
class TestAdminBlock(TestCase):

@classmethod
def setUpTestData(cls):
Expand Down Expand Up @@ -82,6 +82,15 @@ def test_block_link(self):
'<a href="{}">{}</a>', expected_url, expected_name)
self.assertEqual(link, expected_link)

def test_block_slug_link(self):
block = Block.objects.create(name="Test Block", slug="test-block")
site = AdminSite()
admin = BlockAdmin(block, site)
link = admin.block_slug_link(block)

expected_link = '<a href="/block/test-block" target="_blank" rel="noopener noreferrer" title="Open test-block block in new tab" >test-block&nbsp;<small>&#8599;</small></a>'
self.assertEqual(link, expected_link)


class TestAdminBlockExport(TestCase):

Expand Down
14 changes: 7 additions & 7 deletions backend/experiment/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ def test_get_experiment_collection(self):
session['participant_id'] = self.participant.id
session.save()
# check that first_experiments is returned correctly
response = self.client.get('/experiment/collection/test_series/')
response = self.client.get('/experiment/test_series/')
self.assertEqual(response.json().get(
'nextBlock').get('slug'), 'block1')
# create session
Expand All @@ -87,7 +87,7 @@ def test_get_experiment_collection(self):
participant=self.participant,
finished_at=timezone.now()
)
response = self.client.get('/experiment/collection/test_series/')
response = self.client.get('/experiment/test_series/')
self.assertIn(response.json().get('nextBlock').get(
'slug'), ('block2', 'block3'))
self.assertEqual(response.json().get('dashboard'), [])
Expand All @@ -101,7 +101,7 @@ def test_get_experiment_collection(self):
participant=self.participant,
finished_at=timezone.now()
)
response = self.client.get('/experiment/collection/test_series/')
response = self.client.get('/experiment/test_series/')
response_json = response.json()
self.assertEqual(response_json.get(
'nextBlock').get('slug'), 'block4')
Expand All @@ -117,15 +117,15 @@ def test_get_experiment_collection(self):

def test_get_experiment_collection_not_found(self):
# if ExperimentCollection does not exist, return 404
response = self.client.get('/experiment/collection/not_found/')
response = self.client.get('/experiment/not_found/')
self.assertEqual(response.status_code, 404)

def test_get_experiment_collection_inactive(self):
# if ExperimentCollection is inactive, return 404
collection = ExperimentCollection.objects.get(slug='test_series')
collection.active = False
collection.save()
response = self.client.get('/experiment/collection/test_series/')
response = self.client.get('/experiment/test_series/')
self.assertEqual(response.status_code, 404)

def test_experiment_collection_with_dashboard(self):
Expand All @@ -144,7 +144,7 @@ def test_experiment_collection_with_dashboard(self):
intermediate_phase.dashboard = True
intermediate_phase.save()
# check that first_experiments is returned correctly
response = self.client.get('/experiment/collection/test_series/')
response = self.client.get('/experiment/test_series/')
self.assertEqual(type(response.json().get('dashboard')), list)

def test_experiment_collection_total_score(self):
Expand Down Expand Up @@ -246,7 +246,7 @@ def test_get_block(self):
Session(block=block, participant=participant, finished_at=timezone.now()) for index in range(3)
])

response = self.client.get('/experiment/test-block/')
response = self.client.get('/experiment/block/test-block/')

self.assertEqual(
response.json()['slug'], 'test-block'
Expand Down
6 changes: 3 additions & 3 deletions backend/experiment/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,9 @@
# Experiment
path('render_markdown/', render_markdown, name='render_markdown'),
path('validate_playlist/<str:rules_id>', validate_block_playlist, name='validate_block_playlist'),
path('<slug:slug>/', get_block, name='experiment'),
path('<slug:slug>/feedback/', post_feedback, name='feedback'),
path('collection/<slug:slug>/', get_experiment_collection,
path('block/<slug:slug>/', get_block, name='block'),
path('block/<slug:slug>/feedback/', post_feedback, name='feedback'),
path('<slug:slug>/', get_experiment_collection,
name='experiment_collection'),

# Robots.txt
Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/e2e/test_beatalignment.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ class TestBeatAlignment(BaseTest):
def test_beatalignment(self):
block_name = "beat_alignment"
block_slug = self.config['block_slugs'][block_name]
self.driver.get(f"{self.base_url}/{block_slug}")
self.driver.get(f"{self.base_url}/block/{block_slug}")

self.check_for_error(block_name, block_slug)

Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/e2e/test_categorization.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ def test_categorization(self):
self.driver.delete_all_cookies()

block_slug = self.config['block_slugs'][block_name]
self.driver.get(f"{self.base_url}/{block_slug}")
self.driver.get(f"{self.base_url}/block/{block_slug}")

# if page body contains the word "Error", raise an exception
self.check_for_error(block_name, block_slug)
Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/e2e/test_eurovision.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ def test_eurovision(self):
try:

block_slug = self.config['block_slugs'][block_name]
self.driver.get(f"{self.base_url}/{block_slug}")
self.driver.get(f"{self.base_url}/block/{block_slug}")

# if page body contains the word "Error", raise an exception
self.check_for_error(block_name, block_slug)
Expand Down
2 changes: 1 addition & 1 deletion e2e/tests/smoke/test_thkids.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

class TestTHKids(BaseTest):
def test_th_kids(self):
self.driver.get(f"{self.base_url}/collection/thkids")
self.driver.get(f"{self.base_url}/thkids")

dashboard = WebDriverWait(self.driver, 5).until(expected_conditions.presence_of_element_located((By.CLASS_NAME, "dashboard")))
print("\n✅ The frontend is working and the thkids collection page is working!")
Expand Down
6 changes: 3 additions & 3 deletions frontend/src/API.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,11 @@ axios.defaults.withCredentials = true;
// API endpoints
export const URLS = {
block: {
get: (slug: string) => "/experiment/" + slug + "/",
feedback: (slug: string) => "/experiment/" + slug + "/feedback/",
get: (slug: string) => "/experiment/block/" + slug + "/",
feedback: (slug: string) => "/experiment/block/" + slug + "/feedback/",
},
experiment_collection: {
get: (slug: string) => `/experiment/collection/${slug}/`
get: (slug: string) => `/experiment/${slug}/`
},
participant: {
current: "/participant/",
Expand Down
8 changes: 4 additions & 4 deletions frontend/src/components/App/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ const App = () => {
{/* Default experiment */}
<Route path="/" exact>
<Redirect
to={URLS.block.replace(":slug", EXPERIMENT_SLUG)}
to={URLS.experimentCollection.replace(":slug", EXPERIMENT_SLUG)}
/>
</Route>

Expand All @@ -80,12 +80,12 @@ const App = () => {

<Route path={URLS.internalRedirect} component={InternalRedirect} />

{/* Experiment Collection */}
<Route path={URLS.experimentCollection} component={ExperimentCollection} />

{/* Block */}
<Route path={URLS.block} component={Block} />

{/* Experiment */}
<Route path={URLS.experimentCollection} component={ExperimentCollection} />

<Route path={URLS.session} />

{/* Store profile */}
Expand Down
4 changes: 2 additions & 2 deletions frontend/src/components/Block/Block.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ describe('Block Component', () => {
const spy = vi.spyOn(API, 'getNextRound');

render(
<MemoryRouter initialEntries={['/test']}>
<Route path="/:slug" component={Block} />
<MemoryRouter initialEntries={['/block/test']}>
<Route path="/block/:slug" component={Block} />
</MemoryRouter>
);
const button = await screen.findByText('Continue');
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import React from 'react';
import { MemoryRouter } from 'react-router-dom';
import { render, screen, waitFor } from '@testing-library/react';
import { it, expect, describe } from 'vitest';
import MockAdapter from "axios-mock-adapter";
import axios from 'axios';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const ExperimentCollection = ({ match }: ExperimentCollectionProps) => {
setHasShownConsent(true);
}

const getExperimentHref = (slug: string) => `/${slug}${participantIdUrl ? `?participant_id=${participantIdUrl}` : ""}`;
const getBlockHref = (slug: string) => `/block/${slug}${participantIdUrl ? `?participant_id=${participantIdUrl}` : ""}`;

if (loadingExperimentCollection) {
return (
Expand Down Expand Up @@ -74,7 +74,7 @@ const ExperimentCollection = ({ match }: ExperimentCollectionProps) => {
}

if (!displayDashboard && nextBlock) {
return <Redirect to={getExperimentHref(nextBlock.slug)} />
return <Redirect to={getBlockHref(nextBlock.slug)} />
}

return (
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from 'react';
import { render, screen } from '@testing-library/react';
import { BrowserRouter as Router } from 'react-router-dom';
import { it, expect, describe } from 'vitest';

import ExperimentCollectionAbout from './ExperimentCollectionAbout';

Expand Down Expand Up @@ -29,6 +30,6 @@ describe('ExperimentCollectionAbout', () => {
)

expect(screen.getByRole('link').innerHTML).toContain('Terug');
expect(screen.getByRole('link').getAttribute('href')).toBe('/collection/some_slug');
expect(screen.getByRole('link').getAttribute('href')).toBe('/some_slug');
});
})
})
Loading

0 comments on commit efd6595

Please sign in to comment.