Apply changes from first day of reviewing kobotoolbox#3057

with @noliveleger

kpi/deployment_backends/kobocat_backend.py

@@ -61,8 +61,8 @@ class KobocatDeploymentBackend(BaseDeploymentBackend):
     ]
 
     SYNCED_DATA_FILE_TYPES = {
-        AssetFile.FORM_MEDIA: AssetFile.BACKEND_DATA_TYPE,
-        AssetFile.PAIRED_DATA: PairedData.BACKEND_DATA_TYPE,
+        AssetFile.FORM_MEDIA: 'media',
+        AssetFile.PAIRED_DATA: 'paired_data',
     }
 
     def bulk_assign_mapped_perms(self):
@@ -810,36 +810,37 @@ def sync_media_files(self, file_type: str = AssetFile.FORM_MEDIA):
 
         queryset = self._get_metadata_queryset(file_type=file_type)
 
-        for obj in queryset:
+        for media_file in queryset:
 
-            uniq = obj.backend_uniqid
+            backend_media_id = media_file.backend_media_id
 
             # File does not exist in KC
-            if uniq not in kc_filenames:
-                if obj.deleted_at is None:
+            if backend_media_id not in kc_filenames:
+                if media_file.deleted_at is None:
                     # New file
-                    self.__save_kc_metadata(obj)
+                    self.__save_kc_metadata(media_file)
                 else:
                     # Orphan, delete it
-                    obj.delete(force=True)
+                    media_file.delete(force=True)
                 continue
 
             # Existing file
-            if uniq in kc_filenames:
-                kc_file = kc_files[uniq]
-                if obj.deleted_at is None:
+            if backend_media_id in kc_filenames:
+                kc_file = kc_files[backend_media_id]
+                if media_file.deleted_at is None:
                     # If md5 differs, we need to re-upload it.
-                    if obj.md5_hash != kc_file['md5']:
+                    if media_file.md5_hash != kc_file['md5']:
                         self.__delete_kc_metadata(kc_file)
-                        self.__save_kc_metadata(obj)
+                        self.__save_kc_metadata(media_file)
                 elif kc_file['from_kpi']:
-                    self.__delete_kc_metadata(kc_file, obj)
+                    self.__delete_kc_metadata(kc_file, media_file)
                 else:
                     # Remote file has been uploaded directly to KC. We
                     # cannot delete it, but we need to vacuum KPI.
-                    obj.delete(force=True)
-                    # Skip deletion of key corresponding to `uniq` in `kc_files`
-                    # to avoid unique constraint failure in case user deleted
+                    media_file.delete(force=True)
+                    # Skip deletion of key corresponding to `backend_media_id`
+                    # in `kc_files` to avoid unique constraint failure in case
+                    # user deleted
                     # and re-uploaded the same file in a row between
                     # two deployments
                     # Example:
@@ -855,7 +856,7 @@ def sync_media_files(self, file_type: str = AssetFile.FORM_MEDIA):
                 # Remove current filename from `kc_files`.
                 # All files which will remain in this dict (after this loop)
                 # will be considered obsolete and will be deleted
-                del kc_files[uniq]
+                del kc_files[backend_media_id]
 
         # Remove KC orphan files previously uploaded through KPI
         for kc_file in kc_files.values():
