Merge immortal/main into audit branch

app/routes/users/availability.py

@@ -141,41 +141,41 @@ def add_disponibility():
 @json_endpoint
 @login_required
 def add_disponibilities_bulk():
-    """Add multiple disponibility blocks at once."""
+    """Replace the current player's disponibility blocks atomically."""
     data = request.get_json(silent=True) or {}
     accepted, rejected = _load_slots(data.get('slots'))
 
+    if rejected:
+        return jsonify(
+            {
+                'error': 'Invalid slots; nothing was changed.',
+                'rejected': rejected,
+            }
+        ), 400
+
+    PlayerDisponibility.query.filter_by(player_id=current_user.id).delete()
+
     created = []
     for slot in accepted:
         start_time = slot['start_time']
-        existing = PlayerDisponibility.query.filter_by(
+        disponibility = PlayerDisponibility(
             player_id=current_user.id,
             day_of_week=slot['day_of_week'],
             start_time=start_time,
-        ).first()
+            end_time=slot_end(start_time),
-        if not existing:
+        )
-            disponibility = PlayerDisponibility(
+        db.session.add(disponibility)
-                player_id=current_user.id,
+        db.session.flush()
-                day_of_week=slot['day_of_week'],
+        created.append(
-                start_time=start_time,
+            {
-                end_time=slot_end(start_time),
+                'id': disponibility.id,
-            )
+                'day_of_week': disponibility.day_of_week,
-            db.session.add(disponibility)
+                'day_name': day_name(disponibility.day_of_week),
-            db.session.flush()
+                'start_time': disponibility.start_time.strftime('%H:%M'),
-            created.append(
+            }
-                {
+        )
-                    'id': disponibility.id,
-                    'day_of_week': disponibility.day_of_week,
-                    'day_name': day_name(disponibility.day_of_week),
-                    'start_time': disponibility.start_time.strftime('%H:%M'),
-                }
-            )
     db.session.commit()
-    # `rejected` is reported rather than swallowed. What was accepted is
+    return jsonify({'success': True, 'created': created, 'rejected': []})
-    # still saved — dropping a whole batch because one cell was malformed
-    # would be its own kind of surprise — but the client can now tell the
-    # difference between "nine slots saved" and "ten sent, nine saved".
-    return jsonify({'success': True, 'created': created, 'rejected': rejected})
 
 
 @users_bp.route('/disponibilities/clear', methods=['POST'])

app/templates/pages/profile.html

@@ -193,9 +193,6 @@
                 <p class="text-muted">{{ _('Loading...') }}</p>
             </div>
             <div class="form-actions">
-                <button type="button" class="btn btn-primary" data-action="save-disponibilities">
-                    <i class="fas fa-save"></i> {{ _('Save Disponibilities') }}
-                </button>
                 <button type="button" class="btn btn-secondary" data-action="clear-disponibilities">
                     <i class="fas fa-trash"></i> {{ _('Clear All') }}
                 </button>
@@ -369,7 +366,7 @@ function saveCoachAvailability() {
             const msg = document.createElement('div');
             msg.className = 'alert alert-success';
             msg.style.marginTop = '10px';
-            msg.innerHTML = '<i class="fas fa-check"></i> Availability saved!';
+            msg.innerHTML = '<span><i class="fas fa-check"></i> Availability saved!</span>';
             document.getElementById('availability-grid').appendChild(msg);
             setTimeout(() => msg.remove(), 3000);
         }
@@ -571,7 +568,7 @@ function saveDisponibilities() {
             var msg = document.createElement('div');
             msg.className = 'alert alert-success';
             msg.style.marginTop = '10px';
-            msg.innerHTML = '<i class="fas fa-check"></i> Disponibilities saved successfully!';
+            msg.innerHTML = '<span><i class="fas fa-check"></i> Disponibilities saved successfully!</span>';
             document.getElementById('disponibilities-grid').appendChild(msg);
             setTimeout(function() { msg.remove(); }, 3000);
         }
@@ -610,7 +607,6 @@ document.addEventListener('DOMContentLoaded', function() {
 // dispatched by the delegated listener in main.js. This replaces inline
 // onclick attributes, which no CSP nonce is able to authorise.
 registerActions({
-    'save-disponibilities': saveDisponibilities,
     'clear-disponibilities': clearDisponibilities,
     'clear-availability': clearAllAvailability,
 });

tests/test_timeslots.py

@@ -92,7 +92,7 @@ class TestPlayerAvailability:
             assert PlayerDisponibility.query.count() == 0
 
 
-class TestBulkAvailabilityReportsWhatItDropped:
+class TestBulkAvailabilityReplacementIsAtomic:
     def test_valid_slots_are_saved(self, app, client, as_role):
         from app.models import PlayerDisponibility
 
@@ -107,28 +107,64 @@ class TestBulkAvailabilityReportsWhatItDropped:
         with app.app_context():
             assert PlayerDisponibility.query.count() == 1
 
-    def test_a_dropped_slot_is_named(self, app, client, as_role):
+    def test_a_good_payload_replaces_the_slots(self, app, client, as_role):
-        """It used to `continue` and answer success, so the client could not
-        tell nine saved from ten sent."""
         from app.models import PlayerDisponibility
 
-        as_role('player')
+        player_id = as_role('player')
+        client.post(
+            '/users/disponibilities/add_bulk',
+            json={'slots': [{'day_of_week': 1, 'start_time': '09:00'}]},
+        )
+
+        response = client.post(
+            '/users/disponibilities/add_bulk',
+            json={'slots': [{'day_of_week': 3, 'start_time': '18:00'}]},
+        )
+
+        assert response.status_code == 200
+        with app.app_context():
+            slots = PlayerDisponibility.query.filter_by(player_id=player_id).all()
+            assert [(s.day_of_week, s.start_time) for s in slots] == [(3, time(18, 0))]
+
+    def test_an_empty_payload_clears_the_slots(self, app, client, as_role):
+        from app.models import PlayerDisponibility
+
+        player_id = as_role('player')
+        client.post(
+            '/users/disponibilities/add_bulk',
+            json={'slots': [{'day_of_week': 1, 'start_time': '09:00'}]},
+        )
+
+        response = client.post('/users/disponibilities/add_bulk', json={'slots': []})
+
+        assert response.status_code == 200
+        with app.app_context():
+            assert PlayerDisponibility.query.filter_by(player_id=player_id).count() == 0
+
+    def test_a_malformed_slot_changes_nothing(self, app, client, as_role):
+        from app.models import PlayerDisponibility
+
+        player_id = as_role('player')
+        client.post(
+            '/users/disponibilities/add_bulk',
+            json={'slots': [{'day_of_week': 1, 'start_time': '09:00'}]},
+        )
 
         response = client.post(
             '/users/disponibilities/add_bulk',
             json={
                 'slots': [
-                    {'day_of_week': 1, 'start_time': '09:00'},
+                    {'day_of_week': 3, 'start_time': '18:00'},
                     {'day_of_week': 1, 'start_time': 'nope'},
                 ]
             },
         )
 
-        body = response.get_json()
+        assert response.status_code == 400
-        assert len(body['created']) == 1
+        assert len(response.get_json()['rejected']) == 1
-        assert len(body['rejected']) == 1, 'the caller must learn a slot was dropped'
         with app.app_context():
-            assert PlayerDisponibility.query.count() == 1
+            slots = PlayerDisponibility.query.filter_by(player_id=player_id).all()
+            assert [(s.day_of_week, s.start_time) for s in slots] == [(1, time(9, 0))]
 
 
 class TestCoachAvailabilityIsNotWipedByABadPayload: