From eb74fecbe3c6a7ba092a7db09215f5dc84792e36 Mon Sep 17 00:00:00 2001
From: J-E Castagnede <j.e.castagnede@gmail.com>
Date: Fri, 14 Oct 2022 14:45:50 +0200
Subject: [PATCH] fix form

---
 geotrek/common/forms.py            |  1 -
 geotrek/common/tests/test_views.py | 86 ++++++++++++++----------------
 geotrek/common/urls.py             |  2 +-
 geotrek/common/views.py            | 20 +++++--
 4 files changed, 56 insertions(+), 53 deletions(-)

diff --git a/geotrek/common/forms.py b/geotrek/common/forms.py
index 8721b8a1eb..84e1dd9fd7 100644
--- a/geotrek/common/forms.py
+++ b/geotrek/common/forms.py
@@ -374,7 +374,6 @@ class SyncRandoForm(forms.Form):
     def helper(self):
         helper = FormHelper()
         helper.form_id = 'form-sync'
-        helper.form_action = reverse('common:sync_randos')
         helper.form_class = 'search'
         # submit button with boostrap attributes, disabled by default
         helper.add_input(Button('sync-web', _("Launch Sync"),
diff --git a/geotrek/common/tests/test_views.py b/geotrek/common/tests/test_views.py
index 29b43247b5..3f049a346e 100644
--- a/geotrek/common/tests/test_views.py
+++ b/geotrek/common/tests/test_views.py
@@ -99,25 +99,29 @@ def test_trek_document_wrong_portal(self, mock_request_get):
 class ViewsTest(TestCase):
     @classmethod
     def setUpTestData(cls):
-        cls.user = UserFactory.create(username='homer', password='dooh')
-
-    def setUp(self):
-        self.client.force_login(user=self.user)
+        cls.user = UserFactory()
+        cls.super_user = SuperUserFactory()
 
     def test_settings_json(self):
+        self.client.force_login(self.user)
         url = reverse('common:settings_json')
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
 
     def test_admin_check_extents(self):
+        """ Admin can access to extents view"""
         url = reverse('common:check_extents')
-        response = self.client.get(url)
-        self.assertEqual(response.status_code, 302)
-        self.user.is_superuser = True
-        self.user.save()
+        self.client.force_login(self.super_user)
         response = self.client.get(url)
         self.assertEqual(response.status_code, 200)
 
+    def test_simple_user_check_extents(self):
+        """ Simple user can't access to extents view"""
+        url = reverse('common:check_extents')
+        self.client.force_login(self.user)
+        response = self.client.get(url)
+        self.assertEqual(response.status_code, 302)
+
     @override_settings(COLUMNS_LISTS={})
     @mock.patch('geotrek.common.mixins.views.logger')
     def test_custom_columns_mixin_error_log(self, mock_logger):
@@ -134,7 +138,8 @@ class MissingColumns(CustomColumnsMixin, MapEntityList):
 class ViewsImportTest(TestCase):
     @classmethod
     def setUpTestData(cls):
-        cls.user = UserFactory.create(username='homer', password='dooh')
+        cls.user = UserFactory()
+        cls.super_user = SuperUserFactory()
 
     def setUp(self):
         self.client.force_login(user=self.user)
@@ -157,8 +162,7 @@ def test_import_update_access(self):
         self.assertEqual(response.status_code, 200)
 
     def test_import_from_file_good_zip_file(self):
-        self.user.is_superuser = True
-        self.user.save()
+        self.client.force_login(user=self.super_user)
 
         with open('geotrek/common/tests/data/test.zip', 'rb') as real_archive:
             url = reverse('common:import_dataset')
@@ -177,8 +181,7 @@ def test_import_from_file_good_zip_file(self):
     @mock.patch('geotrek.common.tasks.current_task')
     @mock.patch('geotrek.common.tasks.import_datas.delay')
     def test_import_from_file_good_geojson_file(self, mocked, mocked_current_task):
-        self.user.is_superuser = True
-        self.user.save()
+        self.client.force_login(user=self.user)
         FileType.objects.create(type="Photographie")
         mocked.side_effect = import_datas
         mocked_current_task.request.id = '1'
@@ -200,8 +203,7 @@ def test_import_from_file_good_geojson_file(self, mocked, mocked_current_task):
 
     @mock.patch('geotrek.common.tasks.import_datas.delay')
     def test_import_from_file_bad_file(self, mocked):
-        self.user.is_superuser = True
-        self.user.save()
+        self.client.force_login(user=self.user)
 
         Parser.label = "Test"
 
@@ -223,8 +225,7 @@ def test_import_from_file_bad_file(self, mocked):
         Parser.label = None
 
     def test_import_form_no_parser_no_superuser(self):
-        self.user.is_superuser = False
-        self.user.save()
+        self.client.force_login(user=self.user)
 
         real_archive = open('geotrek/common/tests/data/test.zip', 'rb+')
         url = reverse('common:import_dataset')
@@ -241,8 +242,7 @@ def test_import_form_no_parser_no_superuser(self):
         self.assertNotContains(response_real, '<form  method="post"')
 
     def test_import_from_web_bad_parser(self):
-        self.user.is_superuser = True
-        self.user.save()
+        self.client.force_login(user=self.user)
 
         url = reverse('common:import_dataset')
 
@@ -257,8 +257,7 @@ def test_import_from_web_bad_parser(self):
         # There is no parser available for user not superuser
 
     def test_import_from_web_good_parser(self):
-        self.user.is_superuser = True
-        self.user.save()
+        self.client.force_login(user=self.user)
 
         url = reverse('common:import_dataset')
         real_key = self.client.get(url).context['form_without_file'].fields['parser'].choices[0][0]
@@ -276,47 +275,44 @@ def test_import_from_web_good_parser(self):
 class SyncRandoViewTest(TestCase):
     @classmethod
     def setUpTestData(cls):
-        cls.super_user = SuperUserFactory.create(username='admin', password='super')
-        cls.simple_user = User.objects.create_user(username='homer', password='doooh')
+        cls.super_user = SuperUserFactory()
+        cls.simple_user = UserFactory()
 
     def setUp(self):
         if os.path.exists(os.path.join(settings.TMP_DIR, 'sync_rando', 'tmp_sync')):
             shutil.rmtree(os.path.join(settings.TMP_DIR, 'sync_rando', 'tmp_sync'))
 
     def test_get_sync_superuser(self):
-        self.client.login(username='admin', password='super')
-        response = self.client.get(reverse('common:sync_randos_view'))
+        self.client.force_login(self.super_user)
+        response = self.client.get(reverse('common:sync_randos'))
         self.assertEqual(response.status_code, 200)
 
     def test_post_sync_superuser(self):
-        """
-        test if sync can be launched by superuser post
-        """
-        self.client.login(username='admin', password='super')
-        response = self.client.post(reverse('common:sync_randos'), data={})
+        """ Sync can be launched by superuser post """
+        self.client.force_login(self.super_user)
+        response = self.client.post(reverse('common:sync_randos_view'), data={})
         self.assertRedirects(response, '/commands/syncview')
 
-    def test_get_sync_simpleuser(self):
-        self.client.login(username='homer', password='doooh')
+    def test_get_sync_simple_user(self):
+        """ Simple user can't access to Sync rando view """
+        self.client.force_login(self.simple_user)
         response = self.client.get(reverse('common:sync_randos_view'))
-        self.assertRedirects(response, '/login/?next=/commands/syncview')
+        self.assertEquals(response.status_code, 403)
 
-    def test_post_sync_simpleuser(self):
-        """
-        test if sync can be launched by simple user post
-        """
-        self.client.login(username='homer', password='doooh')
+    def test_post_sync_simple_user(self):
+        """ Sync can't be launched by simple user post """
+        self.client.force_login(self.simple_user)
         response = self.client.post(reverse('common:sync_randos'), data={})
-        self.assertRedirects(response, '/login/?next=/commands/sync')
+        self.assertEquals(response.status_code, 403)
 
     def test_get_sync_states_superuser(self):
-        self.client.login(username='admin', password='super')
+        self.client.force_login(self.super_user)
         response = self.client.post(reverse('common:sync_randos_state'), data={})
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.content, b'[]')
 
-    def test_get_sync_states_simpleuser(self):
-        self.client.login(username='homer', password='doooh')
+    def test_get_sync_states_simple_user(self):
+        self.client.force_login(self.simple_user)
         response = self.client.post(reverse('common:sync_randos_state'), data={})
         self.assertRedirects(response, '/login/?next=/commands/statesync/')
 
@@ -327,9 +323,7 @@ def test_get_sync_states_simpleuser(self):
                                            'skip_tiles': True, 'skip_pdf': True,
                                            'skip_dem': True, 'skip_profile_png': True})
     def test_get_sync_rando_states_superuser_with_sync_rando(self, mocked_stdout):