@@ -1226,9 +1227,9 @@ def __save_kc_metadata(self, file_: SyncBackendMediaInterface):
 
         kwargs = {
             'data': {
-                'data_value': file_.backend_data_value,
+                'data_value': file_.backend_media_id,
                 'xform': self.xform_id,
-                'data_type': file_.backend_data_type,
+                'data_type': self.SYNCED_DATA_FILE_TYPES[file_.file_type],
                 'from_kpi': True,
                 'data_filename': file_.filename,
                 'data_file_type': file_.mimetype,

kpi/exceptions.py

@@ -55,10 +55,6 @@ class ImportAssetException(Exception):
     pass
 
 
-class ImportAssetException(Exception):
-    pass
-
-
 class InvalidSearchException(exceptions.APIException):
     status_code = 400
     default_detail = _('Invalid search. Please try again')
@@ -112,15 +108,6 @@ class ObjectDeploymentDoesNotExist(exceptions.APIException):
     default_code = 'deployment_does_not_exist'
 
 
-class PairedParentException(Exception):
-
-    def __init__(self,
-                 message=_('Parent is not set. '
-                           'Must call `asset.get_paired_data()` first')):
-        self.message = message
-        super().__init__(self.message)
-
-
 class ReadOnlyModelError(Exception):
 
     def __init__(self, msg='This model is read only', *args, **kwargs):

kpi/filters.py

@@ -169,6 +169,8 @@ def _get_queryset_for_data_sharing_enabled(
 
         asset_ids = perms.values('asset')
 
+        # `SearchFilter` handles futher filtering to include only assets with
+        # `data_sharing__enabled` (`self.DATA_SHARING_PARAMETER`) set to true
         return queryset.filter(pk__in=asset_ids)
 
     def _get_discoverable(self, queryset):
@@ -256,7 +258,9 @@ def _get_queryset_for_discoverable_child_assets(
 
         if parent_obj.has_perm(get_anonymous_user(), PERM_DISCOVER_ASSET):
             self._return_queryset = True
-            return queryset.filter(pk__in=self._get_publics())
+            return queryset.filter(
+                pk__in=self._get_publics(), parent=parent_obj
+            )
 
         return queryset
 

kpi/interfaces/sync_backend_media.py

@@ -9,19 +9,8 @@ class SyncBackendMediaInterface:
 
     """
 
-    # Type of file sent to back end during synchronization
-    BACKEND_DATA_TYPE = None
-
-    @property
-    def backend_data_value(self):
-        raise AbstractPropertyError
-
-    @property
-    def backend_data_type(self):
-        raise AbstractPropertyError
-
     @property
-    def backend_uniqid(self):
+    def backend_media_id(self):
         raise AbstractPropertyError
 
     def delete(self, **kwargs):

kpi/models/asset.py

@@ -59,7 +59,6 @@
 from kpi.exceptions import (
     BadPermissionsException,
     DeploymentDataException,
-    PairedParentException,
 )
 from kpi.fields import (
     KpiUidField,
@@ -505,7 +504,7 @@ class Asset(ObjectPermissionMixin,
     # ToDo Move the field to another table with one-to-one relationship
     _deployment_data = JSONBField(default=dict)
 
-    # JSON with subset of fields and allowed users to use it
+    # JSON with subset of fields to share
     # {
     #   'enable': True,
     #   'fields': []  # shares all when empty
@@ -851,40 +850,6 @@ def get_label_for_permission(
         )
         return label
 
-    def get_paired_parent(self, paired_data_uid: str) -> Union['Asset', None]:
-
-        # Validate `paired_data_uid`, i.e., must exist in `self.paired_data`
-        parent_uid = None
-        for key, values in self.paired_data.items():
-            if values['paired_data_uid'] == paired_data_uid:
-                parent_uid = key
-                break
-
-        if not parent_uid:
-            return None
-
-        try:
-            parent_asset = Asset.objects.get(uid=parent_uid)
-        except Asset.DoesNotExist:
-            return None
-
-        # Data sharing must be enabled on the parent
-        parent_data_sharing = parent_asset.data_sharing
-        if not parent_data_sharing.get('enabled'):
-            return None
-
-        # Validate `self.owner` is still allowed to see parent data
-        allowed_users = parent_data_sharing.get('users', [])
-        if allowed_users and self.owner.username not in allowed_users:
-            return None
-
-        if not parent_asset.has_deployment:
-            return None
-
-        self.__parent_paired_asset = parent_asset
-
-        return parent_asset
-
     def get_partial_perms(
         self, user_id: int, with_filters: bool = False
     ) -> Union[list, dict, None]:
@@ -961,25 +926,6 @@ def latest_version(self):
         except IndexError:
             return None
 
-    @property
-    def paired_data_fields(self):
-        try:
-            parent_asset = self.__parent_paired_asset
-        except AttributeError:
-            raise PairedParentException
-
-        parent_fields = parent_asset.data_sharing['fields']
-        asset_fields = self.paired_data[parent_asset.uid]['fields']
-
-        if parent_fields and asset_fields:
-            return list(set(parent_fields).intersection(set(asset_fields)))
-
-        if not asset_fields:
-            return parent_fields
-
-        if not parent_fields:
-            return asset_fields
-
     @staticmethod
     def optimize_queryset_for_list(queryset):
         """ Used by serializers to improve performance when listing assets """

kpi/models/asset_file.py

@@ -36,8 +36,6 @@ class AssetFile(models.Model,
     FORM_MEDIA = 'form_media'
     PAIRED_DATA = 'paired_data'
 
-    BACKEND_DATA_TYPE = 'media'
-
     TYPE_CHOICES = (
         (MAP_LAYER, MAP_LAYER),
         (FORM_MEDIA, FORM_MEDIA),
@@ -81,34 +79,16 @@ class AssetFile(models.Model,
     synced_with_backend = models.BooleanField(default=False)
 
     @property
-    def backend_data_value(self):
+    def backend_media_id(self):
         """
-        Implements `SyncBackendMediaInterface.backend_data_value()`
+        Implements `SyncBackendMediaInterface.backend_media_id()`
         """
         return (
             self.metadata['redirect_url']
             if self.is_remote_url
             else self.filename
         )
 
-    @property
-    def backend_data_type(self):
-        """
-        Implements `SyncBackendMediaInterface.backend_data_type()`
-        """
-        return self.BACKEND_DATA_TYPE
-
-    @property
-    def backend_uniqid(self):
-        """
-        Implements `SyncBackendMediaInterface.backend_uniqid()`
-        """
-        return (
-            self.metadata['filename']
-            if not self.is_remote_url
-            else self.metadata['redirect_url']
-        )
-
     def delete(self, using=None, keep_parents=False, force=False):
         # Delete object and files on storage if `force` is True or file type
         # is anything else than 'form_media'

kpi/models/object_permission.py

@@ -811,7 +811,7 @@ def has_perm(self, user_obj: User, perm: str) -> bool:
                 return False
         return result
 
-    def has_perms(self, user_obj: User, perms: list, all_: bool = False) -> bool:  # noqa
+    def has_perms(self, user_obj: User, perms: list, all_: bool = True) -> bool:  # noqa
         """
         Returns True or False whether user `user_obj` has several
         permissions (`perms`) on current object.