improve UI/UX for removing runs from the schedule without deleting th…

…em (#634)

[#186669498]

bundles/admin/scheduleEditor/index.js

@@ -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));
+            }
           },
         }),
       );

bundles/admin/scheduleEditor/speedrun.js

@@ -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,

bundles/public/orderTarget.tsx

@@ -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}
             </>

tests/test_speedrun.py

@@ -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
@@ -169,108 +172,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):

tracker/admin/event.py

@@ -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(

tracker/models/event.py

@@ -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(

tracker/util.py

@@ -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)

tracker/views/api.py

@@ -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