refactoring content_get_all and get_last_active

tracim/lib/core/content.py

@@ -343,56 +343,57 @@ class ContentApi(object):
     ) -> Query:
         return self._base_query(workspace)
 
-    def get_child_folders(self, parent: Content=None, workspace: Workspace=None, filter_by_allowed_content_types: list=[], removed_item_ids: list=[], allowed_node_types=None) -> typing.List[Content]:
-        """
-        This method returns child items (folders or items) for left bar treeview.
-
-        :param parent:
-        :param workspace:
-        :param filter_by_allowed_content_types:
-        :param removed_item_ids:
-        :param allowed_node_types: This parameter allow to hide folders for which the given type of content is not allowed.
-               For example, if you want to move a Page from a folder to another, you should show only folders that accept pages
-        :return:
-        """
-        filter_by_allowed_content_types = filter_by_allowed_content_types or []  # FDV
-        removed_item_ids = removed_item_ids or []  # FDV
-
-        if not allowed_node_types:
-            allowed_node_types = [ContentType.Folder]
-        elif allowed_node_types==ContentType.Any:
-            allowed_node_types = ContentType.all()
-
-        parent_id = parent.content_id if parent else None
-        folders = self._base_query(workspace).\
-            filter(Content.parent_id==parent_id).\
-            filter(Content.type.in_(allowed_node_types)).\
-            filter(Content.content_id.notin_(removed_item_ids)).\
-            all()
-
-        if not filter_by_allowed_content_types or \
-                        len(filter_by_allowed_content_types)<=0:
-            # Standard case for the left treeview: we want to show all contents
-            # in the left treeview... so we still filter because for example
-            # comments must not appear in the treeview
-            return [folder for folder in folders \
-                    if folder.type in ContentType.allowed_types_for_folding()]
-
-        # Now this is a case of Folders only (used for moving content)
-        # When moving a content, you must get only folders that allow to be filled
-        # with the type of content you want to move
-        result = []
-        for folder in folders:
-            for allowed_content_type in filter_by_allowed_content_types:
-
-                is_folder = folder.type == ContentType.Folder
-                content_type__allowed = folder.properties['allowed_content'][allowed_content_type] == True
-
-                if is_folder and content_type__allowed:
-                    result.append(folder)
-                    break
-
-        return result
+    # TODO - G.M - 2018-07-17 - [Cleanup] Drop this method if unneeded
+    # def get_child_folders(self, parent: Content=None, workspace: Workspace=None, filter_by_allowed_content_types: list=[], removed_item_ids: list=[], allowed_node_types=None) -> typing.List[Content]:
+    #     """
+    #     This method returns child items (folders or items) for left bar treeview.
+    # 
+    #     :param parent:
+    #     :param workspace:
+    #     :param filter_by_allowed_content_types:
+    #     :param removed_item_ids:
+    #     :param allowed_node_types: This parameter allow to hide folders for which the given type of content is not allowed.
+    #            For example, if you want to move a Page from a folder to another, you should show only folders that accept pages
+    #     :return:
+    #     """
+    #     filter_by_allowed_content_types = filter_by_allowed_content_types or []  # FDV
+    #     removed_item_ids = removed_item_ids or []  # FDV
+    # 
+    #     if not allowed_node_types:
+    #         allowed_node_types = [ContentType.Folder]
+    #     elif allowed_node_types==ContentType.Any:
+    #         allowed_node_types = ContentType.all()
+    # 
+    #     parent_id = parent.content_id if parent else None
+    #     folders = self._base_query(workspace).\
+    #         filter(Content.parent_id==parent_id).\
+    #         filter(Content.type.in_(allowed_node_types)).\
+    #         filter(Content.content_id.notin_(removed_item_ids)).\
+    #         all()
+    # 
+    #     if not filter_by_allowed_content_types or \
+    #                     len(filter_by_allowed_content_types)<=0:
+    #         # Standard case for the left treeview: we want to show all contents
+    #         # in the left treeview... so we still filter because for example
+    #         # comments must not appear in the treeview
+    #         return [folder for folder in folders \
+    #                 if folder.type in ContentType.allowed_types_for_folding()]
+    # 
+    #     # Now this is a case of Folders only (used for moving content)
+    #     # When moving a content, you must get only folders that allow to be filled
+    #     # with the type of content you want to move
+    #     result = []
+    #     for folder in folders:
+    #         for allowed_content_type in filter_by_allowed_content_types:
+    # 
+    #             is_folder = folder.type == ContentType.Folder
+    #             content_type__allowed = folder.properties['allowed_content'][allowed_content_type] == True
+    # 
+    #             if is_folder and content_type__allowed:
+    #                 result.append(folder)
+    #                 break
+    # 
+    #     return result
 
     def create(self, content_type: str, workspace: Workspace, parent: Content=None, label: str ='', filename: str = '', do_save=False, is_temporary: bool=False, do_notify=True) -> Content:
         # TODO - G.M - 2018-07-16 - raise Exception instead of assert
