Andal.LND/edx-platform
4
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: Remove pointless Maintenance and Announcement apps (#35852)

Browse Source 
The Studio Maintenance app had two features:

* "Force Course Publish", which literally doesn't do anything. All it
  does is tell you what version *would* be seen by users *if* the course
  were to be published--no publishing actually occurs via this feature.

* "Announcements", which writes to the announcements_announcement
  database table, but doesn't actually display anywhere.

Having these pages in the platform is actively misleading and creates a
maintenance burden for edx-platform developers, so we remove them.

Note that this commit does not include a migration for the announcements
Django app. So, announcements_announcement table will not be deleted.
Given the small expected size of any past-authored announcements, we are
not worried about leaving them in the database perpetually.
This commit is contained in:
Kyle McCormick
2024-11-26 10:15:27 -05:00
committed by GitHub
parent bfa756b7c9
commit 9274852f2d
36 changed files with 5 additions and 1637 deletions
Download Patch File Download Diff File Expand all files Collapse all files

0
cms/djangoapps/maintenance/__init__.py
View File

311
cms/djangoapps/maintenance/tests.py
View File

@@ -1,311 +0,0 @@
"""
Tests for the maintenance app views.
"""
import json
import ddt
from django.conf import settings
from django.urls import reverse
from cms.djangoapps.contentstore.management.commands.utils import get_course_versions
from common.djangoapps.student.tests.factories import AdminFactory, UserFactory
from openedx.features.announcements.models import Announcement
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.tests.factories import CourseFactory, BlockFactory # lint-amnesty, pylint: disable=wrong-import-order
from .views import COURSE_KEY_ERROR_MESSAGES, MAINTENANCE_VIEWS
# This list contains URLs of all maintenance app views.
MAINTENANCE_URLS = [reverse(view['url']) for view in MAINTENANCE_VIEWS.values()]
class TestMaintenanceIndex(ModuleStoreTestCase):
"""
Tests for maintenance index view.
"""
def setUp(self):
super().setUp()
self.user = AdminFactory()
login_success = self.client.login(username=self.user.username, password=self.TEST_PASSWORD)
self.assertTrue(login_success)
self.view_url = reverse('maintenance:maintenance_index')
def test_maintenance_index(self):
"""
Test that maintenance index view lists all the maintenance app views.
"""
response = self.client.get(self.view_url)
self.assertContains(response, 'Maintenance', status_code=200)
# Check that all the expected links appear on the index page.
for url in MAINTENANCE_URLS:
self.assertContains(response, url, status_code=200)
@ddt.ddt
class MaintenanceViewTestCase(ModuleStoreTestCase):
"""
Base class for maintenance view tests.
"""
view_url = ''
def setUp(self):
super().setUp()
self.user = AdminFactory()
login_success = self.client.login(username=self.user.username, password=self.TEST_PASSWORD)
self.assertTrue(login_success)
def verify_error_message(self, data, error_message):
"""
Verify the response contains error message.
"""
response = self.client.post(self.view_url, data=data, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
self.assertContains(response, error_message, status_code=200)
def tearDown(self):
"""
Reverse the setup.
"""
self.client.logout()
super().tearDown()
@ddt.ddt
class MaintenanceViewAccessTests(MaintenanceViewTestCase):
"""
Tests for access control of maintenance views.
"""
@ddt.data(*MAINTENANCE_URLS)
def test_require_login(self, url):
"""
Test that maintenance app requires user login.
"""
# Log out then try to retrieve the page
self.client.logout()
response = self.client.get(url)
# Expect a redirect to the login page
redirect_url = '{login_url}?next={original_url}'.format(
login_url=settings.LOGIN_URL,
original_url=url,
)
# Studio login redirects to LMS login
self.assertRedirects(response, redirect_url, target_status_code=302)
@ddt.data(*MAINTENANCE_URLS)
def test_global_staff_access(self, url):
"""
Test that all maintenance app views are accessible to global staff user.
"""
response = self.client.get(url)
self.assertEqual(response.status_code, 200)
@ddt.data(*MAINTENANCE_URLS)
def test_non_global_staff_access(self, url):
"""
Test that all maintenance app views are not accessible to non-global-staff user.
"""
user = UserFactory(username='test', email='test@example.com', password=self.TEST_PASSWORD)
login_success = self.client.login(username=user.username, password=self.TEST_PASSWORD)
self.assertTrue(login_success)
response = self.client.get(url)
self.assertContains(
response,
f'Must be {settings.PLATFORM_NAME} staff to perform this action.',
status_code=403
)
@ddt.ddt
class TestForcePublish(MaintenanceViewTestCase):
"""
Tests for the force publish view.
"""
def setUp(self):
super().setUp()
self.view_url = reverse('maintenance:force_publish_course')
def setup_test_course(self):
"""
Creates the course and add some changes to it.
Returns:
course: a course object
"""
course = CourseFactory.create()
# Add some changes to course
chapter = BlockFactory.create(category='chapter', parent_location=course.location)
self.store.create_child(
self.user.id,
chapter.location,
'html',
block_id='html_component'
)
# verify that course has changes.
self.assertTrue(self.store.has_changes(self.store.get_item(course.location)))
return course
@ddt.data(
('', COURSE_KEY_ERROR_MESSAGES['empty_course_key']),
('edx', COURSE_KEY_ERROR_MESSAGES['invalid_course_key']),
('course-v1:e+d+X', COURSE_KEY_ERROR_MESSAGES['course_key_not_found']),
)
@ddt.unpack
def test_invalid_course_key_messages(self, course_key, error_message):
"""
Test all error messages for invalid course keys.
"""
# validate that course key contains error message
self.verify_error_message(
data={'course-id': course_key},
error_message=error_message
)
def test_already_published(self):
"""
Test that when a course is forcefully publish, we get a 'course is already published' message.
"""
course = self.setup_test_course()
# publish the course
source_store = modulestore()._get_modulestore_for_courselike(course.id) # pylint: disable=protected-access
source_store.force_publish_course(course.id, self.user.id, commit=True)
# now course is published, we should get `already published course` error.
self.verify_error_message(
data={'course-id': str(course.id)},
error_message='Course is already in published state.'
)
def verify_versions_are_different(self, course):
"""
Verify draft and published versions point to different locations.
Arguments:
course (object): a course object.
"""
# get draft and publish branch versions
versions = get_course_versions(str(course.id))
# verify that draft and publish point to different versions
self.assertNotEqual(versions['draft-branch'], versions['published-branch'])
def get_force_publish_course_response(self, course):
"""
Get force publish the course response.
Arguments:
course (object): a course object.
Returns:
response : response from force publish post view.
"""
# Verify versions point to different locations initially
self.verify_versions_are_different(course)
# force publish course view
data = {
'course-id': str(course.id)
}
response = self.client.post(self.view_url, data=data, HTTP_X_REQUESTED_WITH='XMLHttpRequest')
response_data = json.loads(response.content.decode('utf-8'))
return response_data
def test_force_publish_dry_run(self):
"""
Test that dry run does not publishes the course but shows possible outcome if force published is executed.
"""
course = self.setup_test_course()
response = self.get_force_publish_course_response(course)
self.assertIn('current_versions', response)
# verify that course still has changes as we just dry ran force publish course.
self.assertTrue(self.store.has_changes(self.store.get_item(course.location)))
# verify that both branch versions are still different
self.verify_versions_are_different(course)
@ddt.ddt
class TestAnnouncementsViews(MaintenanceViewTestCase):
"""
Tests for the announcements edit view.
"""
def setUp(self):
super().setUp()
self.admin = AdminFactory.create(
email='staff@edx.org',
username='admin',
password=self.TEST_PASSWORD
)
self.client.login(username=self.admin.username, password=self.TEST_PASSWORD)
self.non_staff_user = UserFactory.create(
email='test@edx.org',
username='test',
password=self.TEST_PASSWORD
)
def test_index(self):
"""
Test create announcement view
"""
url = reverse("maintenance:announcement_index")
response = self.client.get(url)
self.assertContains(response, '<div class="announcement-container">')
def test_create(self):
"""
Test create announcement view
"""
url = reverse("maintenance:announcement_create")
self.client.post(url, {"content": "Test Create Announcement", "active": True})
result = Announcement.objects.filter(content="Test Create Announcement").exists()
self.assertTrue(result)
def test_edit(self):
"""
Test edit announcement view
"""
announcement = Announcement.objects.create(content="test")
announcement.save()
url = reverse("maintenance:announcement_edit", kwargs={"pk": announcement.pk})
response = self.client.get(url)
self.assertContains(response, '<div class="wrapper-form announcement-container">')
self.client.post(url, {"content": "Test Edit Announcement", "active": True})
announcement = Announcement.objects.get(pk=announcement.pk)
self.assertEqual(announcement.content, "Test Edit Announcement")
def test_delete(self):
"""
Test delete announcement view
"""
announcement = Announcement.objects.create(content="Test Delete")
announcement.save()
url = reverse("maintenance:announcement_delete", kwargs={"pk": announcement.pk})
self.client.post(url)
result = Announcement.objects.filter(content="Test Edit Announcement").exists()
self.assertFalse(result)
def _test_403(self, viewname, kwargs=None):
url = reverse("maintenance:%s" % viewname, kwargs=kwargs)
response = self.client.get(url)
self.assertEqual(response.status_code, 403)
def test_authorization(self):
self.client.login(username=self.non_staff_user, password=self.TEST_PASSWORD)
announcement = Announcement.objects.create(content="Test Delete")
announcement.save()
self._test_403("announcement_index")
self._test_403("announcement_create")
self._test_403("announcement_edit", {"pk": announcement.pk})
self._test_403("announcement_delete", {"pk": announcement.pk})

25
cms/djangoapps/maintenance/urls.py
View File

@@ -1,25 +0,0 @@
"""
URLs for the maintenance app.
"""
from django.urls import path, re_path
from .views import (
AnnouncementCreateView,
AnnouncementDeleteView,
AnnouncementEditView,
AnnouncementIndexView,
ForcePublishCourseView,
MaintenanceIndexView
)
app_name = 'cms.djangoapps.maintenance'
urlpatterns = [
path('', MaintenanceIndexView.as_view(), name='maintenance_index'),
re_path(r'^force_publish_course/?$', ForcePublishCourseView.as_view(), name='force_publish_course'),
re_path(r'^announcements/(?P<page>\d+)?$', AnnouncementIndexView.as_view(), name='announcement_index'),
path('announcements/create', AnnouncementCreateView.as_view(), name='announcement_create'),
re_path(r'^announcements/edit/(?P<pk>\d+)?$', AnnouncementEditView.as_view(), name='announcement_edit'),
path('announcements/delete/<int:pk>', AnnouncementDeleteView.as_view(), name='announcement_delete'),
]

301
cms/djangoapps/maintenance/views.py
View File

@@ -1,301 +0,0 @@
"""
Views for the maintenance app.
"""
import logging
from django.core.validators import ValidationError
from django.db import transaction
from django.urls import reverse, reverse_lazy
from django.utils.decorators import method_decorator
from django.utils.translation import gettext as _
from django.views.generic import View
from django.views.generic.edit import CreateView, DeleteView, UpdateView
from django.views.generic.list import ListView
from opaque_keys import InvalidKeyError
from opaque_keys.edx.keys import CourseKey
from cms.djangoapps.contentstore.management.commands.utils import get_course_versions
from common.djangoapps.edxmako.shortcuts import render_to_response
from common.djangoapps.util.json_request import JsonResponse
from common.djangoapps.util.views import require_global_staff
from openedx.features.announcements.forms import AnnouncementForm
from openedx.features.announcements.models import Announcement
from xmodule.modulestore import ModuleStoreEnum # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.django import modulestore # lint-amnesty, pylint: disable=wrong-import-order
from xmodule.modulestore.exceptions import ItemNotFoundError # lint-amnesty, pylint: disable=wrong-import-order
log = logging.getLogger(__name__)
# This dict maintains all the views that will be used Maintenance app.
MAINTENANCE_VIEWS = {
'force_publish_course': {
'url': 'maintenance:force_publish_course',
'name': _('Force Publish Course'),
'slug': 'force_publish_course',
'description': _(
'Sometimes the draft and published branches of a course can get out of sync. Force publish course command '
'resets the published branch of a course to point to the draft branch, effectively force publishing the '
'course. This view dry runs the force publish command'
),
},
'announcement_index': {
'url': 'maintenance:announcement_index',
'name': _('Edit Announcements'),
'slug': 'announcement_index',
'description': _(
'This view shows the announcement editor to create or alter announcements that are shown on the right'
'side of the dashboard.'
),
},
}
COURSE_KEY_ERROR_MESSAGES = {
'empty_course_key': _('Please provide course id.'),
'invalid_course_key': _('Invalid course key.'),
'course_key_not_found': _('No matching course found.')
}
class MaintenanceIndexView(View):
"""
Index view for maintenance dashboard, used by global staff.
This view lists some commands/tasks that can be used to dry run or execute directly.
"""
@method_decorator(require_global_staff)
def get(self, request):
"""Render the maintenance index view. """
return render_to_response('maintenance/index.html', {
'views': MAINTENANCE_VIEWS,
})
class MaintenanceBaseView(View):
"""
Base class for Maintenance views.
"""
template = 'maintenance/container.html'
def __init__(self, view=None):
super().__init__()
self.context = {
'view': view if view else '',
'form_data': {},
'error': False,
'msg': ''
}
def render_response(self):
"""
A short method to render_to_response that renders response.
"""
if self.request.headers.get('x-requested-with') == 'XMLHttpRequest':
return JsonResponse(self.context)
return render_to_response(self.template, self.context)
@method_decorator(require_global_staff)
def get(self, request):
"""
Render get view.
"""
return self.render_response()
def validate_course_key(self, course_key, branch=ModuleStoreEnum.BranchName.draft):
"""
Validates the course_key that would be used by maintenance app views.
Arguments:
course_key (string): a course key
branch: a course locator branch, default value is ModuleStoreEnum.BranchName.draft .
values can be either ModuleStoreEnum.BranchName.draft or ModuleStoreEnum.BranchName.published.
Returns:
course_usage_key (CourseLocator): course usage locator
"""
if not course_key:
raise ValidationError(COURSE_KEY_ERROR_MESSAGES['empty_course_key'])
course_usage_key = CourseKey.from_string(course_key)
if not modulestore().has_course(course_usage_key):
raise ItemNotFoundError(COURSE_KEY_ERROR_MESSAGES['course_key_not_found'])
# get branch specific locator
course_usage_key = course_usage_key.for_branch(branch)
return course_usage_key
class ForcePublishCourseView(MaintenanceBaseView):
"""
View for force publishing state of the course, used by the global staff.
This view uses `force_publish_course` method of modulestore which publishes the draft state of the course. After
the course has been forced published, both draft and publish draft point to same location.
"""
def __init__(self):
super().__init__(MAINTENANCE_VIEWS['force_publish_course'])
self.context.update({
'current_versions': [],
'updated_versions': [],
'form_data': {
'course_id': '',
'is_dry_run': True
}
})
def get_course_branch_versions(self, versions):
"""
Returns a dict containing unicoded values of draft and published draft versions.
"""
return {
'draft-branch': str(versions['draft-branch']),
'published-branch': str(versions['published-branch'])
}
@transaction.atomic
@method_decorator(require_global_staff)
def post(self, request):
"""
This method force publishes a course if dry-run argument is not selected. If dry-run is selected, this view
shows possible outcome if the `force_publish_course` modulestore method is executed.
Arguments:
course_id (string): a request parameter containing course id
is_dry_run (string): a request parameter containing dry run value.
It is obtained from checkbox so it has either values 'on' or ''.
"""
course_id = request.POST.get('course-id')
self.context.update({
'form_data': {
'course_id': course_id
}
})
try:
course_usage_key = self.validate_course_key(course_id)
except InvalidKeyError:
self.context['error'] = True
self.context['msg'] = COURSE_KEY_ERROR_MESSAGES['invalid_course_key']
except ItemNotFoundError as exc:
self.context['error'] = True
self.context['msg'] = str(exc)
except ValidationError as exc:
self.context['error'] = True
self.context['msg'] = str(exc)
if self.context['error']:
return self.render_response()
source_store = modulestore()._get_modulestore_for_courselike(course_usage_key) # pylint: disable=protected-access
if not hasattr(source_store, 'force_publish_course'):
self.context['msg'] = _('Force publishing course is not supported with old mongo courses.')
log.warning(
'Force publishing course is not supported with old mongo courses. \
%s attempted to force publish the course %s.',
request.user,
course_id,
exc_info=True
)
return self.render_response()
current_versions = self.get_course_branch_versions(get_course_versions(course_id))
# if publish and draft are NOT different
if current_versions['published-branch'] == current_versions['draft-branch']:
self.context['msg'] = _('Course is already in published state.')
log.warning(
'Course is already in published state. %s attempted to force publish the course %s.',
request.user,
course_id,
exc_info=True
)
return self.render_response()
self.context['current_versions'] = current_versions
log.info(
'%s dry ran force publish the course %s.',
request.user,
course_id,
exc_info=True
)
return self.render_response()
class AnnouncementBaseView(View):
"""
Base view for Announcements pages
"""
@method_decorator(require_global_staff)
def dispatch(self, request, *args, **kwargs):
return super().dispatch(request, *args, **kwargs)
class AnnouncementIndexView(ListView, MaintenanceBaseView):
"""
View for viewing the announcements shown on the dashboard, used by the global staff.
"""
model = Announcement
object_list = Announcement.objects.order_by('-active')
context_object_name = 'announcement_list'
paginate_by = 8
def __init__(self):
super().__init__(MAINTENANCE_VIEWS['announcement_index'])
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context['view'] = MAINTENANCE_VIEWS['announcement_index']
return context
@method_decorator(require_global_staff)
def get(self, request, *args, **kwargs):
context = self.get_context_data()
return render_to_response(self.template, context)
class AnnouncementEditView(UpdateView, AnnouncementBaseView):
"""
View for editing an announcement.
"""
model = Announcement
form_class = AnnouncementForm
success_url = reverse_lazy('maintenance:announcement_index')
template_name = '/maintenance/_announcement_edit.html'
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context['action_url'] = reverse('maintenance:announcement_edit', kwargs={'pk': context['announcement'].pk})
return context
class AnnouncementCreateView(CreateView, AnnouncementBaseView):
"""
View for creating an announcement.
"""
model = Announcement
form_class = AnnouncementForm
success_url = reverse_lazy('maintenance:announcement_index')
template_name = '/maintenance/_announcement_edit.html'
def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)
context['action_url'] = reverse('maintenance:announcement_create')
return context
class AnnouncementDeleteView(DeleteView, AnnouncementBaseView):
"""
View for deleting an announcement.
"""
model = Announcement
success_url = reverse_lazy('maintenance:announcement_index')
template_name = '/maintenance/_announcement_delete.html'