From dd7aad182c129768a84247f0c89bb323d13b2a10 Mon Sep 17 00:00:00 2001 From: muhammad-ammar Date: Thu, 14 Nov 2019 13:21:15 +0500 Subject: [PATCH] remove success-url validation --- CHANGELOG.rst | 5 +++ enterprise/__init__.py | 2 +- enterprise/forms.py | 7 ---- .../js/enterprise_selection_page.js | 10 +++--- .../enterprise_customer_select_form.html | 1 - enterprise/views.py | 11 ++++--- spec/javascripts/enterprise_select_spec.js | 13 ++------ .../views/test_enterprise_selection.py | 32 ++++++------------- 8 files changed, 29 insertions(+), 52 deletions(-) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index bf31e4098a..df37436921 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -14,6 +14,11 @@ Change Log Unreleased ---------- +[2.0.21] - 2019-11-14 +--------------------- + +* Remove success url validation for select enterprise page. + [2.0.20] - 2019-11-13 --------------------- diff --git a/enterprise/__init__.py b/enterprise/__init__.py index ad248c89c2..da4976c11c 100644 --- a/enterprise/__init__.py +++ b/enterprise/__init__.py @@ -4,6 +4,6 @@ from __future__ import absolute_import, unicode_literals -__version__ = "2.0.20" +__version__ = "2.0.21" default_app_config = "enterprise.apps.EnterpriseConfig" # pylint: disable=invalid-name diff --git a/enterprise/forms.py b/enterprise/forms.py index d7ae07f694..28602103df 100644 --- a/enterprise/forms.py +++ b/enterprise/forms.py @@ -5,7 +5,6 @@ from __future__ import absolute_import, unicode_literals from django import forms -from django.urls import Resolver404, resolve from django.utils.translation import ugettext as _ from enterprise.models import EnterpriseCustomer, EnterpriseCustomerUser @@ -50,10 +49,4 @@ def clean(self): if not EnterpriseCustomerUser.objects.filter(enterprise_customer=enterprise, user_id=self._user_id).exists(): raise forms.ValidationError(_("Wrong Enterprise")) - try: - # validate if path is a valid - resolve(cleaned_data['success_url']) - except Resolver404: - raise forms.ValidationError(_("Incorrect success url")) - return cleaned_data diff --git a/enterprise/static/enterprise/js/enterprise_selection_page.js b/enterprise/static/enterprise/js/enterprise_selection_page.js index df0ba0e0a5..44b06017ad 100644 --- a/enterprise/static/enterprise/js/enterprise_selection_page.js +++ b/enterprise/static/enterprise/js/enterprise_selection_page.js @@ -6,7 +6,6 @@ function setupFormSubmit() { $('#select-enterprise-form').submit(function(event){ event.preventDefault(); var selectedEnterpriseUUID = $("#id_enterprise").val(); - var successURL = $("#id_success_url").val(); $("#activate-progress-icon").removeClass("is-hidden"); $("#select-enterprise-submit").attr("disabled", true); @@ -14,16 +13,17 @@ function setupFormSubmit() { $.ajax({ url : "/enterprise/select/active", method : "POST", + beforeSend: function (xhr) { xhr.setRequestHeader("X-CSRFToken", $.cookie("csrftoken")); }, + data : { - enterprise : selectedEnterpriseUUID, - success_url: successURL + enterprise : selectedEnterpriseUUID }, - success: function(xhr) { - redirectToURL(xhr.success_url); + success: function() { + redirectToURL($("#id_success_url").val()); }, error : function(xhr) { diff --git a/enterprise/templates/enterprise/enterprise_customer_select_form.html b/enterprise/templates/enterprise/enterprise_customer_select_form.html index e207bdaa8e..be6e9074d2 100644 --- a/enterprise/templates/enterprise/enterprise_customer_select_form.html +++ b/enterprise/templates/enterprise/enterprise_customer_select_form.html @@ -39,7 +39,6 @@