@@ -724,10 +725,21 @@ class ContentApi(object):
             ),
         ))
 
-    def get_all(self, parent_id: int=None, content_type: str=ContentType.Any, workspace: Workspace=None) -> typing.List[Content]:
-        assert parent_id is None or isinstance(parent_id, int) # DYN_REMOVE
-        if not content_type:
-            content_type = ContentType.Any
+    def _get_all_query(
+        self,
+        parent_id: int = None,
+        content_type: str = ContentType.Any,
+        workspace: Workspace = None
+    ) -> Query:
+        """
+        Extended filter for better "get all data" query
+        :param parent_id: filter by parent_id
+        :param content_type: filter by content_type slug
+        :param workspace: filter by workspace
+        :return:
+        """
+        assert parent_id is None or isinstance(parent_id, int)
+        assert content_type is not None
         resultset = self._base_query(workspace)
 
         if content_type!=ContentType.Any:
@@ -740,28 +752,32 @@ class ContentApi(object):
             resultset = resultset.filter(Content.parent_id==parent_id)
         if parent_id == 0 or parent_id is False:
             resultset = resultset.filter(Content.parent_id == None)
-        # parent_id == None give all contents
-
-        return resultset.all()
-
-    def get_children(self, parent_id: int, content_types: list, workspace: Workspace=None) -> typing.List[Content]:
-        """
-        Return parent_id childs of given content_types
-        :param parent_id: parent id
-        :param content_types: list of types
-        :param workspace: workspace filter
-        :return: list of content
-        """
-        resultset = self._base_query(workspace)
-        resultset = resultset.filter(Content.type.in_(content_types))
 
-        if parent_id:
-            resultset = resultset.filter(Content.parent_id==parent_id)
-        if parent_id is False:
-            resultset = resultset.filter(Content.parent_id == None)
+        return resultset
 
-        return resultset.all()
+    def get_all(self, parent_id: int=None, content_type: str=ContentType.Any, workspace: Workspace=None) -> typing.List[Content]:
+        return self._get_all_query(parent_id, content_type, workspace).all()
 
+    # TODO - G.M - 2018-07-17 - [Cleanup] Drop this method if unneeded
+    # def get_children(self, parent_id: int, content_types: list, workspace: Workspace=None) -> typing.List[Content]:
+    #     """
+    #     Return parent_id childs of given content_types
+    #     :param parent_id: parent id
+    #     :param content_types: list of types
+    #     :param workspace: workspace filter
+    #     :return: list of content
+    #     """
+    #     resultset = self._base_query(workspace)
+    #     resultset = resultset.filter(Content.type.in_(content_types))
+    #
+    #     if parent_id:
+    #         resultset = resultset.filter(Content.parent_id==parent_id)
+    #     if parent_id is False:
+    #         resultset = resultset.filter(Content.parent_id == None)
+    #
+    #     return resultset.all()
+
+    # TODO - G.M - 2018-07-17 - [Cleanup] Drop this method if unneeded
     # TODO find an other name to filter on is_deleted / is_archived
     def get_all_with_filter(self, parent_id: int=None, content_type: str=ContentType.Any, workspace: Workspace=None) -> typing.List[Content]:
         assert parent_id is None or isinstance(parent_id, int) # DYN_REMOVE
@@ -781,33 +797,34 @@ class ContentApi(object):
 
         return resultset.all()
 
