Merge pull request #2533 from liqd/2016-07-as-delete-follow-up

Delete follow-up: metadata and options

src/adhocracy_core/adhocracy_core/catalog/adhocracy.py

@@ -9,7 +9,6 @@
 from substanced.util import find_service
 from adhocracy_core.catalog.index import ReferenceIndex
 from adhocracy_core.exceptions import RuntimeConfigurationError
-from adhocracy_core.utils import is_deleted
 from adhocracy_core.utils import is_hidden
 from adhocracy_core.interfaces import IItem
 from adhocracy_core.interfaces import search_query
@@ -43,7 +42,7 @@ class AdhocracyCatalogIndexes:
     """
 
     tag = catalog.Keyword()
-    private_visibility = catalog.Keyword()  # visible / deleted / hidden
+    private_visibility = catalog.Keyword()  # visible / hidden
     badge = catalog.Keyword()
     item_badge = catalog.Keyword()
     title = catalog.Field()
@@ -81,18 +80,12 @@ def index_item_creation_date(resource, default) -> str:
 def index_visibility(resource, default) -> [str]:
     """Return value for the private_visibility index.
 
-    The return value will be one of [visible], [deleted], [hidden], or
-    [deleted, hidden].
+    Te return value will be one of [visible], [hidden]
     """
-    # FIXME: be more dry, this almost the same like what
-    # utils.get_reason_if_blocked is doing
-    result = []
-    if is_deleted(resource):
-        result.append('deleted')
     if is_hidden(resource):
-        result.append('hidden')
-    if not result:
-        result.append('visible')
+        result = ['hidden']
+    else:
+        result = ['visible']
     return result
 
 

src/adhocracy_core/adhocracy_core/catalog/test_adhocracy.py

@@ -87,26 +87,12 @@ def test_index_visibility_visible(context):
     assert index_visibility(context, 'default') == ['visible']
 
 
-def test_index_visibility_deleted(context):
-    from .adhocracy import index_visibility
-    assert index_visibility(context, 'default') == ['visible']
-    context.deleted = True
-    assert index_visibility(context, 'default') == ['deleted']
-
-
 def test_index_visibility_hidden(context):
     from .adhocracy import index_visibility
     context.hidden = True
     assert index_visibility(context, 'default') == ['hidden']
 
 
-def test_index_visibility_both(context):
-    from .adhocracy import index_visibility
-    context.deleted = True
-    context.hidden = True
-    assert sorted(index_visibility(context, 'default')) == ['deleted', 'hidden']
-
-
 @mark.usefixtures('integration')
 def test_includeme_register_index_visibilityreator(registry):
     from adhocracy_core.sheets.metadata import IMetadata

src/adhocracy_core/adhocracy_core/interfaces.py

@@ -901,7 +901,7 @@ class SearchQuery(namedtuple('Query', ['interfaces',
     depth (int):
        path depth to search descendants
     only_visible (bool):
-        filter hidden and deleted resources
+        filter hidden resources
     allows ([str], str):
         filter resources that don't allow the :term:`principals <principal>`
         the given permission ([principal], permission).

src/adhocracy_core/adhocracy_core/rest/schemas.py

@@ -234,7 +234,7 @@ def _add_colander_invalid_error(error: Invalid, request: IRequest,
 def validate_visibility(view: callable):
     """Decorator for :term:`view` to check if `context` is visible.
 