-        if os.path.exists(os.path.join(settings.TMP_DIR, 'sync_rando', 'tmp_sync')):
-            shutil.rmtree(os.path.join(settings.TMP_DIR, 'sync_rando', 'tmp_sync'))
-        self.client.login(username='admin', password='super')
+        self.client.force_login(self.super_user)
         launch_sync_rando.apply()
         response = self.client.post(reverse('common:sync_randos_state'), data={})
         self.assertEqual(response.status_code, 200)
@@ -344,7 +338,7 @@ def test_get_sync_rando_states_superuser_with_sync_rando(self, mocked_stdout):
                                            'skip_tiles': True, 'skip_pdf': True,
                                            'skip_dem': True, 'skip_profile_png': True})
     def test_get_sync_rando_states_superuser_with_sync_mobile_fail(self, mocked_stdout, command):
-        self.client.login(username='admin', password='super')
+        self.client.force_login(self.super_user)
         launch_sync_rando.apply()
         response = self.client.post(reverse('common:sync_randos_state'), data={})
         self.assertEqual(response.status_code, 200)
diff --git a/geotrek/common/urls.py b/geotrek/common/urls.py
index 0a1d5dc663..cbfc17621e 100644
--- a/geotrek/common/urls.py
+++ b/geotrek/common/urls.py
@@ -17,7 +17,7 @@ class LangConverter(converters.StringConverter):
     path('commands/import-update.json', import_update_json, name='import_update_json'),
     path('commands/import', views.import_view, name='import_dataset'),
     path('commands/sync', views.SyncRandoRedirect.as_view(), name='sync_randos'),
