From 02c46b42e4f6c1b8a22358c8ed9a4184df3bb961 Mon Sep 17 00:00:00 2001 From: Waheed Ahmed Date: Mon, 27 Feb 2017 17:47:44 +0500 Subject: [PATCH] Publisher cleanup. ECOM-7314 --- course_discovery/apps/publisher/models.py | 4 - .../apps/publisher/tests/test_model.py | 6 - .../apps/publisher/tests/test_views.py | 169 ------------------ course_discovery/apps/publisher/urls.py | 2 - course_discovery/apps/publisher/views.py | 40 +---- .../publisher_comments/tests/test_views.py | 145 --------------- .../apps/publisher_comments/urls.py | 2 - .../apps/publisher_comments/views.py | 39 ---- 8 files changed, 2 insertions(+), 405 deletions(-) delete mode 100644 course_discovery/apps/publisher_comments/tests/test_views.py delete mode 100644 course_discovery/apps/publisher_comments/views.py diff --git a/course_discovery/apps/publisher/models.py b/course_discovery/apps/publisher/models.py index f105b623f6..70472cf9c1 100644 --- a/course_discovery/apps/publisher/models.py +++ b/course_discovery/apps/publisher/models.py @@ -344,10 +344,6 @@ class Seat(TimeStampedModel, ChangedByMixin): def __str__(self): return '{course}: {type}'.format(course=self.course_run.course.title, type=self.type) - @property - def post_back_url(self): - return reverse('publisher:publisher_seats_edit', kwargs={'pk': self.id}) - @property def is_valid_seat(self): return self.type == self.AUDIT or self.type in [self.VERIFIED, self.PROFESSIONAL] and self.price > 0 diff --git a/course_discovery/apps/publisher/tests/test_model.py b/course_discovery/apps/publisher/tests/test_model.py index ed0cd45a3d..4f1a2b0688 100644 --- a/course_discovery/apps/publisher/tests/test_model.py +++ b/course_discovery/apps/publisher/tests/test_model.py @@ -320,12 +320,6 @@ def test_str(self): ) ) - def test_post_back_url(self): - self.assertEqual( - self.seat.post_back_url, - reverse('publisher:publisher_seats_edit', kwargs={'pk': self.seat.id}) - ) - class UserAttributeTests(TestCase): """ Tests for the publisher `UserAttribute` model. """ diff --git a/course_discovery/apps/publisher/tests/test_views.py b/course_discovery/apps/publisher/tests/test_views.py index 62357cd393..75ba17df66 100644 --- a/course_discovery/apps/publisher/tests/test_views.py +++ b/course_discovery/apps/publisher/tests/test_views.py @@ -493,175 +493,6 @@ def test_create_course_run_and_seat(self): self.assertEqual(new_seat.course_run.course.course_team_admin, new_user) -class SeatsCreateUpdateViewTests(TestCase): - """ Tests for the publisher `CreateSeatView` and `UpdateSeatView`. """ - - def setUp(self): - super(SeatsCreateUpdateViewTests, self).setUp() - self.seat = factories.SeatFactory(type=Seat.PROFESSIONAL, credit_hours=0) - self.organization_extension = factories.OrganizationExtensionFactory() - self.seat.course_run.course.organizations.add(self.organization_extension.organization) - self.seat_dict = model_to_dict(self.seat) - self.seat_dict.pop('upgrade_deadline') - self.user = UserFactory() - self.site = Site.objects.get(pk=settings.SITE_ID) - self.client.login(username=self.user.username, password=USER_PASSWORD) - self.seat_edit_url = reverse('publisher:publisher_seats_edit', kwargs={'pk': self.seat.id}) - - def test_seat_form_without_login(self): - """ Verify that user can't access new seat form page when not logged in. """ - self.client.logout() - response = self.client.get(reverse('publisher:publisher_seats_new')) - - self.assertRedirects( - response, - expected_url='{url}?next={next}'.format( - url=reverse('login'), - next=reverse('publisher:publisher_seats_new') - ), - status_code=302, - target_status_code=302 - ) - - def test_seat_view_page(self): - """ Verify that we can open new seat page. """ - response = self.client.get(reverse('publisher:publisher_seats_new')) - # Assert that we can load seat page. - self.assertEqual(response.status_code, 200) - - def test_create_seat(self): - """ Verify that we can create a new seat. """ - seat_price = 670.00 - self.seat_dict['price'] = seat_price - response = self.client.post(reverse('publisher:publisher_seats_new'), self.seat_dict) - seat = Seat.objects.get(course_run=self.seat.course_run, price=seat_price) - - self.user.groups.add(self.organization_extension.group) - # edit permission require on seat edit page only. - assign_perm( - OrganizationExtension.EDIT_COURSE_RUN, self.organization_extension.group, self.organization_extension - ) - self.assertRedirects( - response, - expected_url=reverse('publisher:publisher_seats_edit', kwargs={'pk': seat.id}), - status_code=302, - target_status_code=200 - ) - - self.assertEqual(seat.price, seat_price) - - def test_update_seat_with_admin(self): - """ Verify that publisher admin can update an existing seat. """ - self.user.groups.add(Group.objects.get(name=ADMIN_GROUP_NAME)) - self.assertEqual(self.seat.type, Seat.PROFESSIONAL) - updated_seat_price = 470.00 - self.seat_dict['price'] = updated_seat_price - self.seat_dict['type'] = Seat.VERIFIED - self.assertNotEqual(self.seat.price, updated_seat_price) - self.assertNotEqual(self.seat.changed_by, self.user) - response = self.client.post( - reverse('publisher:publisher_seats_edit', kwargs={'pk': self.seat.id}), - self.seat_dict - ) - - self.assertRedirects( - response, - expected_url=self.seat_edit_url, - status_code=302, - target_status_code=200 - ) - - seat = Seat.objects.get(id=self.seat.id) - # Assert that seat is updated. - self.assertEqual(seat.price, updated_seat_price) - self.assertEqual(seat.changed_by, self.user) - self.assertEqual(seat.type, Seat.VERIFIED) - - self.seat_dict['type'] = Seat.HONOR - response = self.client.post(self.seat_edit_url, self.seat_dict) - seat = Seat.objects.get(id=self.seat.id) - # Assert that we can change seat type. - self.assertEqual(seat.type, Seat.HONOR) - - self.assertRedirects( - response, - expected_url=self.seat_edit_url, - status_code=302, - target_status_code=200 - ) - - # add new and check the comment on edit page. - comment = CommentFactory(content_object=self.seat, user=self.user, site=self.site) - response = self.client.get(self.seat_edit_url) - self.assertContains(response, 'Comment:') - self.assertContains(response, comment.comment) - - def test_edit_seat_page_with_non_staff(self): - """ Verify that non internal user can't access seat edit page without permission. """ - non_internal_user, __ = create_non_staff_user_and_login(self) - response = self.client.get(reverse('publisher:publisher_seats_edit', kwargs={'pk': self.seat.id})) - - self.assertEqual(response.status_code, 403) - - non_internal_user.groups.add(Group.objects.get(name=INTERNAL_USER_GROUP_NAME)) - response = self.client.get(reverse('publisher:publisher_seats_edit', kwargs={'pk': self.seat.id})) - - self.assertEqual(response.status_code, 200) - - def test_update_seat_with_internal_user(self): - """ Tests update seat for internal user. """ - non_internal_user, __ = create_non_staff_user_and_login(self) - - self.assertEqual(self.seat.type, Seat.PROFESSIONAL) - updated_seat_price = 470.00 - self.seat_dict['price'] = updated_seat_price - self.seat_dict['type'] = Seat.VERIFIED - self.assertNotEqual(self.seat.price, updated_seat_price) - - response = self.client.post( - reverse('publisher:publisher_seats_edit', kwargs={'pk': self.seat.id}), - self.seat_dict - ) - - # verify that non internal user can't update course seat without permission - self.assertEqual(response.status_code, 403) - - non_internal_user.groups.add(Group.objects.get(name=INTERNAL_USER_GROUP_NAME)) - - response = self.client.post( - reverse('publisher:publisher_seats_edit', kwargs={'pk': self.seat.id}), - self.seat_dict - ) - - self.assertRedirects( - response, - expected_url=reverse('publisher:publisher_seats_edit', kwargs={'pk': self.seat.id}), - status_code=302, - target_status_code=200 - ) - - seat = Seat.objects.get(id=self.seat.id) - # Assert that seat is updated. - self.assertEqual(seat.price, updated_seat_price) - self.assertEqual(seat.type, Seat.VERIFIED) - - self.seat_dict['type'] = Seat.HONOR - response = self.client.post( - reverse('publisher:publisher_seats_edit', kwargs={'pk': self.seat.id}), - self.seat_dict - ) - seat = Seat.objects.get(id=self.seat.id) - # Assert that we can change seat type. - self.assertEqual(seat.type, Seat.HONOR) - - self.assertRedirects( - response, - expected_url=reverse('publisher:publisher_seats_edit', kwargs={'pk': self.seat.id}), - status_code=302, - target_status_code=200 - ) - - @ddt.ddt class CourseRunDetailTests(TestCase): """ Tests for the course-run detail view. """ diff --git a/course_discovery/apps/publisher/urls.py b/course_discovery/apps/publisher/urls.py index 9411e17660..7505ff739d 100644 --- a/course_discovery/apps/publisher/urls.py +++ b/course_discovery/apps/publisher/urls.py @@ -19,8 +19,6 @@ ), url(r'^course_runs/(?P\d+)/$', views.CourseRunDetailView.as_view(), name='publisher_course_run_detail'), url(r'^course_runs/(?P\d+)/edit/$', views.CourseRunEditView.as_view(), name='publisher_course_runs_edit'), - url(r'^seats/new/$', views.CreateSeatView.as_view(), name='publisher_seats_new'), - url(r'^seats/(?P\d+)/edit/$', views.UpdateSeatView.as_view(), name='publisher_seats_edit'), url( r'^user/toggle/email_settings/$', views.ToggleEmailNotification.as_view(), diff --git a/course_discovery/apps/publisher/views.py b/course_discovery/apps/publisher/views.py index 7a1598073c..0418168d4f 100644 --- a/course_discovery/apps/publisher/views.py +++ b/course_discovery/apps/publisher/views.py @@ -20,9 +20,9 @@ from course_discovery.apps.core.models import User from course_discovery.apps.publisher import emails, mixins from course_discovery.apps.publisher.choices import CourseRunStateChoices, CourseStateChoices, PublisherUserRole -from course_discovery.apps.publisher.forms import CustomCourseForm, CustomCourseRunForm, CustomSeatForm, SeatForm +from course_discovery.apps.publisher.forms import CustomCourseForm, CustomCourseRunForm, CustomSeatForm from course_discovery.apps.publisher.models import (Course, CourseRun, CourseRunState, CourseState, CourseUserRole, - OrganizationExtension, Seat, UserAttributes) + OrganizationExtension, UserAttributes) from course_discovery.apps.publisher.utils import (get_internal_users, is_internal_user, is_project_coordinator_user, is_publisher_admin, make_bread_crumbs) from course_discovery.apps.publisher.wrappers import CourseRunWrapper @@ -30,8 +30,6 @@ logger = logging.getLogger(__name__) -SEATS_HIDDEN_FIELDS = ['price', 'currency', 'upgrade_deadline', 'credit_provider', 'credit_hours'] - ROLE_WIDGET_HEADINGS = { PublisherUserRole.PartnerManager: _('PARTNER MANAGER'), PublisherUserRole.ProjectCoordinator: _('PROJECT COORDINATOR'), @@ -614,40 +612,6 @@ def post(self, request, *args, **kwargs): return render(request, self.template_name, context, status=400) -class CreateSeatView(mixins.LoginRequiredMixin, mixins.FormValidMixin, CreateView): - """ Create Seat View.""" - model = Seat - form_class = SeatForm - template_name = 'publisher/seat_form.html' - success_url = 'publisher:publisher_seats_edit' - - def get_context_data(self, **kwargs): - context = super(CreateSeatView, self).get_context_data(**kwargs) - context['hidden_fields'] = SEATS_HIDDEN_FIELDS - return context - - def get_success_url(self): - return reverse(self.success_url, kwargs={'pk': self.object.id}) - - -class UpdateSeatView(mixins.LoginRequiredMixin, mixins.PublisherPermissionMixin, mixins.FormValidMixin, UpdateView): - """ Update Seat View.""" - model = Seat - form_class = SeatForm - permission = OrganizationExtension.EDIT_COURSE_RUN - template_name = 'publisher/seat_form.html' - success_url = 'publisher:publisher_seats_edit' - - def get_context_data(self, **kwargs): - context = super(UpdateSeatView, self).get_context_data(**kwargs) - context['hidden_fields'] = SEATS_HIDDEN_FIELDS - context['comment_object'] = self.object - return context - - def get_success_url(self): - return reverse(self.success_url, kwargs={'pk': self.object.id}) - - class ToggleEmailNotification(mixins.LoginRequiredMixin, View): """ Toggle User Email Notification Settings.""" diff --git a/course_discovery/apps/publisher_comments/tests/test_views.py b/course_discovery/apps/publisher_comments/tests/test_views.py deleted file mode 100644 index af1d2e6dc7..0000000000 --- a/course_discovery/apps/publisher_comments/tests/test_views.py +++ /dev/null @@ -1,145 +0,0 @@ -from django.conf import settings -from django.contrib.sites.models import Site -from django.core import mail -from django.core.urlresolvers import reverse -from django.forms import model_to_dict -from django.test import TestCase - -from course_discovery.apps.core.tests.factories import USER_PASSWORD, UserFactory -from course_discovery.apps.course_metadata.tests import toggle_switch -from course_discovery.apps.publisher.choices import PublisherUserRole -from course_discovery.apps.publisher.models import Seat -from course_discovery.apps.publisher.tests import factories -from course_discovery.apps.publisher_comments.tests.factories import CommentFactory - - -# pylint: disable=no-member -class CommentsTests(TestCase): - """ Tests for the Comment functionality on `Courser`, `CourseRun` And `Seat` edit pages. """ - def setUp(self): - super(CommentsTests, self).setUp() - self.user = UserFactory(is_staff=True, is_superuser=True) - self.organization_extension = factories.OrganizationExtensionFactory() - - self.client.login(username=self.user.username, password=USER_PASSWORD) - self.site = Site.objects.get(pk=settings.SITE_ID) - self.course_edit_page = 'publisher:publisher_courses_edit' - self.course_run_edit_page = 'publisher:publisher_course_runs_edit' - self.seat_edit_page = 'publisher:publisher_seats_edit' - self.edit_comment_page = 'publisher_comments:comment_edit' - - self.seat = factories.SeatFactory(type=Seat.PROFESSIONAL, credit_hours=0) - self.course_run = self.seat.course_run - self.course = self.course_run.course - self.course.organizations.add(self.organization_extension.organization) - - # assign the role against a course - factories.CourseUserRoleFactory( - course=self.course, role=PublisherUserRole.MarketingReviewer, user=self.user - ) - - toggle_switch('enable_publisher_email_notifications', True) - - def test_comment_edit_with_seat(self): - """ Verify that only comments attached with specific seat appears on edited page. """ - comments = self._generate_comments_for_all_content_types() - response = self.client.get(reverse(self.seat_edit_page, kwargs={'pk': self.seat.id})) - self.assertContains(response, comments.get(self.seat).comment) - self.assertNotContains(response, comments.get(self.course).comment) - self.assertNotContains(response, comments.get(self.course_run).comment) - - def test_edit_course_comment(self): - """ Verify that course comment can be edited. """ - self._edit_comment_page( - self.course, reverse(self.course_edit_page, kwargs={'pk': self.course.id}) - ) - - def test_edit_seat_comment(self): - """ Verify that seat comment can be edited. """ - self._edit_comment_page( - self.seat, reverse(self.seat_edit_page, kwargs={'pk': self.seat.id}) - ) - - def test_mail_outbox_count(self): - """ Verify that separate emails send for adding and editing the comment . """ - self._edit_comment_page( - self.course, reverse(self.course_edit_page, kwargs={'pk': self.course.id}) - ) - - # mail has 2 emails one due to newly added comment and other is due to editing. - self.assertEqual(len(mail.outbox), 2) - - def test_edit_comment_of_other_user(self): - """ Verify that comment can be edited by the comment author only. """ - comment = self._generate_comment(content_object=self.course, user=self.user) - comment_url = reverse(self.edit_comment_page, kwargs={'pk': comment.id}) - response = self.client.get(comment_url) - self.assertEqual(response.status_code, 200) - - # logout and login with other user. - self.client.logout() - user = UserFactory(is_staff=True, is_superuser=True) - self.client.login(username=user.username, password=USER_PASSWORD) - response = self.client.get(reverse(self.edit_comment_page, kwargs={'pk': comment.id})) - self.assertEqual(response.status_code, 404) - - def _edit_comment_page(self, content_object, expected_url): - """ DRY method for posting the edited comment.""" - comment = self._generate_comment(content_object=content_object, user=self.user) - comment_url = reverse(self.edit_comment_page, kwargs={'pk': comment.id}) - - response = self.client.get(comment_url) - self._assert_edit_comment(response, comment) - - new_comment = "This is updated comment" - content_object_dict = model_to_dict(comment) - content_object_dict['comment'] = new_comment - response = self.client.post(comment_url, content_object_dict) - self.assertRedirects( - response, - expected_url=expected_url, - status_code=302, target_status_code=200 - ) - - response = self.client.get(comment_url) - self.assertContains(response, new_comment) - - def _generate_comment(self, content_object, user): - """ DRY method to generate the comment.""" - return CommentFactory(content_object=content_object, user=user, site=self.site) - - def _assert_edit_comment(self, response, comment): - """ DRY method for asserting the edited comment page.""" - self.assertContains(response, 'Edit Comment') - self.assertContains(response, 'Submit date') - self.assertContains(response, comment.comment) - self.assertContains(response, comment.submit_date) - - # assert the customize fields exists in comment object - self.assertTrue(hasattr(comment, 'modified')) - - def _generate_comments_for_all_content_types(self): - """ DRY method generate the comments for all available content types""" - data = {} - for content in [self.course, self.course_run, self.seat]: - data[content] = self._generate_comment(content_object=content, user=self.user) - - return data - - def _add_assert_multiple_comments(self, content_object, page_path): - """ DRY method to add comments on edit page for specific object. """ - response = self.client.get(reverse(page_path, kwargs={'pk': content_object.id})) - self.assertContains(response, 'Total Comments 0') - - comments = [] - for __ in range(1, 2): - comments.append(self._generate_comment(content_object=content_object, user=self.user)) - - # assert emails send - self.assertEqual(len(mail.outbox), 1) - - response = self.client.get(reverse(page_path, kwargs={'pk': content_object.id})) - for comment in comments: - self.assertContains(response, comment.comment) - - self.assertContains(response, 'Total Comments 1') diff --git a/course_discovery/apps/publisher_comments/urls.py b/course_discovery/apps/publisher_comments/urls.py index 11c0427a7d..46d08fe59d 100644 --- a/course_discovery/apps/publisher_comments/urls.py +++ b/course_discovery/apps/publisher_comments/urls.py @@ -3,9 +3,7 @@ """ from django.conf.urls import include, url -from course_discovery.apps.publisher_comments import views urlpatterns = [ url(r'^api/', include('course_discovery.apps.publisher_comments.api.urls', namespace='api')), - url(r'^(?P\d+)/edit/$', views.UpdateCommentView.as_view(), name='comment_edit'), ] diff --git a/course_discovery/apps/publisher_comments/views.py b/course_discovery/apps/publisher_comments/views.py deleted file mode 100644 index 3f3a83eb64..0000000000 --- a/course_discovery/apps/publisher_comments/views.py +++ /dev/null @@ -1,39 +0,0 @@ -""" -Customize custom views. -""" -from django.core.urlresolvers import reverse -from django.http import Http404, HttpResponseRedirect -from django.views.generic.edit import UpdateView - -from course_discovery.apps.publisher_comments.forms import CommentEditForm -from course_discovery.apps.publisher_comments.models import Comments - - -# pylint: disable=attribute-defined-outside-init -class UpdateCommentView(UpdateView): - """ Update Comment View.""" - model = Comments - form_class = CommentEditForm - template_name = 'comments/edit_comment.html' - success_url = 'publisher:publisher_seats_edit' - - def get(self, request, *args, **kwargs): - self.object = self.get_object() - if request.user != self.object.user: - raise Http404 - return super(UpdateCommentView, self).get(request, *args, **kwargs) - - def form_valid(self, form): - self.object = form.save() - return HttpResponseRedirect(self.get_success_url()) - - def get_success_url(self): - url = reverse('publisher:publisher_seats_edit', kwargs={'pk': self.object.object_pk}) - if self.object.content_type.model == 'seat': - url = url - elif self.object.content_type.model == 'course': - url = reverse('publisher:publisher_courses_edit', kwargs={'pk': self.object.object_pk}) - elif self.object.content_type.model == 'courserun': - url = reverse('publisher:publisher_course_runs_edit', kwargs={'pk': self.object.object_pk}) - - return url