-    :raises HTTPGone: if `context` is deleted or hidden and request method
+    :raises HTTPGone: if `context` is hidden and request method
                       is GET, HEAD, or POST.
     """
     def wrapped_view(context: IResource, request: IRequest):

src/adhocracy_core/adhocracy_core/rest/test_views.py

@@ -185,31 +185,6 @@ def test_options_without_delete_permission(self, request_, context):
                   'OPTIONS': {}}
         assert wanted == response
 
-    def test_add_metadata_permissions_info_no_metadata(self, request_, context):
-        inst = self.make_one(context, request_)
-        d = {'DummySheet': {}}
-        inst._add_metadata_edit_permission_info(d)
-        assert d == {'DummySheet': {}}
-
-    def test_add_metadata_permissions_info_without_hide_permission(
-            self, request_, context):
-        from adhocracy_core.sheets.metadata import IMetadata
-        request_.has_permission = Mock(return_value=False)
-        inst = self.make_one(context, request_)
-        d = {IMetadata.__identifier__: {}}
-        inst._add_metadata_edit_permission_info(d)
-        assert d == {IMetadata.__identifier__: {'deleted': [True, False]}}
-
-    def test_add_metadata_permissions_info_with_hide_permission(
-            self, request_, context):
-        from adhocracy_core.sheets.metadata import IMetadata
-        request_.has_permission = Mock(return_value=True)
-        inst = self.make_one(context, request_)
-        d = {IMetadata.__identifier__: {}}
-        inst._add_metadata_edit_permission_info(d)
-        assert d == {IMetadata.__identifier__: {'deleted': [True, False],
-                                                'hidden': [True, False]}}
-
     def test_add_workflow_permissions_info(
             self, request_, registry, context, mock_sheet, mock_workflow):
         from adhocracy_core.sheets.workflow import IWorkflowAssignment

src/adhocracy_core/adhocracy_core/rest/views.py

@@ -52,7 +52,6 @@
 from adhocracy_core.schema import References
 from adhocracy_core.sheets.badge import get_assignable_badges
 from adhocracy_core.sheets.badge import IBadgeAssignment
-from adhocracy_core.sheets.metadata import IMetadata
 from adhocracy_core.sheets.workflow import IWorkflowAssignment
 from adhocracy_core.sheets.pool import IPool as IPoolSheet
 from adhocracy_core.sheets.versions import IVersionable
@@ -102,7 +101,6 @@ def _options(self, context: IResource, request: IRequest) -> dict:
             put_sheets = [(s.meta.isheet.__identifier__, empty) for s in edits]
             if put_sheets:
                 put_sheets_dict = dict(put_sheets)
-                self._add_metadata_edit_permission_info(put_sheets_dict)
                 self._add_workflow_edit_permission_info(put_sheets_dict, edits)
                 cstruct['PUT']['request_body']['data'] = put_sheets_dict
             else:
@@ -148,16 +146,6 @@ def _options(self, context: IResource, request: IRequest) -> dict:
             del cstruct['POST']
         return cstruct
 
-    def _add_metadata_edit_permission_info(self, cstruct: dict):
-        """Add info if a user may set the hidden metadata fields."""
-        if IMetadata.__identifier__ not in cstruct:
-            return
-        # everybody who can PUT metadata can delete the resource
-        permission_info = {'deleted': [True, False]}
-        if self.request.has_permission('hide', self.context):
-            permission_info['hidden'] = [True, False]
-        cstruct[IMetadata.__identifier__] = permission_info
-
     def _add_workflow_edit_permission_info(self, cstruct: dict, edit_sheets):
         """Add info if a user may set the workflow_state workflow field."""
         workflow_sheets = [s for s in edit_sheets
@@ -177,7 +165,7 @@ def _add_workflow_edit_permission_info(self, cstruct: dict, edit_sheets):
         permission='view',
     )
     def get(self) -> dict:
-        """Get resource data (unless deleted or hidden)."""
+        """Get resource data (unless hidden)."""
         metric = self._get_get_metric_name()
         with statsd_timer(metric, rate=.1, registry=self.registry):
             schema = create_schema(GETResourceResponseSchema,

src/adhocracy_core/adhocracy_core/scaffolds/adhocracy/development.ini_tmpl

@@ -60,7 +60,7 @@ adhocracy_core.caching.http.mode = no_cache
 #adhocracy.varnish_url =
 # performance workaround: disable filter references by view permission
 #adhocracy.filter_by_view_permission = False
-# performance workaround: disable filter references by visible (not deleted or hidden)
+# performance workaround: disable filter references by visible (not hidden)
 #adhocracy.filter_by_visible = False
 
 mail.queue_path = %(here)s/../var/mail

src/adhocracy_core/adhocracy_core/sheets/metadata.py

@@ -69,9 +69,6 @@ class MetadataSchema(MappingSchema):
 
     `modification_date`: Modification date of this resource. defaults to now.
 
-    `deleted`: whether the resource is marked as deleted (only shown to those
-    that specifically ask for it)
-
     `hidden`: whether the resource is marked as hidden (only shown to those
     that have special permissions and ask for it)
 
@@ -82,7 +79,6 @@ class MetadataSchema(MappingSchema):
     item_creation_date = DateTime(missing=drop, readonly=True)
     modified_by = Reference(reftype=MetadataModifiedByReference, readonly=True)
     modification_date = DateTime(missing=drop, readonly=True)
-    deleted = Boolean()
     hidden = Boolean(validator=deferred_check_hide_permission)
 
 
@@ -93,7 +89,7 @@ class MetadataSchema(MappingSchema):
     editable=True,
     creatable=True,
     readable=True,
-    permission_edit='delete',
+    permission_edit='hide',
 )
 
 

src/adhocracy_core/adhocracy_core/sheets/test_metadata.py

@@ -26,7 +26,7 @@ def make_one(self, **kwargs):
     def test_deserialize_empty(self):
         inst = self.make_one()
         result = inst.deserialize({})
-        assert result == {'deleted': False, 'hidden': False}
+        assert result == {'hidden': False}
 
     def test_deserialize_hiding_requires_permission(self, context, request_):
         import colander
@@ -44,7 +44,6 @@ def test_serialize_empty(self):
         assert result['item_creation_date'] == null
         assert result['modification_date'] == null
         assert result['modified_by'] is None
-        assert result['deleted'] == 'false'
         assert result['hidden'] == 'false'
 
     def test_serialize_empty_and_bind(self, context, mock_now, request_):
@@ -74,6 +73,7 @@ def test_create(self, meta, context):
         assert inst.meta.creatable is True
         assert inst.meta.readable is True
         assert inst.meta.sheet_class is AttributeResourceSheet
+        assert inst.meta.permission_edit == 'hide'
 
     @mark.usefixtures('integration')
     def test_includeme_register_sheet(self, meta, config):

src/adhocracy_core/adhocracy_core/utils/__init__.py

@@ -179,18 +179,6 @@ def is_batchmode(request: Request) -> bool:
     return getattr(request, '__is_batchmode__', False)
 
 
-def is_deleted(resource: IResource) -> dict:
-    """Check whether a resource is deleted.
-
-    This also returns True for descendants of deleted resources, as a positive
-    deleted status is inherited.
-    """
-    for context in lineage(resource):
-        if getattr(context, 'deleted', False):
-            return True
-    return False
-
-
 def is_hidden(resource: IResource) -> dict:
     """Check whether a resource is hidden.
 
