From a1e976e37ab4ea7e406fa967f09862d28d8f78ec Mon Sep 17 00:00:00 2001
From: Paul Marillonnet <pmarillonnet@entrouvert.com>
Date: Tue, 23 Jun 2020 11:04:08 +0200
Subject: [PATCH 2/3] tests: fix overly-confident references to object
 identifiers (#43916)

---
 tests/test_dataviz.py      | 42 ++++++++++++++++++++------------------
 tests/test_manager.py      | 20 ++++++++++--------
 tests/test_maps_cells.py   |  2 +-
 tests/test_notification.py | 37 ++++++++++++++++++---------------
 4 files changed, 54 insertions(+), 47 deletions(-)

diff --git a/tests/test_dataviz.py b/tests/test_dataviz.py
index 3854569b..109b816f 100644
--- a/tests/test_dataviz.py
+++ b/tests/test_dataviz.py
@@ -372,20 +372,21 @@ def test_chartng_cell_view(app, normal_user):
             cell = ChartNgCell(page=page, order=1, placeholder='content')
             cell.data_reference = 'plop:example'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/')  # get data in cache
+            location = '/api/dataviz/graph/%s/' % cell.id
+            resp = app.get(location)  # get data in cache
             resp = app.get('/')
             assert 'min-height: 250px' in resp.text
-            assert '/api/dataviz/graph/1/' in resp.text
+            assert location in resp.text
 
-            resp = app.get('/api/dataviz/graph/1/?width=400')
+            resp = app.get(location + '?width=400')
             assert resp.content_type == 'image/svg+xml'
 
-            resp = app.get('/api/dataviz/graph/1/?width=')  # no crash
+            resp = app.get(location + '?width=')  # no crash
             assert resp.content_type == 'image/svg+xml'
 
             page.public = False
             page.save()
-            resp = app.get('/api/dataviz/graph/1/?width=400', status=403)
+            resp = app.get(location + '?width=400', status=403)
 
             page.public = True
             page.save()
@@ -394,14 +395,14 @@ def test_chartng_cell_view(app, normal_user):
             cell.public = False
             cell.groups.set([group])
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/?width=400', status=403)
+            resp = app.get(location + '?width=400', status=403)
 
             app = login(app, username='normal-user', password='normal-user')
-            resp = app.get('/api/dataviz/graph/1/?width=400', status=403)
+            resp = app.get(location + '?width=400', status=403)
 
             normal_user.groups.set([group])
             normal_user.save()
-            resp = app.get('/api/dataviz/graph/1/?width=400', status=200)
+            resp = app.get(location + '?width=400', status=200)
 
             # table visualization
             cell.chart_type = 'table'
@@ -412,20 +413,20 @@ def test_chartng_cell_view(app, normal_user):
             # unsupported dataset
             cell.data_reference = 'plop:seventh'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/')  # get data in cache
+            resp = app.get(location)  # get data in cache
             resp = app.get('/')
             assert 'Unsupported dataset' in resp.text
 
             cell.chart_type = 'bar'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/?width=400', status=200)
+            resp = app.get(location + '?width=400', status=200)
             assert 'Unsupported dataset' in resp.text
 
             # durations
             cell.data_reference = 'plop:eighth'
             cell.chart_type = 'table'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/')  # get data in cache
+            resp = app.get(location)  # get data in cache
             resp = app.get('/')
             assert '<td>Less than an hour</td>' in resp.text
             assert '<td>1 day and 10 hours</td>' in resp.text
@@ -434,7 +435,7 @@ def test_chartng_cell_view(app, normal_user):
 
             cell.chart_type = 'bar'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/?width=400', status=200)
+            resp = app.get(location + '?width=400', status=200)
             assert '>Less than an hour<' in resp.text
             assert '>1 day and 10 hours<' in resp.text
             assert '>2 hours<' in resp.text
@@ -444,19 +445,19 @@ def test_chartng_cell_view(app, normal_user):
             cell.data_reference = 'plop:tenth'
             cell.chart_type = 'table'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/')  # get data in cache
+            resp = app.get(location)  # get data in cache
             resp = app.get('/')
             assert '<td>10.0%</td>' in resp.text
 
             cell.chart_type = 'bar'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/?width=400', status=200)
+            resp = app.get(location + '?width=400', status=200)
             assert '>10.0%<' in resp.text
 
             # deleted visualization
             cell.data_reference = 'plop:eleventh'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/')
+            resp = app.get(location)
             assert 'not found' in resp.text
 
             # cell with missing cached_json (probably after import and missing
@@ -511,32 +512,33 @@ def test_table_cell(app, admin_user):
             cell.data_reference = 'plop:example'
             cell.chart_type = 'table'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/')
+            location = '/api/dataviz/graph/%s/' % cell.id
+            resp = app.get(location)
             resp = app.get('/')
             assert resp.text.count('Total') == 1
 
             cell.data_reference = 'plop:second'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/')
+            resp = app.get(location)
             resp = app.get('/')
             assert resp.text.count('Total') == 1
 
             cell.data_reference = 'plop:third'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/')
+            resp = app.get(location)
             resp = app.get('/')
             assert '114' in resp.text
             assert resp.text.count('Total') == 2
 
             cell.data_reference = 'plop:fourth'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/')
+            resp = app.get(location)
             resp = app.get('/')
             assert resp.text.count('Total') == 0
 
             # total of durations is not computed
             cell.data_reference = 'plop:eigth'
             cell.save()
-            resp = app.get('/api/dataviz/graph/1/')
+            resp = app.get(location)
             resp = app.get('/')
             assert resp.text.count('Total') == 0
diff --git a/tests/test_manager.py b/tests/test_manager.py
index bdecb31c..71e3c61a 100644
--- a/tests/test_manager.py
+++ b/tests/test_manager.py
@@ -728,14 +728,14 @@ def test_add_edit_cell(app, admin_user):
     cells = CellBase.get_cells(page_id=page.id)
     assert len(cells) == 1
     assert isinstance(cells[0], TextCell)
-    assert resp.location.endswith('/manage/pages/1/#cell-%s' % cells[0].get_reference())
+    assert resp.location.endswith('/manage/pages/%s/#cell-%s' % (page.id, cells[0].get_reference()))
 
     resp = app.get('/manage/pages/%s/' % page.id)
     assert ('data-cell-reference="%s"' % cells[0].get_reference()) in resp.text
     resp.forms[0]['c%s-text' % cells[0].get_reference()].value = 'Hello world'
     resp = resp.forms[0].submit()
     assert resp.status_int == 302
-    assert resp.location.endswith('/manage/pages/1/#cell-%s' % cells[0].get_reference())
+    assert resp.location.endswith('/manage/pages/%s/#cell-%s' % (page.id, cells[0].get_reference()))
 
     resp = app.get('/manage/pages/%s/' % page.id)
     assert resp.forms[0]['c%s-text' % cells[0].get_reference()].value == 'Hello world'
@@ -1656,12 +1656,13 @@ def test_page_versionning(app, admin_user):
     # check with asynchronous cells
     resp = app.get('/manage/pages/%s/add-cell-to-content/data_jsoncell/default/' % page.id)
     resp = resp.follow()
-    resp.forms[3]['cdata_jsoncell-1-template_string'].value = 'A{{json.data.0.text}}B'
-    resp.forms[3]['cdata_jsoncell-1-url'].value = 'http://example.com'
+    cell_id = JsonCell.objects.last().id
+    resp.forms[3]['cdata_jsoncell-%s-template_string' % cell_id].value = 'A{{json.data.0.text}}B'
+    resp.forms[3]['cdata_jsoncell-%s-url' % cell_id].value = 'http://example.com'
     resp = resp.forms[3].submit().follow()
     assert PageSnapshot.objects.all().count() == 5  # add + change
 
-    resp.forms[3]['cdata_jsoncell-1-template_string'].value = 'C{{json.data.0.text}}D'
+    resp.forms[3]['cdata_jsoncell-%s-template_string' % cell_id].value = 'C{{json.data.0.text}}D'
     resp = resp.forms[3].submit().follow()
     assert PageSnapshot.objects.all().count() == 6
 
@@ -1749,16 +1750,17 @@ def test_json_cell_syntax_validation(app, admin_user):
     # syntax error
     resp = app.get('/manage/pages/%s/add-cell-to-content/data_jsoncell/default/' % page.id)
     resp = resp.follow()
-    resp.forms[0]['cdata_jsoncell-1-template_string'].value = '{% syntax|error %}'
-    resp.forms[0]['cdata_jsoncell-1-url'].value = 'http://example.com'
+    cell_id = JsonCell.objects.last().id
+    resp.forms[0]['cdata_jsoncell-%s-template_string' % cell_id].value = '{% syntax|error %}'
+    resp.forms[0]['cdata_jsoncell-%s-url' % cell_id].value = 'http://example.com'
     resp = resp.forms[0].submit()
     assert 'syntax error: Invalid block tag' in resp.text
     assert JsonCell.objects.count() == 1
     assert JsonCell.objects.first().template_string is None
     # valid syntax
     resp = app.get('/manage/pages/%s/' % page.id)
-    resp.forms[0]['cdata_jsoncell-1-template_string'].value = '{{ ok }}'
-    resp.forms[0]['cdata_jsoncell-1-url'].value = 'http://example.com'
+    resp.forms[0]['cdata_jsoncell-%s-template_string' % cell_id].value = '{{ ok }}'
+    resp.forms[0]['cdata_jsoncell-%s-url' % cell_id].value = 'http://example.com'
     resp = resp.forms[0].submit().follow()
     assert 'syntax error' not in resp.text
     assert JsonCell.objects.count() == 1
diff --git a/tests/test_maps_cells.py b/tests/test_maps_cells.py
index fd334bd1..5ba764c5 100644
--- a/tests/test_maps_cells.py
+++ b/tests/test_maps_cells.py
@@ -146,7 +146,7 @@ def test_cell_rendering(app, layer, tiles_layer):
     assert 'data-max-zoom="19"' in rendered
     assert 'data-init-lat="48.83369263315934"' in rendered
     assert 'data-init-lng="2.3233688436448574"' in rendered
-    assert '/ajax/mapcell/geojson/1/%s/' % layer.slug in rendered
+    assert '/ajax/mapcell/geojson/%s/%s/' % (cell.id, layer.slug) in rendered
     assert 'data-group-markers="1"' not in rendered
     resp = app.get('/test_map_cell/')
     assert 'xstatic/leaflet.js' in resp.text
diff --git a/tests/test_notification.py b/tests/test_notification.py
index d56198cd..d61d879e 100644
--- a/tests/test_notification.py
+++ b/tests/test_notification.py
@@ -146,24 +146,26 @@ def test_notification_cell(app, john_doe, jane_doe):
 
     app = login_app(app, username='jane.doe', password='jane.doe')
     resp = app.get('/api/menu-badges/?page[]=%s' % page.id)
-    assert resp.json == {'1': {'badge': '1'}}
+    assert resp.json == {'{}'.format(page.id): {'badge': '1'}}
 
 
 def test_notification_ws(john_doe):
 
-    def notify(data, check_id, count):
+    def notify(data, count, check_id=None):
         resp = client.post(reverse('api-notification-add'), json.dumps(data),
                            content_type='application/json')
         assert resp.status_code == 200
         result = json.loads(force_text(resp.content))
-        assert result == {'data': {'id': check_id}, 'err': 0}
+        assert result['err'] == 0
+        if 'id' in data:
+            assert check_id is not None and result['data']['id'] == check_id
         assert Notification.objects.filter(user=john_doe).count() == count
-        return Notification.objects.find(john_doe, check_id).get()
+        return Notification.objects.filter(user=john_doe).order_by('id').last()
 
     login(john_doe)
-    notify({'summary': 'foo'}, '1', 1)
-    notify({'summary': 'bar'}, '2', 2)
-    notify({'summary': 'bar', 'id': 'ns:noti3'}, 'ns:noti3', 3)
+    notify({'summary': 'foo'}, 1)
+    notify({'summary': 'bar'}, 2)
+    notify({'summary': 'bar', 'id': 'ns:noti3'}, 3, check_id='ns:noti3')
     notif = {
         'summary': 'bar',
         'url': 'http://www.example.net',
@@ -172,7 +174,7 @@ def test_notification_ws(john_doe):
         'start_timestamp': '2016-11-11T11:11',
         'end_timestamp': '2016-12-12T12:12',
     }
-    result = notify(notif, '4', 4)
+    result = notify(notif, 4)
     assert result.summary == notif['summary']
     assert result.url == notif['url']
     assert result.body == notif['body']
@@ -182,23 +184,24 @@ def test_notification_ws(john_doe):
 
     del notif['end_timestamp']
     notif['duration'] = 3600
-    result = notify(notif, '5', 5)
+    result = notify(notif, 5)
     assert result.end_timestamp.isoformat()[:19] == '2016-11-11T12:11:00'
 
     notif['duration'] = '3600'
-    result = notify(notif, '6', 6)
+    result = notify(notif, 6)
     assert result.end_timestamp.isoformat()[:19] == '2016-11-11T12:11:00'
 
-    resp = client.get(reverse('api-notification-ack', kwargs={'notification_id': '6'}))
+    notif_id = Notification.objects.order_by('id').last().id
+    resp = client.get(reverse('api-notification-ack', kwargs={'notification_id': '%s' % notif_id}))
     assert resp.status_code == 200
     assert Notification.objects.filter(acked=True).count() == 1
-    assert Notification.objects.filter(acked=True).get().public_id == '6'
+    assert Notification.objects.filter(acked=True).get().public_id == '%s' % notif_id
 
-    resp = client.get(reverse('api-notification-forget', kwargs={'notification_id': '5'}))
+    resp = client.get(reverse('api-notification-forget', kwargs={'notification_id': '%s' % (notif_id - 1)}))
     assert resp.status_code == 200
     assert Notification.objects.filter(acked=True).count() == 2
-    notif = Notification.objects.find(john_doe, '5').get()
-    assert notif.public_id == '5'
+    notif = Notification.objects.find(john_doe, '%s' % (notif_id - 1)).get()
+    assert notif.public_id == '%s' % (notif_id - 1)
     assert notif.acked is True
     assert notif.end_timestamp < now()
 
@@ -206,12 +209,12 @@ def test_notification_ws(john_doe):
     assert resp.status_code == 200
     assert json.loads(force_text(resp.content))['new'] == 3
     assert json.loads(force_text(resp.content))['total'] == 3
-    resp = client.get(reverse('api-notification-ack', kwargs={'notification_id': '1'}))
+    resp = client.get(reverse('api-notification-ack', kwargs={'notification_id': '%s' % (notif_id - 5)}))
     resp = client.get(reverse('api-notification-count'))
     assert resp.status_code == 200
     assert json.loads(force_text(resp.content))['new'] == 2
     assert json.loads(force_text(resp.content))['total'] == 3
-    resp = client.get(reverse('api-notification-forget', kwargs={'notification_id': '1'}))
+    resp = client.get(reverse('api-notification-forget', kwargs={'notification_id': '%s' % (notif_id - 5)}))
     resp = client.get(reverse('api-notification-count'))
     assert resp.status_code == 200
     assert json.loads(force_text(resp.content))['new'] == 2
-- 
2.27.0

