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

improve UI/UX for removing runs from the schedule without deleting them [#186669498] #634

Merged
merged 1 commit into from
Dec 13, 2023
Merged
Changes from all 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
8 changes: 6 additions & 2 deletions bundles/admin/scheduleEditor/index.js
Original file line number Diff line number Diff line change
@@ -107,7 +107,9 @@ function dispatch(dispatch) {
},
moveSpeedrun: (source, destination, before) => {
dispatch(actions.models.setInternalModelField('speedrun', source, 'moving', true));
dispatch(actions.models.setInternalModelField('speedrun', destination, 'moving', true));
if (destination != null) {
dispatch(actions.models.setInternalModelField('speedrun', destination, 'moving', true));
}
dispatch(actions.models.setInternalModelField('speedrun', source, 'errors', null));
dispatch(
actions.models.command({
@@ -122,7 +124,9 @@ function dispatch(dispatch) {
},
always: () => {
dispatch(actions.models.setInternalModelField('speedrun', source, 'moving', false));
dispatch(actions.models.setInternalModelField('speedrun', destination, 'moving', false));
if (destination != null) {
dispatch(actions.models.setInternalModelField('speedrun', destination, 'moving', false));
}
},
}),
);
3 changes: 2 additions & 1 deletion bundles/admin/scheduleEditor/speedrun.js
Original file line number Diff line number Diff line change
@@ -162,7 +162,7 @@ class Speedrun extends React.Component {
};

nullOrder_ = () => {
this.props.saveField(this.props.speedrun, 'order', null);
this.props.moveSpeedrun(this.props.speedrun.pk, null, true);
};

cancelEdit_ = () => {
@@ -197,6 +197,7 @@ Speedrun.propTypes = {
speedrun: SpeedrunShape.isRequired,
draft: SpeedrunShape,
moveSpeedrun: PropTypes.func,
updateField: PropTypes.func,
saveField: PropTypes.func,
saveModel: PropTypes.func,
cancelEdit: PropTypes.func,
3 changes: 2 additions & 1 deletion bundles/public/orderTarget.tsx
Original file line number Diff line number Diff line change
@@ -32,7 +32,8 @@ function OrderTarget<T extends { before: boolean }, TP>({
{Target(false)}
{nullOrder ? (
<span style={{ cursor: 'pointer' }} onClick={nullOrder}>
<img src={STATIC_URL + 'dsc.png'} alt="unordered" />
<img src={STATIC_URL + 'dsc.png'} />
</span>
) : null}
</>
180 changes: 100 additions & 80 deletions tests/test_speedrun.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import datetime
import itertools
import random
from typing import Iterable, List, Optional, Union
from unittest import skipIf

import django
@@ -10,6 +12,7 @@

import tracker.models as models
from tracker import settings
from tracker.util import pairwise

from . import randgen
from .util import today_noon
@@ -165,108 +168,125 @@ def setUp(self):
name='Test Run 4', run_time='0:20:00', setup_time='0:05:00', order=None
)

def test_after_to_before(self):
def assertResults(
self,
moving: models.SpeedRun,
other: Optional[models.SpeedRun],
before: bool,
*,
expected_change_count: int = 0,
expected_error_keys: Optional[Union[List[str], type]] = None,
expected_status_code: int = 200,
):
from tracker.views.commands import MoveSpeedRun

output, status = MoveSpeedRun(
{'moving': self.run2.id, 'other': self.run1.id, 'before': True}
output, status_code = MoveSpeedRun(
{
'moving': moving.id,
'other': other.id if other else None,
'before': before,
}
)
self.assertEqual(status, 200)
self.assertEqual(len(output), 2)
self.run1.refresh_from_db()
self.run2.refresh_from_db()
self.run3.refresh_from_db()
self.assertEqual(self.run1.order, 2)
self.assertEqual(self.run2.order, 1)
self.assertEqual(self.run3.order, 3)

def test_after_to_after(self):
from tracker.views.commands import MoveSpeedRun
self.assertEqual(status_code, expected_status_code)
if expected_status_code == 200:
self.assertEqual(len(output), expected_change_count)
if expected_error_keys is not None:
self.assertIsInstance(output, dict, msg='Expected a dict')
self.assertTrue('error' in output, msg='Expected an `error` key in dict')
# a bit goofy perhaps but hopefully commands go away soon
if isinstance(expected_error_keys, type):
self.assertIsInstance(output['error'], expected_error_keys)
else:
self.assertEqual(set(output['error'].keys()), set(expected_error_keys))

def assertRunsInOrder(
self, ordered: Iterable[models.SpeedRun], unordered: Iterable[models.SpeedRun]
):
all_runs = list(itertools.chain(ordered, unordered))

self.assertNotEqual(len(all_runs), 0, msg='Run list was empty')

for r in all_runs:
r.refresh_from_db()

for a, b in pairwise(all_runs):
self.assertEqual(
a.event_id, b.event_id, msg='Runs are from different events'
)

output, status = MoveSpeedRun(
{'moving': self.run3.id, 'other': self.run1.id, 'before': False}
# be exhaustive
self.assertEqual(
len(all_runs),
models.SpeedRun.objects.filter(event=all_runs[0].event_id).count(),
msg='Not all runs for this event were provided',
)
self.assertEqual(status, 200)
self.assertEqual(len(output), 2)
self.run1.refresh_from_db()
self.run2.refresh_from_db()
self.run3.refresh_from_db()
self.assertEqual(self.run1.order, 1)
self.assertEqual(self.run2.order, 3)
self.assertEqual(self.run3.order, 2)

def test_before_to_before(self):
from tracker.views.commands import MoveSpeedRun
for n, (a, b) in enumerate(pairwise(ordered), start=1):
with self.subTest(f'ordered {n}: {a}, {b}'):
self.assertEqual(a.order, n, msg='Order was wrong')
self.assertEqual(b.order, n + 1, msg='Order was wrong')
self.assertEqual(a.endtime, b.starttime, msg='Run times do not match')

output, status = MoveSpeedRun(
{'moving': self.run1.id, 'other': self.run3.id, 'before': True}
)
self.assertEqual(status, 200)
self.assertEqual(len(output), 2)
self.run1.refresh_from_db()
self.run2.refresh_from_db()
self.run3.refresh_from_db()
self.assertEqual(self.run1.order, 2)
self.assertEqual(self.run2.order, 1)
self.assertEqual(self.run3.order, 3)
for r in unordered:
with self.subTest(f'unordered {r}'):
self.assertIsNone(r.order, msg='Run should have been unordered')

def test_before_to_after(self):
from tracker.views.commands import MoveSpeedRun
def test_after_to_before(self):
self.assertResults(self.run2, self.run1, True, expected_change_count=2)
self.assertRunsInOrder([self.run2, self.run1, self.run3], [self.run4])

output, status = MoveSpeedRun(
{'moving': self.run1.id, 'other': self.run2.id, 'before': False}
)
self.assertEqual(status, 200)
self.assertEqual(len(output), 2)
self.run1.refresh_from_db()
self.run2.refresh_from_db()
self.run3.refresh_from_db()
self.assertEqual(self.run1.order, 2)
self.assertEqual(self.run2.order, 1)
self.assertEqual(self.run3.order, 3)
def test_after_to_after(self):
self.assertResults(self.run3, self.run1, False, expected_change_count=2)
self.assertRunsInOrder([self.run1, self.run3, self.run2], [self.run4])

def test_unordered_to_before(self):
from tracker.views.commands import MoveSpeedRun
def test_before_to_before(self):
self.assertResults(self.run1, self.run3, True, expected_change_count=2)
self.assertRunsInOrder([self.run2, self.run1, self.run3], [self.run4])

output, status = MoveSpeedRun(
{'moving': self.run4.id, 'other': self.run2.id, 'before': True}
)
self.assertEqual(status, 200)
self.assertEqual(len(output), 3)
self.run2.refresh_from_db()
self.run3.refresh_from_db()
self.run4.refresh_from_db()
self.assertEqual(self.run2.order, 3)
self.assertEqual(self.run3.order, 4)
self.assertEqual(self.run4.order, 2)
def test_before_to_after(self):
self.assertResults(self.run1, self.run2, False, expected_change_count=2)
self.assertRunsInOrder([self.run2, self.run1, self.run3], [self.run4])

def test_unordered_to_before(self):
self.assertResults(self.run4, self.run2, True, expected_change_count=3)
self.assertRunsInOrder([self.run1, self.run4, self.run2, self.run3], [])

def test_unordered_to_after(self):
from tracker.views.commands import MoveSpeedRun
self.assertResults(self.run4, self.run2, False, expected_change_count=2)
self.assertRunsInOrder([self.run1, self.run2, self.run4, self.run3], [])

def test_remove_from_order(self):
self.assertResults(self.run2, None, True, expected_change_count=2)
self.assertRunsInOrder([self.run1, self.run3], [self.run2, self.run4])

def test_already_removed(self):
self.assertResults(self.run4, None, True, expected_status_code=400)

def test_error_cases(self):
self.assertResults(
self.run2,
self.run2,
True,
expected_error_keys=str,
expected_status_code=400,
)

output, status = MoveSpeedRun(
{'moving': self.run4.id, 'other': self.run2.id, 'before': False}
self.assertResults(
self.run4, None, True, expected_error_keys=str, expected_status_code=400
)
self.assertEqual(status, 200)
self.assertEqual(len(output), 2)
self.run2.refresh_from_db()
self.run3.refresh_from_db()
self.run4.refresh_from_db()
self.assertEqual(self.run2.order, 2)
self.assertEqual(self.run3.order, 4)
self.assertEqual(self.run4.order, 3)

def test_too_long_for_anchor(self):
from tracker.views.commands import MoveSpeedRun

self.run2.anchor_time = self.run2.starttime
self.run2.save()

output, status = MoveSpeedRun(
{'moving': self.run3.id, 'other': self.run2.id, 'before': True}
self.assertResults(
self.run3,
self.run2,
True,
expected_error_keys=['setup_time'],
expected_status_code=400,
)

self.assertEqual(status, 400)


class TestSpeedRunAdmin(TransactionTestCase):
def setUp(self):
6 changes: 5 additions & 1 deletion tracker/admin/event.py
Original file line number Diff line number Diff line change
@@ -718,12 +718,12 @@ class SpeedRunAdmin(EventLockedMixin, CustomModelAdmin):
list_display = (
'name',
'category',
'description',
'runners_',
'hosts_',
'commentators_',
'onsite',
'starttime',
'anchored',
'run_time',
'setup_time',
)
@@ -772,6 +772,10 @@ def hosts_(self, instance):
def commentators_(self, instance):
return ', '.join(str(h) for h in instance.commentators.all())

@admin.display(description='Anchored', boolean=True)
def anchored(self, instance):
return instance.anchor_time is not None

def bids(self, instance):
if instance.id is not None:
return format_html(
15 changes: 1 addition & 14 deletions tracker/models/event.py
Original file line number Diff line number Diff line change
@@ -20,19 +20,6 @@
from .fields import TimestampField
from .util import LatestEvent

# TODO: remove when 3.10 is oldest supported version

try:
from itertools import pairwise
except ImportError:

def pairwise(iterable):
# pairwise('ABCDEFG') --> AB BC CD DE EF FG
a, b = itertools.tee(iterable)
next(b, None)
return zip(a, b)


__all__ = [
'Event',
'PostbackURL',
@@ -568,7 +555,7 @@ def clean(self):
'order': 'Next anchor in the order would occur before this one'
}
)
for c, n in pairwise(
for c, n in util.pairwise(
itertools.chain(
[self],
SpeedRun.objects.filter(
19 changes: 19 additions & 0 deletions tracker/util.py
Original file line number Diff line number Diff line change
@@ -12,6 +12,7 @@
try:
import zoneinfo
except ImportError:
# noinspection PyUnresolvedReferences
from backports import zoneinfo


@@ -128,3 +129,21 @@ def flatten(iterable):

def utcnow() -> datetime.datetime:
return datetime.datetime.now(datetime.timezone.utc)


def set_mismatch(expected, actual):
return set(expected) - set(actual), set(actual) - set(expected)


try:
from itertools import pairwise
except ImportError:
# TODO: remove when 3.10 is oldest supported version

def pairwise(iterable):
import itertools

# pairwise('ABCDEFG') --> AB BC CD DE EF FG
a, b = itertools.tee(iterable)
next(b, None)
return zip(a, b)
4 changes: 3 additions & 1 deletion tracker/views/api.py
Original file line number Diff line number Diff line change
@@ -745,9 +745,11 @@ def command(request):
func = getattr(commands, data['command'], None)
if func:
if request.user.has_perm(func.permission):
output, status = func(data)
output, status = func({k: v for k, v in data.items() if k != 'command'})
if status == 200:
output = serializers.serialize('json', output, ensure_ascii=False)
else:
output = json.dumps(output)
else:
output = json.dumps({'error': 'permission denied'})
status = 403
61 changes: 51 additions & 10 deletions tracker/views/commands.py
Original file line number Diff line number Diff line change
@@ -1,24 +1,56 @@
import json

from django.core.exceptions import ValidationError
from django.db import transaction

from tracker.models import SpeedRun
from tracker.util import set_mismatch

__all__ = [
'MoveSpeedRun',
]


def MoveSpeedRun(data):
moving = SpeedRun.objects.get(pk=data['moving'])
other = SpeedRun.objects.get(pk=data['other'])
if moving.event_id != other.event_id:
return json.dumps({'error': 'Runs are not in the same event'}), 400
before = bool(data['before'])
try:
expected_keys = {'moving', 'other', 'before'}
missing_keys, extra_keys = set_mismatch(expected_keys, data.keys())
if missing_keys or extra_keys:
error = ValidationError(
'required keys were missing and/or extra keys were provided'
)
if missing_keys:
error.error_dict = error.update_error_dict(
{'missing_keys': missing_keys}
)
if extra_keys:
error.error_dict = error.update_error_dict({'extra_keys': extra_keys})
raise error
moving = SpeedRun.objects.filter(pk=data['moving'], event__locked=False).first()
other = (
SpeedRun.objects.filter(pk=data['other'], event__locked=False).first()
if data.get('other', None)
else None
)
if moving is None:
raise ValidationError('moving run does not exist or is locked')
if data['other'] is not None and other is None:
raise ValidationError('other run does not exist or is locked')
if moving == other:
raise ValidationError('runs are the same run')
if other and moving.event_id != other.event_id:
raise ValidationError('runs are not from the same event')
before = bool(data['before'])
with transaction.atomic():
if moving.order is None:
if other is None:
if moving.order is None:
raise ValidationError('run is already unordered')
else:
runs = SpeedRun.objects.filter(
event=moving.event, order__gt=moving.order
).select_for_update()
final = None
first = moving.order
diff = -1
elif moving.order is None:
if before:
runs = SpeedRun.objects.filter(
event=moving.event, order__gte=other.order
@@ -81,12 +113,21 @@ def MoveSpeedRun(data):
first_run = SpeedRun.objects.get(event=moving.event, order=first)
first_run.clean()
models = first_run.save()
if other is None:
models = models + [moving]
return models, 200
except ValidationError as e:
result = {}
if hasattr(e, 'error_dict'):
return json.dumps({'error': e.message_dict}), 400
result.update({'error': e.message_dict})
elif len(e.messages) > 1:
result.update({'error': e.messages})
else:
return json.dumps({'error': e.messages}), 400
result.update({'error': e.message})
if getattr(e, 'code', None) is not None:
result['code'] = e.code

return result, 400


MoveSpeedRun.permission = 'tracker.change_speedrun'