@@ -206,10 +194,8 @@ def is_hidden(resource: IResource) -> dict:
 def get_reason_if_blocked(resource: IResource) -> str:
     """Check if a resource is blocked and return Reason, None otherwise."""
     reason = None
-    if is_deleted(resource):
-        reason = 'deleted'
     if is_hidden(resource):
-        reason = 'both' if reason else 'hidden'
+        reason = 'hidden'
     return reason
 
 
@@ -256,13 +242,8 @@ def extract_events_from_changelog_metadata(meta: ChangelogMetadata) -> list:
 @dispatch(ResourceSheetModified)
 def get_visibility_change(event):
     """Return changed visbility for `event.object`."""
-
-    is_deleted = event.new_appstruct.get('deleted', False)
-    is_hidden = event.new_appstruct.get('hidden', False)
-    was_deleted = event.old_appstruct['deleted']
-    was_hidden = event.old_appstruct['hidden']
-    was_visible = not (was_hidden or was_deleted)
-    is_visible = not (is_hidden or is_deleted)
+    was_visible = not event.old_appstruct['hidden']
+    is_visible = not event.new_appstruct.get('hidden', False)
     if was_visible:
         if is_visible:
             return VisibilityChange.visible

src/adhocracy_core/adhocracy_core/utils/test_init.py

@@ -157,34 +157,18 @@ def test_provides_wrong_sheet(self):
         assert self.call_fut(context, IPredicateSheet) is None
 
 
-def test_get_reason_blocked_not_deleted_not_hidden(context):
+def test_get_reason_blocked_not_hidden(context):
     from . import get_reason_if_blocked
