diff --git a/.prospector.yaml b/.prospector.yaml index 1e654102fe..ef872b4a17 100644 --- a/.prospector.yaml +++ b/.prospector.yaml @@ -12,6 +12,7 @@ pylint: options: extension-pkg-allow-list: - ujson + - lxml.etree mccabe: run: false diff --git a/.pylintrc b/.pylintrc index 686428e7b6..f8fabd8338 100644 --- a/.pylintrc +++ b/.pylintrc @@ -3,7 +3,7 @@ # A comma-separated list of package or module names from where C extensions may # be loaded. Extensions are loading into the active Python interpreter and may # run arbitrary code -extension-pkg-allow-list=ujson +extension-pkg-allow-list=ujson,lxml.etree # Add files or directories to the blacklist. They should be base names, not # paths. diff --git a/onadata/apps/api/tools.py b/onadata/apps/api/tools.py index 7d27ad3938..336e66f606 100644 --- a/onadata/apps/api/tools.py +++ b/onadata/apps/api/tools.py @@ -55,8 +55,10 @@ get_role_in_org, is_organization, ) +from onadata.libs.serializers.project_serializer import ProjectSerializer from onadata.libs.utils.api_export_tools import ( - custom_response_handler, get_metadata_format + custom_response_handler, + get_metadata_format, ) from onadata.libs.utils.cache_tools import ( PROJ_BASE_FORMS_CACHE, @@ -491,7 +493,7 @@ def id_string_exists_in_account(): # Ensure the cached project is the updated version. # Django lazy loads related objects as such we need to # ensure the project retrieved is up to date. - reset_project_cache(xform.project, request) + reset_project_cache(xform.project, request, ProjectSerializer) return xform diff --git a/onadata/libs/authentication.py b/onadata/libs/authentication.py index 739a6cffae..7be1d4831e 100644 --- a/onadata/libs/authentication.py +++ b/onadata/libs/authentication.py @@ -21,6 +21,7 @@ from oauth2_provider.models import AccessToken from oauth2_provider.oauth2_validators import OAuth2Validator from oauth2_provider.settings import oauth2_settings +from oidc.utils import authenticate_sso from rest_framework import exceptions from rest_framework.authentication import ( BaseAuthentication, @@ -29,16 +30,10 @@ ) from rest_framework.authtoken.models import Token from rest_framework.exceptions import AuthenticationFailed -from oidc.utils import authenticate_sso from onadata.apps.api.models.temp_token import TempToken from onadata.apps.api.tasks import send_account_lockout_email -from onadata.libs.utils.cache_tools import ( - LOCKOUT_IP, - LOGIN_ATTEMPTS, - cache, - safe_key, -) +from onadata.libs.utils.cache_tools import LOCKOUT_IP, LOGIN_ATTEMPTS, cache, safe_key from onadata.libs.utils.common_tags import API_TOKEN from onadata.libs.utils.email import get_account_lockout_email_data @@ -152,7 +147,7 @@ def authenticate(self, request): raise exceptions.AuthenticationFailed(error_msg) if len(auth) > 2: error_msg = _( - "Invalid token header. " "Token string should not contain spaces." + "Invalid token header. Token string should not contain spaces." ) raise exceptions.AuthenticationFailed(error_msg) @@ -240,7 +235,7 @@ class SSOHeaderAuthentication(BaseAuthentication): cookie or HTTP_SSO header. """ - def authenticate(self, request): # pylint: disable=no-self-use + def authenticate(self, request): return authenticate_sso(request) @@ -367,9 +362,19 @@ class MasterReplicaOAuth2Validator(OAuth2Validator): """ Custom OAuth2Validator class that takes into account replication lag between Master & Replica databases - https://github.com/jazzband/django-oauth-toolkit/blob/3bde632d5722f1f85ffcd8277504955321f00fff/oauth2_provider/oauth2_validators.py#L49 + https://github.com/jazzband/django-oauth-toolkit/blob/ + 3bde632d5722f1f85ffcd8277504955321f00fff/oauth2_provider/oauth2_validators.py#L49 """ + def introspect_token(self, token, token_type_hint, request, *args, **kwargs): + pass + + def validate_silent_authorization(self, request): + pass + + def validate_silent_login(self, request): + pass + def validate_bearer_token(self, token, scopes, request): if not token: return False diff --git a/onadata/libs/baseviewset.py b/onadata/libs/baseviewset.py index 7748951883..9329b85651 100644 --- a/onadata/libs/baseviewset.py +++ b/onadata/libs/baseviewset.py @@ -1,2 +1,10 @@ -class DefaultBaseViewset(object): - pass +# -*- coding: utf-8 -*- +""" +The DefaultBaseViewset class +""" + + +class DefaultBaseViewset: + """ + The DefaultBaseViewset class + """ diff --git a/onadata/libs/filters.py b/onadata/libs/filters.py index 78ad7d7c1f..3704c28f8e 100644 --- a/onadata/libs/filters.py +++ b/onadata/libs/filters.py @@ -4,14 +4,14 @@ """ from uuid import UUID -import six - from django.contrib.auth import get_user_model from django.contrib.contenttypes.models import ContentType from django.core.exceptions import ObjectDoesNotExist from django.db.models import Q from django.http import Http404 from django.shortcuts import get_object_or_404 + +import six from django_filters import rest_framework as django_filter_filters from rest_framework import filters from rest_framework_guardian.filters import ObjectPermissionsFilter @@ -19,10 +19,9 @@ from onadata.apps.api.models import OrganizationProfile, Team from onadata.apps.logger.models import Instance, Project, XForm from onadata.apps.viewer.models import Export -from onadata.libs.utils.numeric import int_or_parse_error -from onadata.libs.utils.common_tags import MEDIA_FILE_TYPES from onadata.libs.permissions import exclude_items_from_queryset_using_xform_meta_perms - +from onadata.libs.utils.common_tags import MEDIA_FILE_TYPES +from onadata.libs.utils.numeric import int_or_parse_error # pylint: disable=invalid-name User = get_user_model() @@ -55,7 +54,7 @@ def filter_queryset(self, request, queryset, view): if form_id: if lookup_field == "pk": int_or_parse_error( - form_id, "Invalid form ID. It must be a positive" " integer" + form_id, "Invalid form ID. It must be a positive integer" ) try: @@ -104,7 +103,6 @@ class XFormListObjectPermissionFilter(AnonDjangoObjectPermissionFilter): class XFormListXFormPKFilter: """Filter forms via 'xform_pk' param.""" - # pylint: disable=no-self-use def filter_queryset(self, request, queryset, view): """Returns an XForm queryset filtered by the 1xform_pk' param.""" xform_pk = view.kwargs.get("xform_pk") @@ -393,7 +391,7 @@ def _instance_filter(self, request, view, keyword): for object_id in [instance_id, project_id]: int_or_parse_error( object_id, - "Invalid value for instanceid. It must be" " a positive integer.", + "Invalid value for instanceid. It must be a positive integer.", ) instance = get_object_or_404(Instance, pk=instance_id) @@ -506,7 +504,7 @@ def filter_queryset(self, request, queryset, view): if instance_id: int_or_parse_error( instance_id, - "Invalid value for instance_id. It must be" " a positive integer.", + "Invalid value for instance_id. It must be a positive integer.", ) instance = get_object_or_404(Instance, pk=instance_id) queryset = queryset.filter(instance=instance) @@ -615,8 +613,8 @@ class UserProfileFilter(filters.BaseFilterBackend): """Filter by the ``users`` query parameter.""" def filter_queryset(self, request, queryset, view): - """Filter by the ``users`` query parameter - returns a queryset of only the users - in the users parameter when `view.action == "list"`""" + """Filter by the ``users`` query parameter - returns a queryset of only the + users in the users parameter when `view.action == "list"`""" if view.action == "list": users = request.GET.get("users") if users: @@ -641,7 +639,7 @@ def filter_queryset(self, request, queryset, view): if instance_id: int_or_parse_error( instance_id, - "Invalid value for instance_id. It must be" " a positive integer", + "Invalid value for instance_id. It must be a positive integer", ) instance = get_object_or_404(Instance, pk=instance_id) @@ -692,7 +690,7 @@ def filter_queryset(self, request, queryset, view): class PublicDatasetsFilter: """Public data set filter where the share attribute is True""" - # pylint: disable=unused-argument,no-self-use + # pylint: disable=unused-argument def filter_queryset(self, request, queryset, view): """Return a queryset of shared=True data if the user is anonymous.""" if request and request.user.is_anonymous: diff --git a/onadata/libs/pagination.py b/onadata/libs/pagination.py index 06888f4e62..ed82a780b7 100644 --- a/onadata/libs/pagination.py +++ b/onadata/libs/pagination.py @@ -1,38 +1,57 @@ -from django.core.paginator import Paginator +# -*- coding=utf-8 -*- +""" +Pagination classes. +""" from django.conf import settings +from django.core.paginator import Paginator from django.db.models import QuerySet + from rest_framework.pagination import ( - PageNumberPagination, InvalidPage, NotFound, replace_query_param) + InvalidPage, + NotFound, + PageNumberPagination, + replace_query_param, +) from rest_framework.request import Request class StandardPageNumberPagination(PageNumberPagination): + """The Standard PageNumberPagination class + + Set's the default ``page_size`` to 1000 with a maximum page_size of 10,000 records + per page. + """ + page_size = 1000 - page_size_query_param = 'page_size' - max_page_size = getattr( - settings, "STANDARD_PAGINATION_MAX_PAGE_SIZE", 10000) + page_size_query_param = "page_size" + max_page_size = getattr(settings, "STANDARD_PAGINATION_MAX_PAGE_SIZE", 10000) def get_first_page_link(self): + """Returns the URL to the first page.""" if self.page.number == 1: return None + url = self.request.build_absolute_uri() + return replace_query_param(url, self.page_query_param, 1) def get_last_page_link(self): + """Returns the URL to the last page.""" if self.page.number == self.paginator.num_pages: return None + url = self.request.build_absolute_uri() - return replace_query_param( - url, self.page_query_param, self.paginator.num_pages) - def generate_link_header( - self, request: Request, queryset: QuerySet - ): + return replace_query_param(url, self.page_query_param, self.paginator.num_pages) + + def generate_link_header(self, request: Request, queryset: QuerySet): + """Generates pagination headers for a HTTP response object""" links = [] page_size = self.get_page_size(request) if not page_size: return {} page_number = request.query_params.get(self.page_query_param, 1) + # pylint: disable=attribute-defined-outside-init self.paginator = self.django_paginator_class(queryset, page_size) self.request = request @@ -42,45 +61,62 @@ def generate_link_header( return {} for rel, link in ( - ('prev', self.get_previous_link()), - ('next', self.get_next_link()), - ('last', self.get_last_page_link()), - ('first', self.get_first_page_link())): + ("prev", self.get_previous_link()), + ("next", self.get_next_link()), + ("last", self.get_last_page_link()), + ("first", self.get_first_page_link()), + ): if link: links.append(f'<{link}>; rel="{rel}"') - return {'Link': ', '.join(links)} + return {"Link": ", ".join(links)} class CountOverridablePaginator(Paginator): + """Count override Paginator + + Allows overriding the count especially in the event it may be expensive request. + """ + + # pylint: disable=too-many-arguments def __init__( - self, object_list, per_page, - orphans: int = 0, allow_empty_first_page: bool = True, - count_override: int = None) -> None: + self, + object_list, + per_page, + orphans: int = 0, + allow_empty_first_page: bool = True, + count_override: int = None, + ) -> None: self.count_override = count_override super().__init__( - object_list, per_page, - orphans=orphans, allow_empty_first_page=allow_empty_first_page) + object_list, + per_page, + orphans=orphans, + allow_empty_first_page=allow_empty_first_page, + ) - @property def count(self): if self.count_override: return self.count_override - return super().count + return super().count() class CountOverridablePageNumberPagination(StandardPageNumberPagination): + """Count override PageNumberPagination + + Allows overriding the count especially in the event it may be expensive request. + """ + django_paginator_class = CountOverridablePaginator def paginate_queryset(self, queryset, request, view, count=None): + # pylint: disable=attribute-defined-outside-init page_size = self.get_page_size(request) if not page_size: return None paginator = self.django_paginator_class( - queryset, - page_size, - count_override=count + queryset, page_size, count_override=count ) page_number = request.query_params.get(self.page_query_param, 1) if page_number in self.last_page_strings: @@ -89,9 +125,10 @@ def paginate_queryset(self, queryset, request, view, count=None): try: self.page = paginator.page(page_number) except InvalidPage as exc: - msg = self.invalid_page_message.format(page_number=page_number, - message=str(exc)) - raise NotFound(msg) + msg = self.invalid_page_message.format( + page_number=page_number, message=str(exc) + ) + raise NotFound(msg) from exc if paginator.num_pages > 1 and self.template is not None: self.display_page_controls = True diff --git a/onadata/libs/permissions.py b/onadata/libs/permissions.py index 91b8b8ced3..7c2182d310 100644 --- a/onadata/libs/permissions.py +++ b/onadata/libs/permissions.py @@ -5,19 +5,22 @@ import json from collections import defaultdict -import six -from django.db.models.base import ModelBase from django.db.models import Q -from guardian.shortcuts import (assign_perm, get_perms, get_users_with_perms, - remove_perm) +from django.db.models.base import ModelBase + +import six +from guardian.shortcuts import assign_perm, get_perms, get_users_with_perms, remove_perm from onadata.apps.api.models import OrganizationProfile -from onadata.apps.logger.models import MergedXForm, Project, XForm,\ - Attachment -from onadata.apps.logger.models.project import (ProjectGroupObjectPermission, - ProjectUserObjectPermission) -from onadata.apps.logger.models.xform import (XFormGroupObjectPermission, - XFormUserObjectPermission) +from onadata.apps.logger.models import Attachment, MergedXForm, Project, XForm +from onadata.apps.logger.models.project import ( + ProjectGroupObjectPermission, + ProjectUserObjectPermission, +) +from onadata.apps.logger.models.xform import ( + XFormGroupObjectPermission, + XFormUserObjectPermission, +) from onadata.apps.main.models.user_profile import UserProfile from onadata.apps.viewer.models import DataDictionary from onadata.libs.exceptions import NoRecordsPermission @@ -25,60 +28,61 @@ from onadata.libs.utils.model_tools import queryset_iterator # Userprofile Permissions -CAN_ADD_USERPROFILE = 'add_userprofile' -CAN_CHANGE_USERPROFILE = 'change_userprofile' -CAN_DELETE_USERPROFILE = 'delete_userprofile' -CAN_ADD_PROJECT_TO_PROFILE = 'can_add_project' -CAN_ADD_XFORM_TO_PROFILE = 'can_add_xform' -CAN_VIEW_PROFILE = 'view_profile' +CAN_ADD_USERPROFILE = "add_userprofile" +CAN_CHANGE_USERPROFILE = "change_userprofile" +CAN_DELETE_USERPROFILE = "delete_userprofile" +CAN_ADD_PROJECT_TO_PROFILE = "can_add_project" +CAN_ADD_XFORM_TO_PROFILE = "can_add_xform" +CAN_VIEW_PROFILE = "view_profile" # Organization Permissions -CAN_VIEW_ORGANIZATION_PROFILE = 'view_organizationprofile' -CAN_ADD_ORGANIZATION_PROFILE = 'add_organizationprofile' -CAN_ADD_ORGANIZATION_PROJECT = 'can_add_project' -CAN_ADD_ORGANIZATION_XFORM = 'can_add_xform' -CAN_CHANGE_ORGANIZATION_PROFILE = 'change_organizationprofile' -CAN_DELETE_ORGANIZATION_PROFILE = 'delete_organizationprofile' -IS_ORGANIZATION_OWNER = 'is_org_owner' +CAN_VIEW_ORGANIZATION_PROFILE = "view_organizationprofile" +CAN_ADD_ORGANIZATION_PROFILE = "add_organizationprofile" +CAN_ADD_ORGANIZATION_PROJECT = "can_add_project" +CAN_ADD_ORGANIZATION_XFORM = "can_add_xform" +CAN_CHANGE_ORGANIZATION_PROFILE = "change_organizationprofile" +CAN_DELETE_ORGANIZATION_PROFILE = "delete_organizationprofile" +IS_ORGANIZATION_OWNER = "is_org_owner" # Xform Permissions -CAN_CHANGE_XFORM = 'change_xform' -CAN_ADD_XFORM = 'add_xform' -CAN_DELETE_XFORM = 'delete_xform' -CAN_VIEW_XFORM = 'view_xform' -CAN_VIEW_XFORM_DATA = 'view_xform_data' -CAN_VIEW_XFORM_ALL = 'view_xform_all' -CAN_ADD_SUBMISSIONS = 'report_xform' -CAN_DELETE_SUBMISSION = 'delete_submission' -CAN_TRANSFER_OWNERSHIP = 'transfer_xform' -CAN_MOVE_TO_FOLDER = 'move_xform' -CAN_EXPORT_XFORM = 'can_export_xform_data' +CAN_CHANGE_XFORM = "change_xform" +CAN_ADD_XFORM = "add_xform" +CAN_DELETE_XFORM = "delete_xform" +CAN_VIEW_XFORM = "view_xform" +CAN_VIEW_XFORM_DATA = "view_xform_data" +CAN_VIEW_XFORM_ALL = "view_xform_all" +CAN_ADD_SUBMISSIONS = "report_xform" +CAN_DELETE_SUBMISSION = "delete_submission" +CAN_TRANSFER_OWNERSHIP = "transfer_xform" +CAN_MOVE_TO_FOLDER = "move_xform" +CAN_EXPORT_XFORM = "can_export_xform_data" # MergedXform Permissions -CAN_VIEW_MERGED_XFORM = 'view_mergedxform' +CAN_VIEW_MERGED_XFORM = "view_mergedxform" # Project Permissions -CAN_ADD_PROJECT = 'add_project' -CAN_VIEW_PROJECT = 'view_project' -CAN_VIEW_PROJECT_DATA = 'view_project_data' -CAN_VIEW_PROJECT_ALL = 'view_project_all' -CAN_CHANGE_PROJECT = 'change_project' -CAN_TRANSFER_PROJECT_OWNERSHIP = 'transfer_project' -CAN_DELETE_PROJECT = 'delete_project' -CAN_ADD_PROJECT_XFORM = 'add_project_xform' -CAN_ADD_SUBMISSIONS_PROJECT = 'report_project_xform' -CAN_EXPORT_PROJECT = 'can_export_project_data' +CAN_ADD_PROJECT = "add_project" +CAN_VIEW_PROJECT = "view_project" +CAN_VIEW_PROJECT_DATA = "view_project_data" +CAN_VIEW_PROJECT_ALL = "view_project_all" +CAN_CHANGE_PROJECT = "change_project" +CAN_TRANSFER_PROJECT_OWNERSHIP = "transfer_project" +CAN_DELETE_PROJECT = "delete_project" +CAN_ADD_PROJECT_XFORM = "add_project_xform" +CAN_ADD_SUBMISSIONS_PROJECT = "report_project_xform" +CAN_EXPORT_PROJECT = "can_export_project_data" # Data dictionary permissions -CAN_ADD_DATADICTIONARY = 'add_datadictionary' -CAN_CHANGE_DATADICTIONARY = 'change_datadictionary' -CAN_DELETE_DATADICTIONARY = 'delete_datadictionary' +CAN_ADD_DATADICTIONARY = "add_datadictionary" +CAN_CHANGE_DATADICTIONARY = "change_datadictionary" +CAN_DELETE_DATADICTIONARY = "delete_datadictionary" -class Role(object): +class Role: """ Base Role class. """ + class_to_permissions = defaultdict(list) name = None @@ -135,12 +139,16 @@ class ReadOnlyRoleNoDownload(Role): """ Read-only no download Role class. """ - name = 'readonly-no-download' - permissions = ((CAN_VIEW_ORGANIZATION_PROFILE, - OrganizationProfile), (CAN_VIEW_XFORM, XForm), - (CAN_VIEW_PROJECT, Project), (CAN_VIEW_XFORM_ALL, XForm), - (CAN_VIEW_PROJECT_ALL, Project), (CAN_VIEW_MERGED_XFORM, - MergedXForm), ) + + name = "readonly-no-download" + permissions = ( + (CAN_VIEW_ORGANIZATION_PROFILE, OrganizationProfile), + (CAN_VIEW_XFORM, XForm), + (CAN_VIEW_PROJECT, Project), + (CAN_VIEW_XFORM_ALL, XForm), + (CAN_VIEW_PROJECT_ALL, Project), + (CAN_VIEW_MERGED_XFORM, MergedXForm), + ) class_to_permissions = { MergedXForm: [CAN_VIEW_MERGED_XFORM], @@ -153,7 +161,8 @@ class ReadOnlyRole(Role): """ Read-only Role class. """ - name = 'readonly' + + name = "readonly" class_to_permissions = { MergedXForm: [CAN_VIEW_MERGED_XFORM], @@ -167,13 +176,13 @@ class DataEntryOnlyRole(Role): """ Data-Entry only Role class. """ - name = 'dataentry-only' + + name = "dataentry-only" class_to_permissions = { MergedXForm: [CAN_VIEW_MERGED_XFORM], OrganizationProfile: [CAN_VIEW_ORGANIZATION_PROFILE], - Project: - [CAN_ADD_SUBMISSIONS_PROJECT, CAN_EXPORT_PROJECT, CAN_VIEW_PROJECT], + Project: [CAN_ADD_SUBMISSIONS_PROJECT, CAN_EXPORT_PROJECT, CAN_VIEW_PROJECT], XForm: [CAN_ADD_SUBMISSIONS], } @@ -183,17 +192,22 @@ class DataEntryMinorRole(Role): Data-Entry minor Role class - user can submit and has readonly access to data they submitted. """ - name = 'dataentry-minor' + + name = "dataentry-minor" class_to_permissions = { MergedXForm: [CAN_VIEW_MERGED_XFORM], OrganizationProfile: [CAN_VIEW_ORGANIZATION_PROFILE], Project: [ - CAN_ADD_SUBMISSIONS_PROJECT, CAN_EXPORT_PROJECT, CAN_VIEW_PROJECT, - CAN_VIEW_PROJECT_DATA + CAN_ADD_SUBMISSIONS_PROJECT, + CAN_EXPORT_PROJECT, + CAN_VIEW_PROJECT, + CAN_VIEW_PROJECT_DATA, ], XForm: [ - CAN_ADD_SUBMISSIONS, CAN_EXPORT_XFORM, CAN_VIEW_XFORM, - CAN_VIEW_XFORM_DATA + CAN_ADD_SUBMISSIONS, + CAN_EXPORT_XFORM, + CAN_VIEW_XFORM, + CAN_VIEW_XFORM_DATA, ], } @@ -203,17 +217,24 @@ class DataEntryRole(Role): Data-Entry Role class - user can submit data and has readonly permissions to all the data including data submitted by others. """ - name = 'dataentry' + + name = "dataentry" class_to_permissions = { MergedXForm: [CAN_VIEW_MERGED_XFORM], OrganizationProfile: [CAN_VIEW_ORGANIZATION_PROFILE], Project: [ - CAN_ADD_SUBMISSIONS_PROJECT, CAN_EXPORT_PROJECT, CAN_VIEW_PROJECT, - CAN_VIEW_PROJECT_ALL, CAN_VIEW_PROJECT_DATA + CAN_ADD_SUBMISSIONS_PROJECT, + CAN_EXPORT_PROJECT, + CAN_VIEW_PROJECT, + CAN_VIEW_PROJECT_ALL, + CAN_VIEW_PROJECT_DATA, ], XForm: [ - CAN_ADD_SUBMISSIONS, CAN_EXPORT_XFORM, CAN_VIEW_XFORM, - CAN_VIEW_XFORM_ALL, CAN_VIEW_XFORM_DATA + CAN_ADD_SUBMISSIONS, + CAN_EXPORT_XFORM, + CAN_VIEW_XFORM, + CAN_VIEW_XFORM_ALL, + CAN_VIEW_XFORM_DATA, ], } @@ -223,17 +244,25 @@ class EditorMinorRole(Role): Editor-Minor Role class - user can submit data, read and edit only the data they submitted. """ - name = 'editor-minor' + + name = "editor-minor" class_to_permissions = { MergedXForm: [CAN_VIEW_MERGED_XFORM], OrganizationProfile: [CAN_VIEW_ORGANIZATION_PROFILE], Project: [ - CAN_ADD_SUBMISSIONS_PROJECT, CAN_CHANGE_PROJECT, - CAN_EXPORT_PROJECT, CAN_VIEW_PROJECT, CAN_VIEW_PROJECT_DATA + CAN_ADD_SUBMISSIONS_PROJECT, + CAN_CHANGE_PROJECT, + CAN_EXPORT_PROJECT, + CAN_VIEW_PROJECT, + CAN_VIEW_PROJECT_DATA, ], XForm: [ - CAN_ADD_SUBMISSIONS, CAN_CHANGE_XFORM, CAN_DELETE_SUBMISSION, - CAN_EXPORT_XFORM, CAN_VIEW_XFORM, CAN_VIEW_XFORM_DATA + CAN_ADD_SUBMISSIONS, + CAN_CHANGE_XFORM, + CAN_DELETE_SUBMISSION, + CAN_EXPORT_XFORM, + CAN_VIEW_XFORM, + CAN_VIEW_XFORM_DATA, ], } @@ -242,19 +271,27 @@ class EditorRole(Role): """ Editor Role class - user can submit, read and edit any submitted data. """ - name = 'editor' + + name = "editor" class_to_permissions = { MergedXForm: [CAN_VIEW_MERGED_XFORM], OrganizationProfile: [CAN_VIEW_ORGANIZATION_PROFILE], Project: [ - CAN_ADD_SUBMISSIONS_PROJECT, CAN_CHANGE_PROJECT, - CAN_EXPORT_PROJECT, CAN_VIEW_PROJECT, CAN_VIEW_PROJECT_ALL, - CAN_VIEW_PROJECT_DATA + CAN_ADD_SUBMISSIONS_PROJECT, + CAN_CHANGE_PROJECT, + CAN_EXPORT_PROJECT, + CAN_VIEW_PROJECT, + CAN_VIEW_PROJECT_ALL, + CAN_VIEW_PROJECT_DATA, ], XForm: [ - CAN_ADD_SUBMISSIONS, CAN_CHANGE_XFORM, CAN_DELETE_SUBMISSION, - CAN_EXPORT_XFORM, CAN_VIEW_XFORM, CAN_VIEW_XFORM_ALL, - CAN_VIEW_XFORM_DATA + CAN_ADD_SUBMISSIONS, + CAN_CHANGE_XFORM, + CAN_DELETE_SUBMISSION, + CAN_EXPORT_XFORM, + CAN_VIEW_XFORM, + CAN_VIEW_XFORM_ALL, + CAN_VIEW_XFORM_DATA, ], } @@ -264,24 +301,40 @@ class ManagerRole(Role): Manager Role class - user can add,delete,edit forms and data as well as control access to data, forms and projects. """ - name = 'manager' + + name = "manager" class_to_permissions = { MergedXForm: [CAN_VIEW_MERGED_XFORM], - OrganizationProfile: - [CAN_ADD_ORGANIZATION_PROJECT, CAN_ADD_ORGANIZATION_XFORM, - CAN_VIEW_ORGANIZATION_PROFILE], + OrganizationProfile: [ + CAN_ADD_ORGANIZATION_PROJECT, + CAN_ADD_ORGANIZATION_XFORM, + CAN_VIEW_ORGANIZATION_PROFILE, + ], Project: [ - CAN_ADD_PROJECT, CAN_ADD_PROJECT_XFORM, - CAN_ADD_SUBMISSIONS_PROJECT, CAN_CHANGE_PROJECT, - CAN_EXPORT_PROJECT, CAN_VIEW_PROJECT, CAN_VIEW_PROJECT_ALL, - CAN_VIEW_PROJECT_DATA + CAN_ADD_PROJECT, + CAN_ADD_PROJECT_XFORM, + CAN_ADD_SUBMISSIONS_PROJECT, + CAN_CHANGE_PROJECT, + CAN_EXPORT_PROJECT, + CAN_VIEW_PROJECT, + CAN_VIEW_PROJECT_ALL, + CAN_VIEW_PROJECT_DATA, + ], + UserProfile: [ + CAN_ADD_PROJECT_TO_PROFILE, + CAN_ADD_XFORM_TO_PROFILE, + CAN_VIEW_PROFILE, ], - UserProfile: [CAN_ADD_PROJECT_TO_PROFILE, CAN_ADD_XFORM_TO_PROFILE, - CAN_VIEW_PROFILE], XForm: [ - CAN_ADD_SUBMISSIONS, CAN_ADD_XFORM, CAN_CHANGE_XFORM, - CAN_DELETE_SUBMISSION, CAN_DELETE_XFORM, CAN_EXPORT_XFORM, - CAN_VIEW_XFORM, CAN_VIEW_XFORM_ALL, CAN_VIEW_XFORM_DATA + CAN_ADD_SUBMISSIONS, + CAN_ADD_XFORM, + CAN_CHANGE_XFORM, + CAN_DELETE_SUBMISSION, + CAN_DELETE_XFORM, + CAN_EXPORT_XFORM, + CAN_VIEW_XFORM, + CAN_VIEW_XFORM_ALL, + CAN_VIEW_XFORM_DATA, ], } @@ -290,52 +343,80 @@ class MemberRole(Role): """ This is a role for a member of an organization. """ - name = 'member' + + name = "member" class OwnerRole(Role): """ This is a role for an owner of a dataset, organization, or project. """ - name = 'owner' + + name = "owner" class_to_permissions = { DataDictionary: [ - CAN_ADD_DATADICTIONARY, CAN_CHANGE_DATADICTIONARY, - CAN_DELETE_DATADICTIONARY + CAN_ADD_DATADICTIONARY, + CAN_CHANGE_DATADICTIONARY, + CAN_DELETE_DATADICTIONARY, ], MergedXForm: [CAN_VIEW_MERGED_XFORM], OrganizationProfile: [ - CAN_ADD_ORGANIZATION_PROJECT, CAN_ADD_ORGANIZATION_XFORM, - CAN_ADD_ORGANIZATION_PROFILE, CAN_ADD_ORGANIZATION_PROJECT, - CAN_ADD_ORGANIZATION_XFORM, CAN_CHANGE_ORGANIZATION_PROFILE, - CAN_DELETE_ORGANIZATION_PROFILE, CAN_VIEW_ORGANIZATION_PROFILE, - IS_ORGANIZATION_OWNER + CAN_ADD_ORGANIZATION_PROJECT, + CAN_ADD_ORGANIZATION_XFORM, + CAN_ADD_ORGANIZATION_PROFILE, + CAN_ADD_ORGANIZATION_PROJECT, + CAN_ADD_ORGANIZATION_XFORM, + CAN_CHANGE_ORGANIZATION_PROFILE, + CAN_DELETE_ORGANIZATION_PROFILE, + CAN_VIEW_ORGANIZATION_PROFILE, + IS_ORGANIZATION_OWNER, ], Project: [ - CAN_ADD_PROJECT, CAN_ADD_PROJECT_XFORM, - CAN_ADD_SUBMISSIONS_PROJECT, CAN_CHANGE_PROJECT, - CAN_DELETE_PROJECT, CAN_EXPORT_PROJECT, - CAN_TRANSFER_PROJECT_OWNERSHIP, CAN_VIEW_PROJECT, - CAN_VIEW_PROJECT_ALL, CAN_VIEW_PROJECT_DATA + CAN_ADD_PROJECT, + CAN_ADD_PROJECT_XFORM, + CAN_ADD_SUBMISSIONS_PROJECT, + CAN_CHANGE_PROJECT, + CAN_DELETE_PROJECT, + CAN_EXPORT_PROJECT, + CAN_TRANSFER_PROJECT_OWNERSHIP, + CAN_VIEW_PROJECT, + CAN_VIEW_PROJECT_ALL, + CAN_VIEW_PROJECT_DATA, ], UserProfile: [ - CAN_ADD_PROJECT_TO_PROFILE, CAN_ADD_XFORM_TO_PROFILE, - CAN_ADD_USERPROFILE, CAN_CHANGE_USERPROFILE, - CAN_DELETE_USERPROFILE, CAN_VIEW_PROFILE + CAN_ADD_PROJECT_TO_PROFILE, + CAN_ADD_XFORM_TO_PROFILE, + CAN_ADD_USERPROFILE, + CAN_CHANGE_USERPROFILE, + CAN_DELETE_USERPROFILE, + CAN_VIEW_PROFILE, ], XForm: [ - CAN_ADD_SUBMISSIONS, CAN_ADD_XFORM, CAN_CHANGE_XFORM, - CAN_DELETE_SUBMISSION, CAN_DELETE_XFORM, CAN_EXPORT_XFORM, - CAN_VIEW_XFORM, CAN_VIEW_XFORM_ALL, CAN_VIEW_XFORM_DATA, - CAN_MOVE_TO_FOLDER, CAN_TRANSFER_OWNERSHIP + CAN_ADD_SUBMISSIONS, + CAN_ADD_XFORM, + CAN_CHANGE_XFORM, + CAN_DELETE_SUBMISSION, + CAN_DELETE_XFORM, + CAN_EXPORT_XFORM, + CAN_VIEW_XFORM, + CAN_VIEW_XFORM_ALL, + CAN_VIEW_XFORM_DATA, + CAN_MOVE_TO_FOLDER, + CAN_TRANSFER_OWNERSHIP, ], } ROLES_ORDERED = [ - ReadOnlyRoleNoDownload, ReadOnlyRole, DataEntryOnlyRole, - DataEntryMinorRole, DataEntryRole, EditorMinorRole, EditorRole, - ManagerRole, OwnerRole + ReadOnlyRoleNoDownload, + ReadOnlyRole, + DataEntryOnlyRole, + DataEntryMinorRole, + DataEntryRole, + EditorMinorRole, + EditorRole, + ManagerRole, + OwnerRole, ] ROLES = {role.name: role for role in ROLES_ORDERED} @@ -347,8 +428,7 @@ def is_organization(obj): UserProfiles do. Check for that first since it avoids a database hit. """ try: - return (hasattr(obj, 'userprofile_ptr') - or obj.organizationprofile is not None) + return hasattr(obj, "userprofile_ptr") or obj.organizationprofile is not None except OrganizationProfile.DoesNotExist: return False @@ -369,7 +449,7 @@ def get_role_in_org(user, organization): """ perms = get_perms(user, organization) - if 'is_org_owner' in perms: + if "is_org_owner" in perms: return OwnerRole.name return get_role(perms, organization) or MemberRole.name @@ -382,8 +462,11 @@ def get_user_perms(obj): model = XFormUserObjectPermission if isinstance(obj, XForm) else None model = ProjectUserObjectPermission if isinstance(obj, Project) else model - return queryset_iterator( - model.objects.filter(content_object_id=obj.pk)) if model else None + return ( + queryset_iterator(model.objects.filter(content_object_id=obj.pk)) + if model + else None + ) def get_group_perms(obj): @@ -393,8 +476,11 @@ def get_group_perms(obj): model = XFormGroupObjectPermission if isinstance(obj, XForm) else None model = ProjectGroupObjectPermission if isinstance(obj, Project) else model - return queryset_iterator( - model.objects.filter(content_object_id=obj.pk)) if model else None + return ( + queryset_iterator(model.objects.filter(content_object_id=obj.pk)) + if model + else None + ) def _get_group_users_with_perms(obj, attach_perms=False, user_perms=None): @@ -404,7 +490,8 @@ def _get_group_users_with_perms(obj, attach_perms=False, user_perms=None): group_obj_perms = get_group_perms(obj) if group_obj_perms is None: return get_users_with_perms( - obj, attach_perms=attach_perms, with_group_users=True) + obj, attach_perms=attach_perms, with_group_users=True + ) group_users = {} if attach_perms: if user_perms: @@ -420,9 +507,8 @@ def _get_group_users_with_perms(obj, attach_perms=False, user_perms=None): group_users[user] = set([perm.permission.codename]) else: group_users = set() if not user_perms else set(user_perms) - for perm in group_obj_perms.distinct('group'): - group_users.union( - set([user for user in perm.group.user_set.all()])) + for perm in group_obj_perms.distinct("group"): + group_users.union(set(user for user in perm.group.user_set.all())) group_users = list(group_obj_perms) return group_users @@ -435,7 +521,8 @@ def _get_users_with_perms(obj, attach_perms=False, with_group_users=None): user_obj_perms = get_user_perms(obj) if user_obj_perms is None: return get_users_with_perms( - obj, attach_perms=attach_perms, with_group_users=with_group_users) + obj, attach_perms=attach_perms, with_group_users=with_group_users + ) user_perms = {} if attach_perms: for perm in user_obj_perms: @@ -445,7 +532,7 @@ def _get_users_with_perms(obj, attach_perms=False, with_group_users=None): user_perms[perm.user] = set([perm.permission.codename]) else: user_perms = [ - perm.user for perm in user_obj_perms.only('user').distinct('user') + perm.user for perm in user_obj_perms.only("user").distinct("user") ] if with_group_users: @@ -454,9 +541,10 @@ def _get_users_with_perms(obj, attach_perms=False, with_group_users=None): return user_perms -def get_object_users_with_permissions(obj, # pylint: disable=invalid-name - username=False, - with_group_users=False): +# pylint: disable=invalid-name +def get_object_users_with_permissions( + obj, username=False, with_group_users=False # pylint: disable=invalid-name +): """ Returns users, roles and permissions for an object. @@ -467,17 +555,21 @@ def get_object_users_with_permissions(obj, # pylint: disable=invalid-name if obj: users_with_perms = _get_users_with_perms( - obj, attach_perms=True, with_group_users=with_group_users).items() - - result = [{ - 'user': user.username if username else user, - 'first_name': user.first_name, - 'last_name': user.last_name, - 'role': get_role(permissions, obj), - 'is_org': is_organization(user.profile), - 'gravatar': user.profile.gravatar, - 'metadata': user.profile.metadata - } for user, permissions in users_with_perms] + obj, attach_perms=True, with_group_users=with_group_users + ).items() + + result = [ + { + "user": user.username if username else user, + "first_name": user.first_name, + "last_name": user.last_name, + "role": get_role(permissions, obj), + "is_org": is_organization(user.profile), + "gravatar": user.profile.gravatar, + "metadata": user.profile.metadata, + } + for user, permissions in users_with_perms + ] return result @@ -494,45 +586,49 @@ def get_team_project_default_permissions(team, project): def _check_meta_perms_enabled(xform): """ - Check for meta-perms settings in the xform metadata model. - :param xform: - :return: bool + Check for meta-perms settings in the xform metadata model. + :param xform: + :return: bool """ return xform.metadata_set.filter(data_type=XFORM_META_PERMS).count() > 0 -def exclude_items_from_queryset_using_xform_meta_perms( - xform, user, queryset): +# pylint: disable=invalid-name +def exclude_items_from_queryset_using_xform_meta_perms(xform, user, queryset): """ Exclude instances from the queryset if meta-perms have been enabled """ - if user.has_perm(CAN_VIEW_XFORM_ALL, xform) or xform.shared_data \ - or not _check_meta_perms_enabled(xform): + if ( + user.has_perm(CAN_VIEW_XFORM_ALL, xform) + or xform.shared_data + or not _check_meta_perms_enabled(xform) + ): return queryset - elif user.has_perm(CAN_VIEW_XFORM_DATA, xform): + if user.has_perm(CAN_VIEW_XFORM_DATA, xform): if queryset.model is Attachment: - return queryset.exclude( - ~Q(instance__user=user), instance__xform=xform) - else: - return queryset.exclude( - ~Q(user=user), xform=xform) + return queryset.exclude(~Q(instance__user=user), instance__xform=xform) + return queryset.exclude(~Q(user=user), xform=xform) + return queryset.none() def filter_queryset_xform_meta_perms(xform, user, instance_queryset): """ - Check for the specific perms if meta-perms have been enabled - CAN_VIEW_XFORM_ALL ==> User should be able to view all the data - CAN_VIEW_XFORM_DATA ===> User should be able to view his/her submitted - data. Otherwise should raise forbidden error. - :param xform: - :param user: - :param instance_queryset: - :return: data - """ - if user.has_perm(CAN_VIEW_XFORM_ALL, xform) or xform.shared_data \ - or not _check_meta_perms_enabled(xform): + Check for the specific perms if meta-perms have been enabled + CAN_VIEW_XFORM_ALL ==> User should be able to view all the data + CAN_VIEW_XFORM_DATA ===> User should be able to view his/her submitted + data. Otherwise should raise forbidden error. + :param xform: + :param user: + :param instance_queryset: + :return: data + """ + if ( + user.has_perm(CAN_VIEW_XFORM_ALL, xform) + or xform.shared_data + or not _check_meta_perms_enabled(xform) + ): return instance_queryset - elif user.has_perm(CAN_VIEW_XFORM_DATA, xform): + if user.has_perm(CAN_VIEW_XFORM_DATA, xform): return instance_queryset.filter(user=user) return instance_queryset.none() @@ -540,33 +636,35 @@ def filter_queryset_xform_meta_perms(xform, user, instance_queryset): def filter_queryset_xform_meta_perms_sql(xform, user, query): """ - Check for the specific perms if meta-perms have been enabled - CAN_VIEW_XFORM_ALL ==> User should be able to view all the data - CAN_VIEW_XFORM_DATA ===> User should be able to view his/her submitted - data. Otherwise should raise forbidden error. - :param xform: - :param user: - :param instance_queryset: - :return: data - """ - if user.has_perm(CAN_VIEW_XFORM_ALL, xform) or xform.shared_data\ - or not _check_meta_perms_enabled(xform): + Check for the specific perms if meta-perms have been enabled + CAN_VIEW_XFORM_ALL ==> User should be able to view all the data + CAN_VIEW_XFORM_DATA ===> User should be able to view his/her submitted + data. Otherwise should raise forbidden error. + :param xform: + :param user: + :param instance_queryset: + :return: data + """ + if ( + user.has_perm(CAN_VIEW_XFORM_ALL, xform) + or xform.shared_data + or not _check_meta_perms_enabled(xform) + ): return query - elif user.has_perm(CAN_VIEW_XFORM_DATA, xform): + if user.has_perm(CAN_VIEW_XFORM_DATA, xform): try: if query and isinstance(query, six.string_types): query = json.loads(query) if isinstance(query, list): query = query[0] else: - query = dict() + query = {} query.update({"_submitted_by": user.username}) return query except (ValueError, AttributeError): - query_list = list() + query_list = [] query_list.append({"_submitted_by": user.username}) query_list.append(query) return query_list - else: - raise NoRecordsPermission() + raise NoRecordsPermission() diff --git a/onadata/libs/serializers/project_serializer.py b/onadata/libs/serializers/project_serializer.py index 3e48abc7bc..cb51776aa9 100644 --- a/onadata/libs/serializers/project_serializer.py +++ b/onadata/libs/serializers/project_serializer.py @@ -24,7 +24,6 @@ get_role, is_organization, ) -from onadata.libs.serializers.dataview_serializer import DataViewMinimalSerializer from onadata.libs.serializers.fields.json_field import JsonField from onadata.libs.serializers.tag_list_serializer import TagListSerializer from onadata.libs.utils.analytics import TrackObjectEvent @@ -622,6 +621,11 @@ def get_data_views(self, obj): else obj.dataview_set.filter(deleted_at__isnull=True) ) + # pylint: disable=import-outside-toplevel + from onadata.libs.serializers.dataview_serializer import ( + DataViewMinimalSerializer, + ) + serializer = DataViewMinimalSerializer( data_views_obj, many=True, context=self.context ) diff --git a/onadata/libs/test_utils/md_table.py b/onadata/libs/test_utils/md_table.py index 01a4850e8b..c79de9606e 100644 --- a/onadata/libs/test_utils/md_table.py +++ b/onadata/libs/test_utils/md_table.py @@ -22,8 +22,7 @@ def _extract_array(mdtablerow): mtchstr = match.groups()[0] if re.match(r"^[\|-]+$", mtchstr): return False - else: - return [_strp_cell(c) for c in mtchstr.split("|")] + return [_strp_cell(c) for c in mtchstr.split("|")] return False @@ -37,6 +36,7 @@ def _is_null_row(r_arr): def md_table_to_ss_structure(mdstr: str) -> List[Tuple[str, List[List[str]]]]: + """Transform markdown to an ss structure""" ss_arr = [] for item in mdstr.split("\n"): arr = _extract_array(item) @@ -61,12 +61,13 @@ def md_table_to_ss_structure(mdstr: str) -> List[Tuple[str, List[List[str]]]]: def md_table_to_workbook(mdstr: str) -> Workbook: """ - Convert Markdown table string to an openpyxl.Workbook. Call wb.save() to persist. + Convert Markdown table string to an openpyxl.Workbook. Call workbook.save() to + persist. """ md_data = md_table_to_ss_structure(mdstr=mdstr) - wb = Workbook(write_only=True) + workbook = Workbook(write_only=True) for key, rows in md_data: - sheet = wb.create_sheet(title=key) - for r in rows: - sheet.append(r) - return wb + sheet = workbook.create_sheet(title=key) + for row in rows: + sheet.append(row) + return workbook diff --git a/onadata/libs/test_utils/pyxform_test_case.py b/onadata/libs/test_utils/pyxform_test_case.py index 6fd6126430..4d46c83bf9 100644 --- a/onadata/libs/test_utils/pyxform_test_case.py +++ b/onadata/libs/test_utils/pyxform_test_case.py @@ -13,20 +13,21 @@ from lxml import etree -# noinspection PyProtectedMember -from lxml.etree import _Element - from pyxform.builder import create_survey_element_from_dict from pyxform.errors import PyXFormError from pyxform.utils import NSMAP from pyxform.validators.odk_validate import ODKValidateError, check_xform from pyxform.xls2json import workbook_to_json + from onadata.libs.test_utils.md_table import md_table_to_ss_structure logger = logging.getLogger(__name__) logger.addHandler(logging.StreamHandler()) logger.setLevel(logging.DEBUG) +# noinspection PyProtectedMember +_Element = etree._Element # pylint: disable=protected-access + if TYPE_CHECKING: from typing import Dict, List, Set, Tuple, Union @@ -35,11 +36,13 @@ class PyxformTestError(Exception): - pass + """Pyxform test errors exception class.""" @dataclass class MatcherContext: + """Data class to store assertion context information.""" + debug: bool nsmap_xpath: "Dict[str, str]" nsmap_subs: "NSMAPSubs" @@ -47,9 +50,10 @@ class MatcherContext: class PyxformMarkdown: - """Transform markdown formatted xlsform to a pyxform survey object""" + """Transform markdown formatted XLSForm to a pyxform survey object""" def md_to_pyxform_survey(self, md_raw, kwargs=None, autoname=True, warnings=None): + """Transform markdown formatted XLSForm to pyxform survey object.""" if kwargs is None: kwargs = {} if autoname: @@ -59,13 +63,13 @@ def md_to_pyxform_survey(self, md_raw, kwargs=None, autoname=True, warnings=None if re.match(r"^\s+#", line): # ignore lines which start with pound sign continue - elif re.match(r"^(.*)(#[^|]+)$", line): + if re.match(r"^(.*)(#[^|]+)$", line): # keep everything before the # outside of the last occurrence # of | _md.append(re.match(r"^(.*)(#[^|]+)$", line).groups()[0].strip()) else: _md.append(line.strip()) - md = "\n".join(_md) + md = "\n".join(_md) # pylint: disable=invalid-name if kwargs.get("debug"): logger.debug(md) @@ -75,8 +79,7 @@ def list_to_dicts(arr): def _row_to_dict(row): out_dict = {} - for i in range(0, len(row)): - col = row[i] + for i, col in enumerate(row): if col not in [None, ""]: out_dict[headers[i]] = col return out_dict @@ -106,12 +109,13 @@ def _ss_structure_to_pyxform_survey(ss_structure, kwargs, warnings=None): def _run_odk_validate(xml): # On Windows, NamedTemporaryFile must be opened exclusively. # So it must be explicitly created, opened, closed, and removed + # pylint: disable=consider-using-with tmp = tempfile.NamedTemporaryFile(suffix=".xml", delete=False) tmp.close() try: - with codecs.open(tmp.name, mode="w", encoding="utf-8") as fp: - fp.write(xml) - fp.close() + with codecs.open(tmp.name, mode="w", encoding="utf-8") as file_handle: + file_handle.write(xml) + file_handle.close() check_xform(tmp.name) finally: # Clean up the temporary file @@ -137,9 +141,12 @@ def _autoname_inputs(kwargs): class PyxformTestCase(PyxformMarkdown, TestCase): + """The pyxform markdown TestCase class""" + maxDiff = None - def assertPyxformXform(self, **kwargs): + # pylint: disable=invalid-name,too-many-locals,too-many-branches,too-many-statements + def assertPyxformXform(self, **kwargs): # noqa """ PyxformTestCase.assertPyxformXform() named arguments: ----------------------------------------------------- @@ -213,12 +220,12 @@ def assertPyxformXform(self, **kwargs): survey_valid = True try: - if "md" in kwargs.keys(): + if "md" in kwargs: kwargs = self._autoname_inputs(kwargs) survey = self.md_to_pyxform_survey( kwargs.get("md"), kwargs, warnings=warnings ) - elif "ss_structure" in kwargs.keys(): + elif "ss_structure" in kwargs: kwargs = self._autoname_inputs(kwargs) survey = self._ss_structure_to_pyxform_survey( kwargs.get("ss_structure"), @@ -228,7 +235,7 @@ def assertPyxformXform(self, **kwargs): else: survey = kwargs.get("survey") - xml = survey._to_pretty_xml() + xml = survey._to_pretty_xml() # pylint: disable=protected-access root = etree.fromstring(xml.encode("utf-8")) # Ensure all namespaces are present, even if unused @@ -249,7 +256,7 @@ def assertPyxformXform(self, **kwargs): def _pull_xml_node_from_root(element_selector): _r = root.findall( - ".//n:%s" % element_selector, + f".//n:{element_selector}", namespaces={"n": "http://www.w3.org/2002/xforms"}, ) if _r: @@ -279,7 +286,7 @@ def _pull_xml_node_from_root(element_selector): + "'odk_validate_error__contains'" + " was empty:" + str(e) - ) + ) from e for v_err in odk_validate_error__contains: self.assertContains( e.args[0], v_err, msg_prefix="odk_validate_error__contains" @@ -288,22 +295,21 @@ def _pull_xml_node_from_root(element_selector): if survey_valid: def _check(keyword, verb): - verb_str = "%s__%s" % (keyword, verb) + verb_str = f"{keyword}__{verb}" - bad_kwarg = "%s_%s" % (code, verb) + bad_kwarg = f"{code}_{verb}" if bad_kwarg in kwargs: - good_kwarg = "%s__%s" % (code, verb) + good_kwarg = f"{code}__{verb}" raise SyntaxError( ( - "'%s' is not a valid parameter. " - "Use double underscores: '%s'" + f"'{bad_kwarg}' is not a valid parameter. " + f"Use double underscores: '{good_kwarg}'" ) - % (bad_kwarg, good_kwarg) ) def check_content(content, expected): if content is None: - self.fail(msg="No '{}' found in document.".format(keyword)) + self.fail(msg=f"No '{keyword}' found in document.") cstr = etree.tostring(content, encoding=str, pretty_print=True) matcher_context = MatcherContext( debug=debug, @@ -342,7 +348,7 @@ def check_content(content, expected): if "body_contains" in kwargs or "body__contains" in kwargs: raise SyntaxError( - "Invalid parameter: 'body__contains'." "Use 'xml__contains' instead" + "Invalid parameter: 'body__contains'.Use 'xml__contains' instead" ) for code in ["xml", "instance", "model", "itext"]: @@ -364,7 +370,7 @@ def check_content(content, expected): "and or optionally 'error__contains=[...]'" "\nError(s): " + "\n".join(errors) ) - elif survey_valid and expecting_invalid_survey: + if survey_valid and expecting_invalid_survey: raise PyxformTestError("Expected survey to be invalid.") search_test_kwargs = ( @@ -381,13 +387,13 @@ def check_content(content, expected): elif k.endswith("__not_contains"): assertion = self.assertNotContains else: - raise PyxformTestError("Unexpected search test kwarg: {}".format(k)) + raise PyxformTestError(f"Unexpected search test kwarg: {k}") if k.startswith("error"): joined = "\n".join(errors) elif k.startswith("warnings"): joined = "\n".join(warnings) else: - raise PyxformTestError("Unexpected search test kwarg: {}".format(k)) + raise PyxformTestError(f"Unexpected search test kwarg: {k}") for text in kwargs[k]: assertion(joined, text, msg_prefix=k) if "warnings_count" in kwargs: @@ -408,7 +414,7 @@ def _assert_contains(content, text, msg_prefix): return text_repr, real_count, msg_prefix - def assertContains(self, content, text, count=None, msg_prefix=""): + def assertContains(self, content, text, count=None, msg_prefix=""): # noqa """ FROM: django source- testcases.py @@ -424,16 +430,16 @@ def assertContains(self, content, text, count=None, msg_prefix=""): self.assertEqual( real_count, count, - msg_prefix + "Found %d instances of %s in content" - " (expected %d)" % (real_count, text_repr, count), + msg_prefix + f"Found {real_count} instances of {text_repr} in content" + f" (expected {count})", ) else: self.assertTrue( real_count != 0, - msg_prefix + "Couldn't find %s in content:\n" % text_repr + content, + msg_prefix + f"Couldn't find {text_repr + content} in content:\n", ) - def assertNotContains(self, content, text, msg_prefix=""): + def assertNotContains(self, content, text, msg_prefix=""): # noqa """ Asserts that a content indicates that some content was retrieved successfully, (i.e., the HTTP status code was as expected), and that @@ -446,7 +452,7 @@ def assertNotContains(self, content, text, msg_prefix=""): self.assertEqual( real_count, 0, - msg_prefix + "Response should not contain %s" % text_repr, + msg_prefix + f"Response should not contain {text_repr}", ) def assert_xpath_exact( @@ -459,9 +465,10 @@ def assert_xpath_exact( """ Process an assertion for xml__xpath_exact. - Compares result strings since expected strings may contain xml namespace prefixes. - To allow parsing required to compare as ETrees would require injecting namespace - declarations into the expected match strings. + Compares result strings since expected strings may contain xml namespace + prefixes. + To allow parsing required to compare as ETrees would require injecting + namespace declarations into the expected match strings. :param matcher_context: A MatcherContext dataclass. :param content: XML to be examined. @@ -502,7 +509,10 @@ def assert_xpath_count( content=content, xpath=xpath, ) - msg = f"XPath found no matches:\n{xpath}\n\nXForm content:\n{matcher_context.content_str}" + msg = ( + f"XPath found no matches:\n{xpath}\n\n" + f"XForm content:\n{matcher_context.content_str}" + ) self.assertEqual(expected, len(observed), msg=msg) @@ -523,8 +533,8 @@ def reorder_attributes(root): In utils.node, it is based on xml.dom.minidom.Element objects. See https://github.com/XLSForm/pyxform/issues/414. """ - for el in root.iter(): - attrib = el.attrib + for elem in root.iter(): + attrib = elem.attrib if len(attrib) > 1: # Sort attributes. Attributes are represented as {namespace}name # so attributes with explicit namespaces will always sort after @@ -540,20 +550,21 @@ def xpath_clean_result_strings( """ Clean XPath results: stringify, remove namespace declarations, clean up whitespace. - :param nsmap_subs: namespace replacements e.g. [('x="http://www.w3.org/2002/xforms", "")] + :param nsmap_subs: namespace replacements e.g. + [('x="http://www.w3.org/2002/xforms", "")] :param results: XPath results to clean. """ xmlex = [(" >", ">"), (" />", "/>")] subs = nsmap_subs + xmlex cleaned = set() - for x in results: - if isinstance(x, _Element): - reorder_attributes(x) - x = etree.tostring(x, encoding=str, pretty_print=True) - x = x.strip() - for s in subs: - x = x.replace(*s) - cleaned.add(x) + for result in results: + if isinstance(result, _Element): + reorder_attributes(result) + result = etree.tostring(result, encoding=str, pretty_print=True) + result = result.strip() + for sub in subs: + result = result.replace(*sub) + cleaned.add(result) return cleaned @@ -580,15 +591,14 @@ def xpath_evaluate( raise PyxformTestError(msg) from e if matcher_context.debug: if 0 == len(results): - logger.debug(f"Results for XPath: {xpath}\n" + "(No matches)" + "\n") + logger.debug("Results for XPath: %s\n(No matches)\n", xpath) else: cleaned = xpath_clean_result_strings( nsmap_subs=matcher_context.nsmap_subs, results=results ) - logger.debug(f"Results for XPath: {xpath}\n" + "\n".join(cleaned) + "\n") + logger.debug("Results for XPath: %s\n%s\n", xpath, "\n".join(cleaned)) if for_exact: return xpath_clean_result_strings( nsmap_subs=matcher_context.nsmap_subs, results=results ) - else: - return set(results) + return set(results) diff --git a/onadata/libs/tests/utils/test_cache_tools.py b/onadata/libs/tests/utils/test_cache_tools.py index aebf48920d..8b5644a55b 100644 --- a/onadata/libs/tests/utils/test_cache_tools.py +++ b/onadata/libs/tests/utils/test_cache_tools.py @@ -2,17 +2,28 @@ """ Test onadata.libs.utils.cache_tools module. """ +from unittest import TestCase + +from django.contrib.auth import get_user_model from django.core.cache import cache -from django.contrib.auth.models import User from django.http.request import HttpRequest -from unittest import TestCase -from onadata.apps.main.models.user_profile import UserProfile from onadata.apps.logger.models.project import Project +from onadata.apps.main.models.user_profile import UserProfile +from onadata.libs.serializers.project_serializer import ProjectSerializer from onadata.libs.utils.cache_tools import ( - PROJ_PERM_CACHE, PROJ_NUM_DATASET_CACHE, PROJ_SUB_DATE_CACHE, - PROJ_FORMS_CACHE, PROJ_BASE_FORMS_CACHE, PROJ_OWNER_CACHE, - safe_key, reset_project_cache, project_cache_prefixes) + PROJ_BASE_FORMS_CACHE, + PROJ_FORMS_CACHE, + PROJ_NUM_DATASET_CACHE, + PROJ_OWNER_CACHE, + PROJ_PERM_CACHE, + PROJ_SUB_DATE_CACHE, + project_cache_prefixes, + reset_project_cache, + safe_key, +) + +User = get_user_model() class TestCacheTools(TestCase): @@ -22,75 +33,75 @@ def test_safe_key(self): """Test safe_key() function returns a hashed key""" self.assertEqual( safe_key("hello world"), - "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9") + "b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9", + ) def test_reset_project_cache(self): """ Test reset_project_cache() function actually resets all project cache entries """ - bob = User.objects.create(username='bob', first_name='bob') + bob = User.objects.create(username="bob", first_name="bob") UserProfile.objects.create(user=bob) project = Project.objects.create( - name='Some Project', created_by=bob, organization=bob) + name="Some Project", created_by=bob, organization=bob + ) # Set dummy values in cache for prefix in project_cache_prefixes: - cache.set(f'{prefix}{project.pk}', 'stale') + cache.set(f"{prefix}{project.pk}", "stale") request = HttpRequest() request.user = bob - request.META = { - 'SERVER_NAME': 'testserver', - 'SERVER_PORT': '80' - } - reset_project_cache(project, request) + request.META = {"SERVER_NAME": "testserver", "SERVER_PORT": "80"} + reset_project_cache(project, request, ProjectSerializer) expected_project_cache = { - 'url': f'http://testserver/api/v1/projects/{project.pk}', - 'projectid': project.pk, - 'owner': 'http://testserver/api/v1/users/bob', - 'created_by': 'http://testserver/api/v1/users/bob', - 'metadata': {}, - 'starred': False, - 'users': [{ - 'is_org': False, - 'metadata': {}, - 'first_name': 'bob', - 'last_name': '', - 'user': 'bob', - 'role': 'owner' - }], - 'forms': [], - 'public': False, - 'tags': [], - 'num_datasets': 0, - 'last_submission_date': None, - 'teams': [], - 'data_views': [], - 'name': 'Some Project', - 'deleted_at': None + "url": f"http://testserver/api/v1/projects/{project.pk}", + "projectid": project.pk, + "owner": "http://testserver/api/v1/users/bob", + "created_by": "http://testserver/api/v1/users/bob", + "metadata": {}, + "starred": False, + "users": [ + { + "is_org": False, + "metadata": {}, + "first_name": "bob", + "last_name": "", + "user": "bob", + "role": "owner", + } + ], + "forms": [], + "public": False, + "tags": [], + "num_datasets": 0, + "last_submission_date": None, + "teams": [], + "data_views": [], + "name": "Some Project", + "deleted_at": None, } self.assertEqual( - cache.get(f'{PROJ_PERM_CACHE}{project.pk}'), - expected_project_cache['users']) + cache.get(f"{PROJ_PERM_CACHE}{project.pk}"), expected_project_cache["users"] + ) self.assertEqual( - cache.get(f'{PROJ_NUM_DATASET_CACHE}{project.pk}'), - expected_project_cache['num_datasets']) + cache.get(f"{PROJ_NUM_DATASET_CACHE}{project.pk}"), + expected_project_cache["num_datasets"], + ) self.assertEqual( - cache.get(f'{PROJ_SUB_DATE_CACHE}{project.pk}'), - expected_project_cache['last_submission_date']) + cache.get(f"{PROJ_SUB_DATE_CACHE}{project.pk}"), + expected_project_cache["last_submission_date"], + ) self.assertEqual( - cache.get(f'{PROJ_FORMS_CACHE}{project.pk}'), - expected_project_cache['forms']) - self.assertEqual( - cache.get(f'{PROJ_BASE_FORMS_CACHE}{project.pk}'), - None) + cache.get(f"{PROJ_FORMS_CACHE}{project.pk}"), + expected_project_cache["forms"], + ) + self.assertEqual(cache.get(f"{PROJ_BASE_FORMS_CACHE}{project.pk}"), None) - project_cache = cache.get(f'{PROJ_OWNER_CACHE}{project.pk}') - project_cache.pop('date_created') - project_cache.pop('date_modified') - self.assertEqual( - project_cache, - expected_project_cache) + project_cache = cache.get(f"{PROJ_OWNER_CACHE}{project.pk}") + project_cache.pop("date_created") + project_cache.pop("date_modified") + self.assertEqual(project_cache, expected_project_cache) diff --git a/onadata/libs/utils/cache_tools.py b/onadata/libs/utils/cache_tools.py index 13f563408f..7442253f73 100644 --- a/onadata/libs/utils/cache_tools.py +++ b/onadata/libs/utils/cache_tools.py @@ -67,12 +67,10 @@ def safe_key(key): return hashlib.sha256(force_bytes(key)).hexdigest() -def reset_project_cache(project, request): +def reset_project_cache(project, request, project_serializer_class): """ Clears and sets project cache """ - # pylint: disable=import-outside-toplevel - from onadata.libs.serializers.project_serializer import ProjectSerializer # Clear all project cache entries for prefix in project_cache_prefixes: @@ -80,5 +78,7 @@ def reset_project_cache(project, request): # Reserialize project and cache value # Note: The ProjectSerializer sets all the other cache entries - project_cache_data = ProjectSerializer(project, context={"request": request}).data + project_cache_data = project_serializer_class( + project, context={"request": request} + ).data cache.set(f"{PROJ_OWNER_CACHE}{project.pk}", project_cache_data) diff --git a/onadata/libs/utils/osm.py b/onadata/libs/utils/osm.py index d813dc7fdf..5ea6c34cdf 100644 --- a/onadata/libs/utils/osm.py +++ b/onadata/libs/utils/osm.py @@ -26,7 +26,7 @@ def _get_xml_obj(xml): try: return fromstring(xml) except _etree.XMLSyntaxError as e: # pylint: disable=c-extension-no-member - if "Attribute action redefined" in e.msg: + if "Attribute action redefined" in str(e): xml = xml.replace(b'action="modify" ', b"") return _get_xml_obj(xml)