-    def get_all_without_exception(self, content_type: str, workspace: Workspace=None) -> typing.List[Content]:
-        assert content_type is not None# DYN_REMOVE
-
-        resultset = self._base_query(workspace)
-
-        if content_type != ContentType.Any:
-            resultset = resultset.filter(Content.type==content_type)
-
-        return resultset.all()
-
-    def get_last_active(self, parent_id: typing.Optional[int], content_type: str, workspace: Workspace=None, limit=10, offset=1) -> typing.List[Content]:
-        assert parent_id is None or isinstance(parent_id, int) # DYN_REMOVE
-        assert content_type is not None# DYN_REMOVE
-        assert isinstance(content_type, str) # DYN_REMOVE
-
-        resultset = self._base_query(workspace) \
-            .filter(Content.workspace_id == Workspace.workspace_id) \
-            .filter(Workspace.is_deleted.is_(False)) \
-            .order_by(desc(Content.updated))
-
-        if content_type!=ContentType.Any:
-            resultset = resultset.filter(Content.type==content_type)
-
-        if parent_id:
-            resultset = resultset.filter(Content.parent_id==parent_id)
+    # TODO - G.M - 2018-07-17 - [Cleanup] Drop this method if unneeded
+    # def get_all_without_exception(self, content_type: str, workspace: Workspace=None) -> typing.List[Content]:
+    #     assert content_type is not None# DYN_REMOVE
+    #
+    #     resultset = self._base_query(workspace)
+    #
+    #     if content_type != ContentType.Any:
+    #         resultset = resultset.filter(Content.type==content_type)
+    #
+    #     return resultset.all()
+
+    def get_last_active(
+            self,
+            parent_id: typing.Optional[int],
+            content_type: str,
+            workspace: Workspace=None,
+            limit: typing.Optional[int]=None,
+            offset: typing.Optional[int]= None,
+    ) -> typing.List[Content]:
 
+        resultset = self._get_all_query(
+            parent_id=parent_id,
+            content_type=content_type,
+            workspace=workspace,
+        )
+        resultset = resultset.order_by(desc(Content.updated))
         resultset = resultset.slice(start=offset, stop=limit)
+
         # result = []
         # for item in resultset:
         #     new_item = None

tracim/models/context_models.py

@@ -71,24 +71,14 @@ class ContentFilter(object):
             show_active: int = 1,
             content_type: str = None,
             offset: int = None,
-            nb_elem: int = None,
+            limit: int = None,
     ) -> None:
-        # 0 is same as None here
-        if parent_id == 0:
-            parent_id = None
-        if workspace_id == 0:
-            workspace_id = None
-        if offset == 0:
-            offset = None
-        if nb_elem == 0:
-            nb_elem = None
-
         self.parent_id = parent_id
         self.workspace_id = workspace_id
         self.show_archived = bool(show_archived)
         self.show_deleted = bool(show_deleted)
         self.show_active = bool(show_active)
-        self.nb_elem = nb_elem
+        self.limit = limit
         self.offset = offset
         self.content_type = content_type
 

tracim/views/core_api/schemas.py

@@ -175,11 +175,11 @@ class FilterContentQuerySchema(marshmallow.Schema):
 
 
 class PaginationSchema(marshmallow.Schema):
-    nb_elem = marshmallow.fields.Int(
+    limit = marshmallow.fields.Int(
         example=2,
         default=0,
         description='if 0 or not set, return all elements, else return only '
-                    'the first nb_elem (according to offset)',
+                    'the first limit elem (according to offset)',
         validate=Range(min=0, error="Value must be positive or 0"),
     )
     offset = marshmallow.fields.Int(

tracim/views/core_api/user_controller.py

@@ -56,10 +56,14 @@ class UserController(Controller):
         Get last_active_content for user
         """
         app_config = request.registry.settings['CFG']
+        content_filter = hapic_data.query
         api = ContentApi(
             current_user=request.current_user,  # User
             session=request.dbsession,
             config=app_config,
+            show_archived=content_filter.show_archived,
+            show_deleted=content_filter.show_deleted,
+            show_active=content_filter.show_active,
         )
         wapi = WorkspaceApi(
             current_user=request.current_user,  # User
@@ -67,14 +71,14 @@ class UserController(Controller):
             config=app_config,
         )
         workspace = None
-        if 'workspace_id' in hapic_data.body:
-            workspace = wapi.get_one(hapic_data.body.workspace_id)
+        if content_filter.workspace_id:
+            workspace = wapi.get_one(content_filter.workspace_id)
         last_actives = api.get_last_active(
-            parent_id=hapic_data.body.parent_id,
-            content_type=hapic_data.body.content_type,
+            parent_id=content_filter.parent_id,
+            content_type=content_filter.content_type or ContentType.Any,
             workspace=workspace,
-            offset=None,
-            limit=None,
+            offset=content_filter.offset or None,
+            limit=content_filter.limit or None,
         )
         return [
             api.get_content_in_context(content) for content in last_actives

tracim/views/core_api/workspace_controller.py

@@ -107,7 +107,7 @@ class WorkspaceController(Controller):
         contents = api.get_all(
             parent_id=content_filter.parent_id,
             workspace=request.current_workspace,
-            content_type=content_filter.content_type,
+            content_type=content_filter.content_type or ContentType.Any,
         )
         contents = [
             api.get_content_in_context(content) for content in contents