diff --git a/admin/templates/base.html b/admin/templates/base.html index fc42b93314c..f7a62ab2bcb 100644 --- a/admin/templates/base.html +++ b/admin/templates/base.html @@ -168,8 +168,8 @@ {% endif %} {% if user.is_superuser %} -
  • - +
  • + {% trans "User Identification Information" %}
    • diff --git a/admin/templates/user_identification_information/user_identification_details.html b/admin/templates/user_identification_information/user_identification_details.html index c49c06c78ed..d618f816098 100644 --- a/admin/templates/user_identification_information/user_identification_details.html +++ b/admin/templates/user_identification_information/user_identification_details.html @@ -14,7 +14,7 @@

    {% trans "User details" %}

    {% if user.is_superuser %} - + {% trans "Return" %} {% else %} diff --git a/admin/templates/user_identification_information/user_identification_institution_list.html b/admin/templates/user_identification_information/user_identification_institution_list.html new file mode 100644 index 00000000000..0589acb2eaa --- /dev/null +++ b/admin/templates/user_identification_information/user_identification_institution_list.html @@ -0,0 +1,48 @@ +{% extends "base.html" %} +{% load i18n %} +{% load static %} + +{% load render_bundle from webpack_loader %} + +{% block top_includes %} + +{% endblock %} + +{% block title %} + {% trans "List of Institutions" %} +{% endblock title %} + +{% block content %} +

    {% trans "List of Institutions" %}

    + + {% include "util/pagination.html" with items=page status=status %} + + + + + + + + + + {% for institution in institutions %} + + + + + + {% endfor %} + +
    {% trans "Logo" %}{% trans "Name" %}{% trans "Description" %}
    + + + + + {{ institution.name }} + {{ institution.description | safe }}
    + {% if not institutions|length %} +

    {% trans "No results found" %}

    + {% endif %} +{% endblock content %} diff --git a/admin/templates/user_identification_information/user_identification_list.html b/admin/templates/user_identification_information/user_identification_list.html index c9d7f9d286f..0f6c2fdfea0 100644 --- a/admin/templates/user_identification_information/user_identification_list.html +++ b/admin/templates/user_identification_information/user_identification_list.html @@ -123,11 +123,9 @@ }

    {% trans "User Identification Information" %}

    -{% if user.is_superuser == False %} -

    - {% blocktrans %}{{ institution_name }}{% endblocktrans %} -

    -{% endif %} +

    + {% blocktrans %}{{ institution_name }}{% endblocktrans %} +

    {% trans "Export All Users Statistics (CSV)" %} {% else %} diff --git a/admin/user_identification_information/urls.py b/admin/user_identification_information/urls.py index 9c72e669250..9eec7eced0d 100644 --- a/admin/user_identification_information/urls.py +++ b/admin/user_identification_information/urls.py @@ -5,9 +5,11 @@ app_name = 'admin' urlpatterns = [ - url(r'^$', views.UserIdentificationListView.as_view(), + url(r'^institutions/$', views.UserIdentificationInstitutionListView.as_view(), + name='user_identification_institutions'), + url(r'^institutions/(?P[0-9]+)/$', views.UserIdentificationListView.as_view(), name='user_identification_list'), - url(r'^csvexport/$', views.ExportFileCSVView.as_view(), + url(r'^institutions/(?P[0-9]+)/csvexport/$', views.ExportFileCSVView.as_view(), name='user_identification_export_csv'), url(r'^(?P[a-z0-9]+)/$', views.UserIdentificationDetailView.as_view(), name='user_identification_detail'), diff --git a/admin/user_identification_information/views.py b/admin/user_identification_information/views.py index d4adf269ea5..d51cb930ac4 100644 --- a/admin/user_identification_information/views.py +++ b/admin/user_identification_information/views.py @@ -7,6 +7,7 @@ from django.http import HttpResponse from django.views.generic import ListView +from admin.base import settings from admin.base.views import GuidView from admin.rdm.utils import RdmPermissionMixin from admin.user_identification_information.utils import ( @@ -14,12 +15,44 @@ get_list_extend_storage ) from api.base import settings as api_settings -from osf.models import OSFUser, UserQuota, Email +from osf.models import OSFUser, UserQuota, Email, Institution from website.util import quota from datetime import datetime from django.contrib.auth.mixins import UserPassesTestMixin +class UserIdentificationInstitutionListView(RdmPermissionMixin, UserPassesTestMixin, ListView): + """ Institution list page for User Identification Information menu. """ + template_name = 'user_identification_information/user_identification_institution_list.html' + paginate_by = 25 + ordering = 'name' + raise_exception = True + model = Institution + + def test_func(self): + """ Check user permissions """ + if not self.is_authenticated: + # If user is not authenticated then redirect to login page + self.raise_exception = False + return False + # Only allow super administrators to access this view + return self.is_super_admin + + def get_queryset(self): + """ GET: set to self.object_list """ + return Institution.objects.filter(is_deleted=False).order_by(self.ordering) + + def get_context_data(self, **kwargs): + """ Get context for this view """ + query_set = kwargs.pop('object_list', self.object_list) + page_size = self.get_paginate_by(query_set) + paginator, page, query_set, is_paginated = self.paginate_queryset(query_set, page_size) + kwargs.setdefault('institutions', query_set) + kwargs.setdefault('page', page) + kwargs.setdefault('logohost', settings.OSF_URL) + return super(UserIdentificationInstitutionListView, self).get_context_data(**kwargs) + + class UserIdentificationInformationListView(ListView): def get_user_quota_info(self, user, storage_type, extend_storage=''): @@ -65,8 +98,15 @@ def get_context_data(self, **kwargs): self.paginator, self.page, self.query_set, self.is_paginated = \ self.paginate_queryset(self.query_set, self.page_size) kwargs['requested_user'] = self.request.user - kwargs['institution_name'] = self.request.user.affiliated_institutions.first().name \ - if self.request.user.is_superuser is False else None + if self.request.user.is_superuser is False: + kwargs['institution_name'] = self.request.user.affiliated_institutions.first().name + else: + institution_id = self.kwargs.get('institution_id') + institution = Institution.objects.filter(id=institution_id).first() + if institution is None: + raise Http404('Institution not found') + kwargs['institution_name'] = institution.name + kwargs['institution_id'] = institution.id kwargs['users'] = self.query_set kwargs['page'] = self.page kwargs['order_by'] = self.get_order_by() @@ -97,6 +137,7 @@ def test_func(self): """check user permissions""" # login check if not self.is_authenticated: + self.raise_exception = False return False # permitted if superuser @@ -112,7 +153,11 @@ def user_list(self): if institution is not None: queryset = OSFUser.objects.filter(affiliated_institutions=institution.id).order_by('id') else: - queryset = OSFUser.objects.all().order_by('id') + institution_id = self.kwargs.get('institution_id') + institution = Institution.objects.filter(id=institution_id).first() + if institution is None: + raise Http404('Institution not found') + queryset = OSFUser.objects.filter(affiliated_institutions=institution_id).order_by('id') dict_users_list = get_list_extend_storage() @@ -219,6 +264,7 @@ def test_func(self): """check user permissions""" # login check if not self.is_authenticated: + self.raise_exception = False return False # permitted if superuser @@ -244,7 +290,11 @@ def get(self, request, **kwargs): if institution is not None: queryset = OSFUser.objects.filter(affiliated_institutions=institution.id).order_by('id') else: - queryset = OSFUser.objects.all().order_by('id') + institution_id = self.kwargs.get('institution_id') + institution = Institution.objects.filter(id=institution_id).first() + if institution is None: + raise Http404('Institution not found') + queryset = OSFUser.objects.filter(affiliated_institutions=institution_id).order_by('id') dict_users_list = get_list_extend_storage() for user in queryset: diff --git a/admin_tests/user_identification_information/test_views.py b/admin_tests/user_identification_information/test_views.py index fb4db74ad10..4ac55fa48dc 100644 --- a/admin_tests/user_identification_information/test_views.py +++ b/admin_tests/user_identification_information/test_views.py @@ -24,6 +24,78 @@ pytestmark = pytest.mark.django_db +class TestUserIdentificationInstitutionListView(AdminTestCase): + def setUp(self): + """Set up test data for all test methods""" + super(TestUserIdentificationInstitutionListView, self).setUp() + self.user = AuthUserFactory() + self.institutions = [InstitutionFactory(), InstitutionFactory()] + self.request = RequestFactory().get('/fake_path') + self.view = views.UserIdentificationInstitutionListView() + self.view = setup_user_view(self.view, self.request, user=self.user) + + def tearDown(self): + """Clean up actions after a test case""" + super(TestUserIdentificationInstitutionListView, self).tearDown() + self.user.delete() + for institution in self.institutions: + institution.delete() + + def test_super_admin_login(self): + """Test superuser login""" + self.request.user.is_superuser = True + self.request.user.is_staff = True + nt.assert_true(self.view.test_func()) + + def test_admin_login(self): + """Test institution administrator login""" + self.request.user.is_superuser = False + self.request.user.is_staff = True + nt.assert_false(self.view.test_func()) + nt.assert_true(self.view.raise_exception) + + def test_user_login(self): + """Test user login""" + self.request.user.is_superuser = False + self.request.user.is_staff = False + nt.assert_false(self.view.test_func()) + nt.assert_true(self.view.raise_exception) + + def test_non_active_user_login(self): + """Test invalid user login""" + self.request.user.is_active = False + nt.assert_false(self.view.test_func()) + nt.assert_true(self.view.raise_exception) + + def test_anonymous_login(self): + """Test anonymous user login""" + self.request.user = AnonymousUser() + nt.assert_false(self.view.test_func()) + nt.assert_false(self.view.raise_exception) + + def test_get(self, *args, **kwargs): + """Test GET method""" + self.request.user.is_superuser = True + self.request.user.is_staff = True + res = self.view.get(self.request, *args, **kwargs) + nt.assert_equal(res.status_code, 200) + + def test_get_queryset(self): + """Test get_queryset method""" + results = self.view.get_queryset() + nt.assert_equal(len(results), len(self.institutions)) + + def test_get_context_data(self): + """Test get_context_data method""" + self.view.object_list = self.view.get_queryset() + results = self.view.get_context_data() + nt.assert_is_instance(results, dict) + nt.assert_in('institutions', results) + nt.assert_in('page', results) + nt.assert_in('logohost', results) + nt.assert_equal(len(results['institutions']), len(self.institutions)) + + class TestUserIdentificationInformationListView(AdminTestCase): def setUp(self): @@ -31,6 +103,7 @@ def setUp(self): self.user2 = AuthUserFactory(fullname='Test User2') self.user3 = AuthUserFactory(fullname='Test User3') self.user.is_superuser = True + self.user2.is_staff = True self.view_permission = views.UserIdentificationInformationListView self.request = RequestFactory().get('/fake_path') self.request.user = self.user @@ -46,9 +119,11 @@ def setUp(self): UserSettingsFactory = S3UserSettingsFactory() UserSettingsFactory.save() - institution = InstitutionFactory() - institution.name = 'test institution' - self.user.affiliated_institutions.add(institution) + self.institution = InstitutionFactory() + self.institution.name = 'test institution' + self.user.affiliated_institutions.add(self.institution) + self.user2.affiliated_institutions.add(self.institution) + self.user3.affiliated_institutions.add(self.institution) self.user.save() self.user2.save() @@ -71,7 +146,7 @@ def test_get_user_quota_info(self): def test_get_queryset(self): view = views.UserIdentificationListView() - view = setup_view(view, self.request) + view = setup_view(view, self.request, institution_id=self.institution.id) results = view.get_user_list() nt.assert_is_instance(results, list) @@ -97,6 +172,38 @@ def test_get_context_data(self, mock_method): view.get_context_data() assert mock_method.called + @mock.patch('admin.user_identification_information.views.UserIdentificationInformationListView.get_queryset') + def test_get_context_data_admin(self, mock_get_queryset): + """Test get_context_data with institution administrator""" + mock_get_queryset.return_value = [ + {'id': 'z298m', 'fullname': 'Test User1', 'eppn': '', 'affiliation': '', 'email': 'freddiemercury+12331754@cos.io', + 'last_login': '', 'usage': 0, 'usage_value': 0.0, 'usage_abbr': 'KB', 'extended_storage': ''}, + {'id': 'n2dch', 'fullname': 'Broken Matt Hardy', 'eppn': '', 'affiliation': '', + 'email': 'freddiemercury+12422709@cos.io', 'last_login': '', 'usage': 0, 'usage_value': 0.0, 'usage_abbr': 'KB', + 'extended_storage': '/Github name\n/Amazon S3'}] + self.request.user = self.user2 + view = views.UserIdentificationInformationListView() + view.paginate_by = 25 + view.object_list = [] + view = setup_view(view, self.request, institution_id=self.institution.id) + results = view.get_context_data() + nt.assert_is_instance(results, dict) + nt.assert_in('users', results) + nt.assert_in('page', results) + nt.assert_equal(len(results['users']), 2) + + @mock.patch('admin.user_identification_information.views.UserIdentificationInformationListView.get_queryset') + def test_get_context_data__institution_not_exist(self, mock_get_queryset): + """Test get_context_data in case institution not exist""" + mock_get_queryset.return_value = [ + {'id': 'dsyem', 'fullname': 'superuser01', 'eppn': '', 'email': 'superuser01@example.com.vn', 'usage': 1352, + 'usage_value': 1.352, 'usage_abbr': 'KB', 'extended_storage': '/Amazon S3'}, ] + view = views.UserIdentificationInformationListView() + view.paginate_by = 25 + view = setup_view(view, self.request, institution_id=0) + with nt.assert_raises(Http404): + view.get_context_data() + class TestUserIdentificationInformationListSorted(AdminTestCase): @@ -118,12 +225,13 @@ def setUp(self): def add_user(self): user = AuthUserFactory() + user.affiliated_institutions.add(self.institution) user.save() return user def view_get(self, url_params): request = RequestFactory().get('/fake_path?{}'.format(url_params)) - view = setup_user_view(views.UserIdentificationListView(), request, user=self.users[0]) + view = setup_user_view(views.UserIdentificationListView(), request, user=self.users[0], institution_id=self.institution.id) return view.get(request) def test_get_order_by_fullname_asc(self): @@ -132,7 +240,7 @@ def test_get_order_by_fullname_asc(self): list_map = list(map(itemgetter('fullname'), response.context_data['users'])) result = [] - for i in range(len(list_map) - 1): + for i in range(len(list_map)): result.append(list_map[i]) nt.assert_equal(result, expected) @@ -154,12 +262,12 @@ def test_get_order_without_status(self): class TestUserIdentificationListView(AdminTestCase): def setUp(self): - institution = InstitutionFactory() + self.institution = InstitutionFactory() self.user = AuthUserFactory(fullname='Test User1') self.user.is_superuser = False self.user.is_staff = False - self.user.affiliated_institutions.add(institution) + self.user.affiliated_institutions.add(self.institution) self.user.save() self.superuser = AuthUserFactory(fullname='Broken Matt Hardy') @@ -168,7 +276,7 @@ def setUp(self): self.superuser.save() self.admin_user = AuthUserFactory(fullname='Test User3') - self.admin_user.affiliated_institutions.add(institution) + self.admin_user.affiliated_institutions.add(self.institution) self.admin_user.is_staff = True self.admin_user.is_superuser = False self.admin_user.save() @@ -208,7 +316,7 @@ def test_get_userlist(self): def test_get_userlist_user_is_superuser(self): self.request.user = self.superuser view = views.UserIdentificationListView() - view = setup_view(view, self.request) + view = setup_view(view, self.request, institution_id=self.institution.id) results = view.get_user_list() nt.assert_is_instance(results, list) @@ -221,56 +329,58 @@ def test_get_userlist_permission_denied(self): view.get_user_list() def test_get_userlist_search_guid(self): - request = RequestFactory().get(reverse('user_identification_information:user_identification_list'), - {'guid': self.superuser._id}) + request = RequestFactory().get(reverse('user_identification_information:user_identification_list', kwargs={'institution_id': self.institution.id}), + {'guid': self.admin_user._id}) request.user = self.superuser view = views.UserIdentificationListView() - view = setup_view(view, request) + view = setup_view(view, request, institution_id=self.institution.id) res = view.get_user_list() - nt.assert_equal(res[0]['id'], self.superuser._id) nt.assert_equal(len(res), 1) + nt.assert_equal(res[0]['id'], self.admin_user._id) def test_get_userlist_search_user_name(self): - request = RequestFactory().get(reverse('user_identification_information:user_identification_list'), - {'username': self.superuser.username}) + request = RequestFactory().get(reverse('user_identification_information:user_identification_list', kwargs={'institution_id': self.institution.id}), + {'username': self.admin_user.username}) request.user = self.superuser view = views.UserIdentificationListView() - view = setup_view(view, request) + view = setup_view(view, request, institution_id=self.institution.id) res = view.get_user_list() - nt.assert_equal(res[0]['email'], self.superuser.username) nt.assert_equal(len(res), 1) + nt.assert_equal(res[0]['email'], self.admin_user.username) def test_get_userlist_search_name(self): - request = RequestFactory().get(reverse('user_identification_information:user_identification_list'), - {'fullname': 'Broken'}) + request = RequestFactory().get(reverse('user_identification_information:user_identification_list', kwargs={'institution_id': self.institution.id}), + {'fullname': 'User3'}) request.user = self.superuser view = views.UserIdentificationListView() - view = setup_view(view, request) + view = setup_view(view, request, institution_id=self.institution.id) res = view.get_user_list() - nt.assert_equal(res[0]['fullname'], self.superuser.fullname) nt.assert_equal(len(res), 1) + nt.assert_equal(res[0]['fullname'], self.admin_user.fullname) def test_get_userlist_search_empty(self): - request = RequestFactory().get(reverse('user_identification_information:user_identification_list'), {'fullname': ''}) + request = RequestFactory().get(reverse('user_identification_information:user_identification_list', kwargs={'institution_id': self.institution.id}), + {'fullname': ''}) request.user = self.superuser view = views.UserIdentificationListView() - view = setup_view(view, request) + view = setup_view(view, request, institution_id=self.institution.id) res = view.get_user_list() - nt.assert_equal(len(res), 5) + nt.assert_equal(len(res), 2) def test_get_userlist_search_none_in_list(self): - request = RequestFactory().get(reverse('user_identification_information:user_identification_list'), + request = RequestFactory().get(reverse('user_identification_information:user_identification_list', kwargs={'institution_id': self.institution.id}), {'fullname': 'admin'}) request.user = self.superuser view = views.UserIdentificationListView() - view = setup_view(view, request) + view = setup_view(view, request, institution_id=self.institution.id) res = view.get_user_list() nt.assert_equal(len(res), 0) def test__permission_anonymous(self): self.request.user = self.anon - with nt.assert_raises(PermissionDenied): - views.UserIdentificationListView.as_view()(self.request) + response = views.UserIdentificationListView.as_view()(self.request) + nt.assert_equal(response.status_code, 302) + nt.assert_in('login', str(response)) def test__permission_normal_user(self): self.request.user = self.user @@ -279,7 +389,7 @@ def test__permission_normal_user(self): def test__permission_super_user(self): self.request.user = self.superuser - res = views.UserIdentificationListView.as_view()(self.request) + res = views.UserIdentificationListView.as_view()(self.request, institution_id=self.institution.id) nt.assert_equal(res.status_code, 200) def test__permission_admin(self): @@ -287,6 +397,16 @@ def test__permission_admin(self): with nt.assert_raises(PermissionDenied): views.UserIdentificationListView.as_view()(self.request) + def test_get_userlist__institution_not_exist(self): + """Test get_userlist in case institution not exist""" + request = RequestFactory().get(reverse('user_identification_information:user_identification_list', kwargs={'institution_id': 0})) + request.user = self.superuser + view = views.UserIdentificationListView() + view = setup_view(view, request) + with nt.assert_raises(Http404): + view.get_user_list() + + class TestUserIdentificationDetailView(AdminTestCase): def setUp(self): @@ -345,14 +465,15 @@ def test__permission_normal_user(self): def test__permission_super_user(self): self.request.user = self.superuser - res = views.UserIdentificationListView.as_view()(self.request, guid=self.superuser._id) + res = views.UserIdentificationDetailView.as_view()(self.request, guid=self.superuser._id) nt.assert_equal(res.status_code, 200) def test__permission_admin(self): self.admin_user.affiliated_institutions.add(InstitutionFactory()) self.request.user = self.admin_user with nt.assert_raises(PermissionDenied): - views.UserIdentificationListView.as_view()(self.request, guid=self.admin_user._id) + views.UserIdentificationDetailView.as_view()(self.request, guid=self.admin_user._id) + class TestExportFileCSVView(AdminTestCase): def setUp(self): @@ -399,15 +520,16 @@ def test_get(self): def test_get_is_super_admin(self): request = RequestFactory().get('/fake_path') request.user = self.superuser - view = setup_view(self.view, request) + view = setup_view(self.view, request, institution_id=self.institution.id) res = view.get(request) nt.assert_equal(res.status_code, 200) nt.assert_equal(res['content-type'], 'text/csv') def test__permission_anonymous(self): self.request.user = self.anon - with nt.assert_raises(PermissionDenied): - views.ExportFileCSVView.as_view()(self.request) + response = views.ExportFileCSVView.as_view()(self.request) + nt.assert_equal(response.status_code, 302) + nt.assert_in('login', str(response)) def test__permission_normal_user(self): self.request.user = self.user @@ -416,7 +538,7 @@ def test__permission_normal_user(self): def test__permission_super_user(self): self.request.user = self.superuser - res = views.ExportFileCSVView.as_view()(self.request) + res = views.ExportFileCSVView.as_view()(self.request, institution_id=self.institution.id) nt.assert_equal(res.status_code, 200) def test__permission_admin(self): @@ -424,3 +546,33 @@ def test__permission_admin(self): self.request.user = self.admin_user with nt.assert_raises(PermissionDenied): views.ExportFileCSVView.as_view()(self.request) + + def test_get__institution_not_exist(self): + """Test get method in case institution not exist""" + request = RequestFactory().get('/fake_path') + request.user = self.superuser + view = setup_view(self.view, request, institution_id=0) + with nt.assert_raises(Http404): + view.get(request) + + @mock.patch('admin.user_identification_information.views.get_list_extend_storage') + def test_get__multiple_extend_storage(self, mock_get_list_extend_storage): + """Test get method with user having multiple extend storage""" + # Mock storage data for multiple users + storage_data = { + self.user.id: ['100MB storage1'], + self.admin_user.id: ['200MB storage2', '300MB storage3'] + } + mock_get_list_extend_storage.return_value = storage_data + + request = RequestFactory().get('/fake_path') + request.user = self.superuser + view = setup_view(self.view, request, institution_id=self.institution.id) + response = view.get(request) + + # Verify response + nt.assert_equal(response.status_code, 200) + nt.assert_equal(response['content-type'], 'text/csv') + content = response.content.decode('utf-8') + nt.assert_in('100MB storage1', content) + nt.assert_in('200MB storage2\n300MB storage3', content)