-    context.deleted = False
     context.hidden = False
     assert get_reason_if_blocked(context) == None
 
 
-def test_get_reason_blocked_is_deleted_not_hidden(context):
-    from . import get_reason_if_blocked
-    context.deleted = True
-    context.hidden = False
-    assert get_reason_if_blocked(context) == 'deleted'
-
-
 def test_get_reason_blocked_not_hidden_is_hidden(context):
     from . import get_reason_if_blocked
-    context.deleted = False
     context.hidden = True
     assert get_reason_if_blocked(context) == 'hidden'
 
 
-def test_get_reason_blocked_is_hidden_is_hidden(context):
-    from . import get_reason_if_blocked
-    context.deleted = True
-    context.hidden = True
-    assert get_reason_if_blocked(context) == 'both'
-
-
 class TestGetVisibilityChange:
 
     @fixture
@@ -207,26 +191,20 @@ def call_fut(self, event):
 
     def test_newly_hidden(self, event):
         from adhocracy_core.interfaces import VisibilityChange
-        event.old_appstruct = {'deleted': False, 'hidden': False}
-        event.new_appstruct = {'deleted': False, 'hidden': True}
+        event.old_appstruct = {'hidden': False}
+        event.new_appstruct = {'hidden': True}
         assert self.call_fut(event) == VisibilityChange.concealed
 
-    def test_newly_undeleted(self, event):
-        from adhocracy_core.interfaces import VisibilityChange
-        event.old_appstruct = {'deleted': True, 'hidden': False}
-        event.new_appstruct = {'deleted': False, 'hidden': False}
-        assert self.call_fut(event) == VisibilityChange.revealed
-
     def test_no_change_invisible(self, event):
         from adhocracy_core.interfaces import VisibilityChange
-        event.old_appstruct = {'deleted': False, 'hidden': True}
-        event.new_appstruct = {'deleted': False, 'hidden': True}
+        event.old_appstruct = {'hidden': True}
+        event.new_appstruct = {'hidden': True}
         assert self.call_fut(event) == VisibilityChange.invisible
 
     def test_no_change_visible(self, event):
         from adhocracy_core.interfaces import VisibilityChange
-        event.old_appstruct = {'deleted': False, 'hidden': False}
-        event.new_appstruct = {'deleted': False, 'hidden': False}
+        event.old_appstruct = {'hidden': False}
+        event.new_appstruct = {'hidden': False}
         assert self.call_fut(event) == VisibilityChange.visible
 
     def test_return_invisible_if_deleted_event(self, context, pool):
@@ -256,55 +234,6 @@ def test_get_modification_date_cached():
     assert result is registry.__modification_date__
 
 
-def test_is_deleted_attribute_is_true(context):
-    from . import is_deleted
-    context.deleted = True
-    assert is_deleted(context) is True
-
-
-def test_is_deleted_attribute_is_false(context):
-    from . import is_deleted
-    context.deleted = False
-    assert is_deleted(context) is False
-
-
-def test_is_deleted_attribute_not_set(context):
-    from . import is_deleted
-    assert is_deleted(context) is False
-
-
-def test_is_deleted_parent_attribute_is_true(context):
-    from . import is_deleted
-    child = testing.DummyResource()
-    context['child'] = child
-    context.deleted = True
-    assert is_deleted(child) is True
-
-
-def test_is_deleted_parent_attribute_is_false(context):
-    from . import is_deleted
-    child = testing.DummyResource()
-    context['child'] = child
-    context.deleted = False
-    assert is_deleted(child) is False
-
-
-def test_is_deleted_parent_attribute_not_set(context):
-    from . import is_deleted
-    child = testing.DummyResource()
-    context['child'] = child
-    assert is_deleted(child) is False
-
-
-def test_is_deleted_parent_attrib_true_child_attrib_false(context):
-    from . import is_deleted
-    child = testing.DummyResource()
-    context['child'] = child
-    context.deleted = True
-    child.deleted = False
-    assert is_deleted(child) is True
-
-
 def test_is_hidden_attribute_is_true(context):
     from . import is_hidden
     context.hidden = True