Skip to content

Commit

Permalink
Merge pull request #622 from edx/ammar/remove-success-url-validation
Browse files Browse the repository at this point in the history
remove success url validation
  • Loading branch information
muhammad-ammar authored Nov 14, 2019
2 parents e8fa734 + dd7aad1 commit ef734db
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 52 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.rst
Original file line number Diff line number Diff line change
Expand Up @@ -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
---------------------

Expand Down
2 changes: 1 addition & 1 deletion enterprise/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
7 changes: 0 additions & 7 deletions enterprise/forms.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
10 changes: 5 additions & 5 deletions enterprise/static/enterprise/js/enterprise_selection_page.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,24 +6,24 @@ 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);

$.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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ <h2 class="select-enterprise-title">{{ select_enterprise_message_title|safe }}</

{% for hidden_field in form.hidden_fields %}
<div>
{{ hidden_field.errors }}
{{ hidden_field }}
</div>
{% endfor %}
Expand Down
11 changes: 7 additions & 4 deletions enterprise/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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):
Expand All @@ -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={
Expand All @@ -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):
Expand Down
13 changes: 2 additions & 11 deletions spec/javascripts/enterprise_select_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' );
Expand All @@ -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
Expand All @@ -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]);
});
Expand Down
32 changes: 9 additions & 23 deletions tests/test_enterprise/views/test_enterprise_selection.py
Original file line number Diff line number Diff line change
Expand Up @@ -113,20 +113,18 @@ 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:
response = self.client.post(self.url, post_data)
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
Expand All @@ -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
Expand Down

0 comments on commit ef734db

Please sign in to comment.