{{ select_enterprise_message_title|safe }} - {{ hidden_field.errors }} {{ hidden_field }} {% endfor %} diff --git a/enterprise/views.py b/enterprise/views.py index d2d2edbd5c..ae1a26d075 100644 --- a/enterprise/views.py +++ b/enterprise/views.py @@ -839,6 +839,10 @@ def get_initial(self): 'success_url': self.request.GET.get('success_url'), 'user_id': self.request.user.id }) + LOGGER.info( + '[Enterprise Selection Page] Request recieved. SuccessURL: %s', + self.request.GET.get('success_url') + ) return initial def get_context_data(self, **kwargs): @@ -849,7 +853,6 @@ def get_context_data(self, **kwargs): 'select_enterprise_message_title': _(u'Select an organization'), 'select_enterprise_message_subtitle': ENTERPRISE_SELECT_SUBTITLE, }) - context_data.update(get_global_context(self.request, None)) return context_data def form_invalid(self, form): @@ -862,7 +865,7 @@ def form_invalid(self, form): def form_valid(self, form): """ - If the form is valid, activate the selected enterprise and return `success_url`. + If the form is valid, activate the selected enterprise. """ enterprise_customer = form.cleaned_data['enterprise'] serializer = EnterpriseCustomerUserWriteSerializer(data={ @@ -874,10 +877,10 @@ def form_valid(self, form): serializer.save() LOGGER.info( '[Enterprise Selection Page] Learner activated an enterprise. User: %s, EnterpriseCustomer: %s', + self.request.user.username, enterprise_customer, - self.request.user.username ) - return JsonResponse({'success_url': form.cleaned_data['success_url']}) + return JsonResponse({}) class HandleConsentEnrollment(View): diff --git a/spec/javascripts/enterprise_select_spec.js b/spec/javascripts/enterprise_select_spec.js index c9bd886503..56cf32aa9b 100644 --- a/spec/javascripts/enterprise_select_spec.js +++ b/spec/javascripts/enterprise_select_spec.js @@ -39,15 +39,12 @@ describe('Enterprise Selection Page', function () { }); it('works expected on correct post data', function () { - var response = { - 'success_url': '/dashboard' - }; var redirectSpy = spyOn(window, 'redirectToURL'); jasmine.Ajax .stubRequest('/enterprise/select/active') .andReturn({ - responseText: JSON.stringify(response) + responseText: JSON.stringify({}) }); $( '#select-enterprise-submit' ).trigger( 'click' ); @@ -56,14 +53,13 @@ describe('Enterprise Selection Page', function () { expect(request.url).toBe('/enterprise/select/active'); expect(request.method).toBe('POST'); expect(request.data().enterprise).toEqual(['6ae013d4-c5c4-474d-8da9-0e559b2448e2']); - expect(request.data().success_url).toEqual(['/dashboard']); expect(redirectSpy.calls.count()).toEqual(1); expect(redirectSpy.calls.first().args).toEqual(['/dashboard']); }); it('works expected on incorrect post data', function () { var response = { - 'errors': ['Incorrect success url'] + 'errors': ['Enterprise not found'] }; jasmine.Ajax @@ -73,16 +69,11 @@ describe('Enterprise Selection Page', function () { responseText: JSON.stringify(response) }); - // remove success url value from hidden input - $('#id_success_url').removeAttr('value'); - $( '#select-enterprise-submit' ).trigger( 'click' ); var request = jasmine.Ajax.requests.mostRecent(); expect(request.url).toBe('/enterprise/select/active'); expect(request.method).toBe('POST'); - expect(request.data().enterprise).toEqual(['6ae013d4-c5c4-474d-8da9-0e559b2448e2']); - expect(request.data().success_url).toEqual(['']); expect($('#select-enterprise-form-error').text().trim()).toEqual(response.errors[0]); }); diff --git a/tests/test_enterprise/views/test_enterprise_selection.py b/tests/test_enterprise/views/test_enterprise_selection.py index 36038faf64..9acfdcb8c6 100644 --- a/tests/test_enterprise/views/test_enterprise_selection.py +++ b/tests/test_enterprise/views/test_enterprise_selection.py @@ -113,7 +113,6 @@ def test_view_post(self): new_enterprise = self.enterprise_choices[2][0] post_data = { 'enterprise': new_enterprise, - 'success_url': self.success_url } with mock.patch('enterprise.views.LOGGER.info') as mock_logger: @@ -121,12 +120,11 @@ def test_view_post(self): assert mock_logger.called assert mock_logger.call_args.args == ( u'[Enterprise Selection Page] Learner activated an enterprise. User: %s, EnterpriseCustomer: %s', + self.user.username, new_enterprise, - self.user.username ) assert response.status_code == 200 - assert response.json().get('success_url') == self.success_url # after selection only the selected enterprise should be active for learner assert EnterpriseCustomerUser.objects.get(user_id=user_id, enterprise_customer=new_enterprise).active @@ -135,31 +133,19 @@ def test_view_post(self): for obj in EnterpriseCustomerUser.objects.filter(user_id=user_id).exclude(enterprise_customer=new_enterprise): assert not obj.active - @ddt.data( - { - 'enterprise': '111', - 'success_url': '', - 'errors': [ - u'Enterprise not found', - u'Select a valid choice. 111 is not one of the available choices.' - ] - }, - { - 'enterprise': None, - 'success_url': '', - 'errors': [u'Incorrect success url'] - }, - ) - @ddt.unpack - def test_post_errors(self, enterprise, success_url, errors): + def test_post_errors(self): """ Test errors are raised if incorrect data is POSTed. """ + incorrect_enterprise = '111' + errors = [ + u'Enterprise not found', + u'Select a valid choice. 111 is not one of the available choices.' + ] + self._login() - selected_enterprise = self.enterprise_choices[2][0] post_data = { - 'enterprise': enterprise or selected_enterprise, - 'success_url': success_url, + 'enterprise': incorrect_enterprise, } response = self.client.post(self.url, post_data) assert response.status_code == 400