-    path('commands/syncview', views.SyncRandoView.as_view(), name='sync_randos_view'),
+    path('commands/syncview', views.SyncRandoFormView.as_view(), name='sync_randos_view'),
     path('commands/statesync/', views.sync_update_json, name='sync_randos_state'),
     path('api/<lang:lang>/themes.json', views.ThemeViewSet.as_view({'get': 'list'}), name="themes_json"),
 ]
diff --git a/geotrek/common/views.py b/geotrek/common/views.py
index 8943e6f349..3bcc4cf290 100644
--- a/geotrek/common/views.py
+++ b/geotrek/common/views.py
@@ -21,7 +21,7 @@
 from django.db.models.functions import Cast
 from django.http import JsonResponse, Http404, HttpResponse, HttpResponseRedirect
 from django.shortcuts import get_object_or_404, render, redirect
-from django.urls import reverse
+from django.urls import reverse, reverse_lazy
 from django.utils import timezone, translation
 from django.utils.decorators import method_decorator
 from django.utils.encoding import force_str
@@ -328,13 +328,21 @@ def last_list(request):
     return redirect('trekking:trek_list')
 
 
-class SyncRandoView(LoginRequiredMixin, UserPassesTestMixin, FormView):
+class SyncRandoFormView(LoginRequiredMixin, UserPassesTestMixin, FormView):
     form_class = SyncRandoForm
     template_name = 'common/sync_rando.html'
+    success_url = reverse_lazy('common:sync_randos_view')
 
     def test_func(self):
+        """ Only superuser can use this view """
         return self.request.user.is_superuser
 
+    def form_valid(self, form):
+        url = "{scheme}://{host}".format(scheme='https' if self.request.is_secure() else 'http',
+                                         host=self.request.get_host())
+        launch_sync_rando.delay(url=url)
+        return super().form_valid(form)
+
 
 @login_required
 @user_passes_test(lambda u: u.is_superuser)
@@ -382,12 +390,14 @@ def sync_update_json(request):
                         content_type="application/json")
 
 
-class SyncRandoRedirect(RedirectView):
+class SyncRandoRedirect(LoginRequiredMixin, UserPassesTestMixin, RedirectView):
     http_method_names = ['post']
     pattern_name = 'common:sync_randos_view'
 
-    @method_decorator(login_required)
-    @method_decorator(user_passes_test(lambda u: u.is_superuser))
+    def test_func(self):
+        """ Only superuser can use this view """
+        return self.request.user.is_superuser
+
     def post(self, request, *args, **kwargs):
         url = "{scheme}://{host}".format(scheme='https' if self.request.is_secure() else 'http',
                                          host=self.request.get_host())