Better support for move endpoint : allow to move content from one workspace to another

tracim/exceptions.py

@@ -65,6 +65,10 @@ class WorkspaceNotFound(NotFound):
     pass
 
 
+class WorkspaceNotFoundInTracimRequest(NotFound):
+    pass
+
+
 class InsufficientUserWorkspaceRole(TracimException):
     pass
 
@@ -111,3 +115,7 @@ class UserDoesNotExist(TracimException):
 
 class UserNotFoundInTracimRequest(TracimException):
     pass
+
+
+class NotSameWorkspace(TracimException):
+    pass

tracim/lib/core/content.py

@@ -25,6 +25,7 @@ from sqlalchemy.sql.elements import and_
 from tracim.lib.utils.utils import cmp_to_key
 from tracim.lib.core.notifications import NotifierFactory
 from tracim.exceptions import SameValueError
+from tracim.exceptions import NotSameWorkspace
 from tracim.lib.utils.utils import current_date_for_filename
 from tracim.models.revision_protection import new_revision
 from tracim.models.auth import User
@@ -862,19 +863,25 @@ class ContentApi(object):
         else:
             raise ValueError('The given value {} is not allowed'.format(new_status))
 
-    def move(self, item: Content,
+    def move(self,
+             item: Content,
              new_parent: Content,
-             must_stay_in_same_workspace:bool=True,
-             new_workspace:Workspace=None):
+             must_stay_in_same_workspace: bool=True,
+             new_workspace: Workspace=None,
+    ):
         if must_stay_in_same_workspace:
             if new_parent and new_parent.workspace_id != item.workspace_id:
                 raise ValueError('the item should stay in the same workspace')
 
         item.parent = new_parent
-        if new_parent:
-            item.workspace = new_parent.workspace
-        elif new_workspace:
+        if new_workspace:
             item.workspace = new_workspace
+            if new_parent.workspace_id != new_workspace.workspace_id:
+                raise NotSameWorkspace(
+                    'new parent workspace and new workspace should be the same.'
+                )
+        else:
+            item.workspace = new_parent.workspace
 
         item.revision_type = ActionDescription.MOVE
 

tracim/lib/utils/authorization.py

@@ -9,7 +9,7 @@ except ImportError:  # python3.4
     JSONDecodeError = ValueError
 
 from tracim.exceptions import InsufficientUserWorkspaceRole, \
-    InsufficientUserProfile
+    InsufficientUserProfile, WorkspaceNotFoundInTracimRequest
 
 if TYPE_CHECKING:
     from tracim import TracimRequest
@@ -101,3 +101,28 @@ def require_workspace_role(minimal_required_role: int):
 
         return wrapper
     return decorator
+
+
+def require_candidate_workspace_role(minimal_required_role: int):
+    """
+    Decorator for view to restrict access of tracim request if role
+    is not high enough. Do nothing is candidate_workspace_role is not found.
+    :param minimal_required_role: value from UserInWorkspace Object like
+    UserRoleInWorkspace.CONTRIBUTOR or UserRoleInWorkspace.READER
+    :return: decorator
+    """
+    def decorator(func):
+
+        def wrapper(self, context, request: 'TracimRequest'):
+            user = request.current_user
+            try:
+                workspace = request.candidate_workspace
+            except WorkspaceNotFoundInTracimRequest:
+                return func(self, context, request)
+
+            if workspace.get_user_role(user) >= minimal_required_role:
+                return func(self, context, request)
+            raise InsufficientUserWorkspaceRole()
+
+        return wrapper
+    return decorator

tracim/lib/utils/request.py

@@ -2,7 +2,7 @@
 from pyramid.request import Request
 from sqlalchemy.orm.exc import NoResultFound
 
-from tracim.exceptions import NotAuthenticated
+from tracim.exceptions import NotAuthenticated, WorkspaceNotFoundInTracimRequest
 from tracim.exceptions import UserNotFoundInTracimRequest
 from tracim.exceptions import UserDoesNotExist
 from tracim.exceptions import WorkspaceNotFound
@@ -34,9 +34,12 @@ class TracimRequest(Request):
             decode_param_names,
             **kw
         )
-        # Current workspace, found by request headers or content
+        # Current workspace, found in request path
         self._current_workspace = None  # type: Workspace
 
+        # Candidate workspace found in request body
+        self._candidate_workspace = None  # type: Workspace
+
         # Authenticated user
         self._current_user = None  # type: User
 
@@ -56,7 +59,7 @@ class TracimRequest(Request):
         :return: Workspace of the request
         """
         if self._current_workspace is None:
-            self.current_workspace = self._get_workspace(self.current_user, self)
+            self.current_workspace = self._get_current_workspace(self.current_user, self)
         return self._current_workspace
 
     @current_workspace.setter
@@ -102,6 +105,21 @@ class TracimRequest(Request):
             self.candidate_user = self._get_candidate_user(self)
         return self._candidate_user
 
+    @property
+    def candidate_workspace(self) -> Workspace:
+        """
+        Get user from headers/body request. This user is not
+        the one found by authentication mecanism. This user
+        can help user to know about who one page is about in
+        a similar way as current_workspace.
+        """
+        if self._candidate_workspace is None:
+            self._candidate_workspace = self._get_candidate_workspace(
+                self.current_user,
+                self
+            )
+        return self._candidate_workspace
+
     def _cleanup(self, request: 'TracimRequest') -> None:
         """
         Close dbsession at the end of the request in order to avoid exception
@@ -171,7 +189,7 @@ class TracimRequest(Request):
             raise NotAuthenticated('User {} not found'.format(login)) from exc
         return user
 
-    def _get_workspace(
+    def _get_current_workspace(
             self,
             user: User,
             request: 'TracimRequest'
@@ -187,7 +205,39 @@ class TracimRequest(Request):
             if 'workspace_id' in request.matchdict:
                 workspace_id = request.matchdict['workspace_id']
             if not workspace_id:
-                raise WorkspaceNotFound('No workspace_id property found in request')
+                raise WorkspaceNotFoundInTracimRequest('No workspace_id property found in request')
+            wapi = WorkspaceApi(
+                current_user=user,
+                session=request.dbsession,
+                config=request.registry.settings['CFG']
+            )
+            workspace = wapi.get_one(workspace_id)
+        except JSONDecodeError:
+            raise WorkspaceNotFound('Bad json body')
+        except NoResultFound:
+            raise WorkspaceNotFound(
+                'Workspace {} does not exist '
+                'or is not visible for this user'.format(workspace_id)
+            )
+        return workspace
+
+    def _get_candidate_workspace(
+            self,
+            user: User,
+            request: 'TracimRequest'
+    ) -> Workspace:
+        """
+        Get current workspace from request
+        :param user: User who want to check the workspace
+        :param request: pyramid request
+        :return: current workspace
+        """
+        workspace_id = ''
+        try:
+            if 'new_workspace_id' in request.json_body:
+                workspace_id = request.json_body['new_workspace_id']
+            if not workspace_id:
+                raise WorkspaceNotFoundInTracimRequest('No new_workspace_id property found in body')
             wapi = WorkspaceApi(
                 current_user=user,
                 session=request.dbsession,

tracim/lib/utils/ttest

@@ -0,0 +1,341 @@
+
+definitions:
+  CommentSchema:
+    properties:
+      content_id:
+        example: 6
+        format: int32
+        type: integer
+      parent_id:
+        example: 34
+        format: int32
+        type: integer
+        x-nullable: true
+      content:
+        type: string
+        example: "Coucou !"
+      author:
+        $ref: '#/definitions/UserDigestSchema'
+  UserDigestSchema:
+    properties:
+      user_id:
+        example: 3
+        format: int32
+        readOnly: true
+        type: integer
+      avatar_url:
+        description: avatar_url is the url to the image file. If no avatar, then set
+          it to null (and frontend will interpret this with a default avatar)
+        example: /api/v2/assets/avatars/suri-cate.jpg
+        format: url
+        type: string
+        x-nullable: true
+      public_name:
+        example: Suri Cate
+        type: string
+  HtmlPageContentSchema:
+    properties:
+      content_type:
+        enum:
+        - thread
+        - file
+        - markdownpage
+        - page
+        - folder
+        example: htmlpage
+        type: string
+      content_id:
+        example: 6
+        format: int32
+        type: integer
+      is_archived:
+        example: false
+        type: boolean
+      is_deleted:
+        example: false
+        type: boolean
+      label:
+        example: Intervention Report 12
+        type: string
+      parent_id:
+        example: 34
+        format: int32
+        type: integer
+        x-nullable: true
+      show_in_ui:
+        description: if false, then do not show content in the treeview. This may
+          his maybe used for specific contents or for sub-contents. Default is True.
+          In first version of the API, this field is always True
+        example: true
+        type: boolean
+      slug:
+        example: intervention-report-12
+        type: string
+      status_slug:
+        description: this slug is found in content_type available statuses
+        enum:
+        - open
+        - closed-validated
+        - closed-unvalidated
+        - closed-deprecated
+        example: closed-deprecated
+        type: string
+      sub_content_types:
+        description: list of content types allowed as sub contents. This field is
+          required for folder contents, set it to empty list in other cases
+        items:
+          type: string
+        type: array
+      workspace_id:
+        example: 19
+        format: int32
+        type: integer
+      current_revision_id:
+        type: integer
+        example: 74
+      created:
+        format: date-time
+        type: string
+      author:
+        $ref: '#/definitions/UserDigestSchema'
+      modified:
+        format: date-time
+        type: string
+      last_modifier:
+        $ref: '#/definitions/UserDigestSchema'
+      content:
+        example: '<p> Coucou </p>'
+        type: string
+    type: object
+  HtmlPageRevisionSchema:
+    properties:
+      content_type:
+        enum:
+        - thread
+        - file
+        - markdownpage
+        - page
+        - folder
+        example: htmlpage
+        type: string
+      content_id:
+        example: 6
+        format: int32
+        type: integer
+      is_archived:
+        example: false
+        type: boolean
+      is_deleted:
+        example: false
+        type: boolean
+      label:
+        example: Intervention Report 12
+        type: string
+      parent_id:
+        example: 34
+        format: int32
+        type: integer
+        x-nullable: true
+      show_in_ui:
+        description: if false, then do not show content in the treeview. This may
+          his maybe used for specific contents or for sub-contents. Default is True.
+          In first version of the API, this field is always True
+        example: true
+        type: boolean
+      slug:
+        example: intervention-report-12
+        type: string
+      status_slug:
+        description: this slug is found in content_type available statuses
+        enum:
+        - open
+        - closed-validated
+        - closed-unvalidated
+        - closed-deprecated
+        example: closed-deprecated
+        type: string
+      sub_content_types:
+        description: list of content types allowed as sub contents. This field is
+          required for folder contents, set it to empty list in other cases
+        items:
+          type: string
+        type: array
+      workspace_id:
+        example: 19
+        format: int32
+        type: integer
+      revision_id:
+        type: integer
+        example: 74
+      created:
+        format: date-time
+        type: string
+      author:
+        $ref: '#/definitions/UserDigestSchema'
+      content:
+        example: '<p> Coucou </p>'
+        type: string
+    type: object
+  HtmlPageRevisionListSchema:
+    properties:
+      revisions:
+        type: array
+        items:
+          $ref: '#/definitions/HtmlPageRevisionSchema'
+      revision_nb:
+        type: integer
+        example: 40
+  HtmlPageModifySchema:
+    type: object
+    properties:
+      label:
+        example: "My Page"
+        type: string
+      content:
+        example: '<p> Coucou </p>'
+        type: string
+  ContentSetStatusSchema:
+    type: object
+    properties:
+      status:
+        example: "open-workinprogress"
+        type: string
+  NoContentSchema:
+    type: object
+info:
+  description: API of Tracim v2
+  title: Tracim v2 API
+  version: 1.0.0
+parameters: {}
+paths:
+  "/api/v2/workspaces/{workspace_id}/htmlpages/{htmlpage_id}":
+    get:
+      description: "get htmlpage content"
+      parameters:
+        - name: "workspace_id"
+          in: path
+          required: true
+          type: integer
+          description: id of the current workspace.
+        - name: "htmlpage_id"
+          in: path
+          required: true
+          type: integer
+          description: content id of htmlpage.
+      responses:
+        '200':
+          description: "nominal case"
+          schema:
+              $ref: '#/definitions/HtmlPageContentSchema'
+    put:
+      description: "modify htmlpage label or/and content"
+      parameters:
+        - in: body
+          name: "body"
+          schema:
+            $ref: '#/definitions/HtmlPageModifySchema'
+        - name: "workspace_id"
+          in: path
+          required: true
+          type: integer
+          description: id of the current workspace.
+        - name: "htmlpage_id"
+          in: path
+          required: true
+          type: integer
+          description: content id of htmlpage.
+      responses:
+        '200':
+          description: "nominal case"
+          schema:
+            $ref: '#/definitions/HtmlPageContentSchema'
+  "/api/v2/workspaces/{workspace_id}/htmlpages/{htmlpage_id}/revisions":
+    get:
+      description: "gets all htmlpages revisions (sorted by"
+      parameters:
+        - name: "workspace_id"
+          in: path
+          required: true
+          type: integer
+          description: id of the current workspace.
+        - name: "htmlpage_id"
+          in: path
+          required: true
+          type: integer
+          description: content id of htmlpage.
+      responses:
+        '200':
+          description: "nominal case"
+          schema:
+            $ref: '#/definitions/HtmlPageRevisionListSchema'
+  "/api/v2/workspaces/{workspace_id}/htmlpages/{htmlpage_id}/status":
+    put:
+      description: "set htmlpage content status"
+      parameters:
+        - in: body
+          name: "body"
+          schema:
+            $ref: '#/definitions/ContentSetStatusSchema'
+        - name: "workspace_id"
+          in: path
+          required: true
+          type: integer
+          description: id of the current workspace.
+        - name: "htmlpage_id"
+          in: path
+          required: true
+          type: integer
+          description: content id of htmlpage.
+      responses:
+        '200':
+          description: "nominal case"
+          schema:
+            $ref: '#/definitions/NoContentSchema'
+  "/api/v2/workspaces/{workspace_id}/contents/{content_id}/comments":
+    get:
+      description: "get all comments related to a content"
+      parameters:
+        - name: "workspace_id"
+          in: path
+          required: true
+          type: integer
+          description: id of the current workspace.
+        - name: "content_id"
+          in: path
+          required: true
+          type: integer
+          description: content id.
+      responses:
+        '200':
+          description: "nominal case"
+          schema:
+            type: array
+            items:
+              $ref: '#/definitions/CommentSchema'
+
+  "/api/v2/workspaces/{workspace_id}/contents/{content_id}/comments/{comments_id}":
+    delete:
+      description: "delete one comment"
+      parameters:
+        - name: "workspace_id"
+          in: path
+          required: true
+          type: integer
+          description: id of the current workspace.
+        - name: "content_id"
+          in: path
+          required: true
+          type: integer
+          description: content id.
+        - name: "comments_id"
+          in: path
+          required: true
+          type: integer
+          description: id of a comment related to content content_id.
+      responses:
+        '204':
+          description: "nominal case"
+          schema:
+            $ref: '#/definitions/NoContentSchema'
+swagger: '2.0'
+tags: []

tracim/models/context_models.py

@@ -17,8 +17,9 @@ class MoveParams(object):
     """
     Json body params for move action
     """
-    def __init__(self, new_parent_id: str):
+    def __init__(self, new_parent_id: str, new_workspace_id: str = None):
         self.new_parent_id = new_parent_id
+        self.new_workspace_id = new_workspace_id
 
 
 class LoginCredentials(object):

tracim/tests/functional/test_workspaces.py

@@ -814,6 +814,132 @@ class TestWorkspaceContents(FunctionalTest):
         assert not [content for content in new_folder1_contents if content['id'] == 8]  # nopep8
         assert [content for content in new_folder2_contents if content['id'] == 8]  # nopep8
 
+    def test_api_put_move_content__ok_200__with_workspace_id(self):
+        """
+        Move content
+        move Apple_Pie (content_id: 8)
+        from Desserts folder(content_id: 3) to Salads subfolder (content_id: 4)
+        of workspace Recipes.
+        """
+        self.testapp.authorization = (
+            'Basic',
+            (
+                'admin@admin.admin',
+                'admin@admin.admin'
+            )
+        )
+        params = {
+            'new_parent_id': '4',  # Salads
+            'new_workspace_id': '2',
+        }
+        params_folder1 = {
+            'parent_id': 3,
+            'show_archived': 0,
+            'show_deleted': 0,
+            'show_active': 1,
+        }
+        params_folder2 = {
+            'parent_id': 4,
+            'show_archived': 0,
+            'show_deleted': 0,
+            'show_active': 1,
+        }
+        folder1_contents = self.testapp.get('/api/v2/workspaces/2/contents', params=params_folder1, status=200).json_body  # nopep8
+        folder2_contents = self.testapp.get('/api/v2/workspaces/2/contents', params=params_folder2, status=200).json_body  # nopep8
+        assert [content for content in folder1_contents if content['id'] == 8]  # nopep8
+        assert not [content for content in folder2_contents if content['id'] == 8]  # nopep8
+        # TODO - G.M - 2018-06-163 - Check content
+        res = self.testapp.put_json(
+            '/api/v2/workspaces/2/contents/8/move',
+            params=params,
+            status=200
+        )
+        new_folder1_contents = self.testapp.get('/api/v2/workspaces/2/contents', params=params_folder1, status=200).json_body  # nopep8
+        new_folder2_contents = self.testapp.get('/api/v2/workspaces/2/contents', params=params_folder2, status=200).json_body  # nopep8
+        assert not [content for content in new_folder1_contents if content['id'] == 8]  # nopep8
+        assert [content for content in new_folder2_contents if content['id'] == 8]  # nopep8
+
+    def test_api_put_move_content__ok_200__to_another_workspace(self):
+        """
+        Move content
+        move Apple_Pie (content_id: 8)
+        from Desserts folder(content_id: 3) to Menus subfolder (content_id: 2)
+        of workspace Business.
+        """
+        self.testapp.authorization = (
+            'Basic',
+            (
+                'admin@admin.admin',
+                'admin@admin.admin'
+            )
+        )
+        params = {
+            'new_parent_id': '2',  # Menus
+        }
+        params_folder1 = {
+            'parent_id': 3,
+            'show_archived': 0,
+            'show_deleted': 0,
+            'show_active': 1,
+        }
+        params_folder2 = {
+            'parent_id': 2,
+            'show_archived': 0,
+            'show_deleted': 0,
+            'show_active': 1,
+        }
+        folder1_contents = self.testapp.get('/api/v2/workspaces/2/contents', params=params_folder1, status=200).json_body  # nopep8
+        folder2_contents = self.testapp.get('/api/v2/workspaces/1/contents', params=params_folder2, status=200).json_body  # nopep8
+        assert [content for content in folder1_contents if content['id'] == 8]  # nopep8
+        assert not [content for content in folder2_contents if content['id'] == 8]  # nopep8
+        # TODO - G.M - 2018-06-163 - Check content
+        res = self.testapp.put_json(
+            '/api/v2/workspaces/2/contents/8/move',
+            params=params,
+            status=200
+        )
+        new_folder1_contents = self.testapp.get('/api/v2/workspaces/2/contents', params=params_folder1, status=200).json_body  # nopep8
+        new_folder2_contents = self.testapp.get('/api/v2/workspaces/1/contents', params=params_folder2, status=200).json_body  # nopep8
+        assert not [content for content in new_folder1_contents if content['id'] == 8]  # nopep8
+        assert [content for content in new_folder2_contents if content['id'] == 8]  # nopep8
+
+    def test_api_put_move_content__err_400__wrong_workspace_id(self):
+        """
+        Move content
+        move Apple_Pie (content_id: 8)
+        from Desserts folder(content_id: 3) to Salads subfolder (content_id: 4)
+        of workspace Recipes.
+        Workspace_id of parent_id don't match with workspace_id of workspace
+        """
+        self.testapp.authorization = (
+            'Basic',
+            (
+                'admin@admin.admin',
+                'admin@admin.admin'
+            )
+        )
+        params = {
+            'new_parent_id': '4',  # Salads
+            'new_workspace_id': '1',
+        }
+        params_folder1 = {
+            'parent_id': 3,
+            'show_archived': 0,
+            'show_deleted': 0,
+            'show_active': 1,
+        }
+        params_folder2 = {
+            'parent_id': 4,
+            'show_archived': 0,
+            'show_deleted': 0,
+            'show_active': 1,
+        }
+        res = self.testapp.put_json(
+            '/api/v2/workspaces/2/contents/8/move',
+            params=params,
+            status=400,
+        )
+
     def test_api_put_delete_content__ok_200__nominal_case(self):
         """
         delete content

tracim/views/core_api/schemas.py

@@ -300,6 +300,12 @@ class ContentMoveSchema(marshmallow.Schema):
         example=42,
         description='id of the new parent content id.'
     )
+    new_workspace_id = marshmallow.fields.Int(
+        example=2,
+        description='id of the new workspace id.',
+        allow_none=True,
+        default=None,
+    )
 
     @post_load
     def make_move_params(self, data):

tracim/views/core_api/workspace_controller.py

@@ -1,5 +1,4 @@
 import typing
-
 import transaction
 from pyramid.config import Configurator
 try:  # Python 3.5+
@@ -7,15 +6,20 @@ try:  # Python 3.5+
 except ImportError:
     from http import client as HTTPStatus
 
-from tracim import hapic, TracimRequest
+from tracim import hapic
+from tracim import TracimRequest
 from tracim.lib.core.workspace import WorkspaceApi
 from tracim.lib.core.content import ContentApi
 from tracim.lib.core.userworkspace import RoleApi
-from tracim.lib.utils.authorization import require_workspace_role
-from tracim.models.data import UserRoleInWorkspace, ActionDescription
+from tracim.lib.utils.authorization import require_workspace_role, \
+    require_candidate_workspace_role
+from tracim.models.data import UserRoleInWorkspace
+from tracim.models.data import ActionDescription
 from tracim.models.context_models import UserRoleWorkspaceInContext
 from tracim.models.context_models import ContentInContext
-from tracim.exceptions import NotAuthenticated
+from tracim.exceptions import NotAuthenticated, InsufficientUserWorkspaceRole
+from tracim.exceptions import WorkspaceNotFoundInTracimRequest
+from tracim.exceptions import NotSameWorkspace
 from tracim.exceptions import InsufficientUserProfile
 from tracim.exceptions import WorkspaceNotFound
 from tracim.views.controllers import Controller
@@ -156,7 +160,10 @@ class WorkspaceController(Controller):
     @hapic.handle_exception(NotAuthenticated, HTTPStatus.UNAUTHORIZED)
     @hapic.handle_exception(InsufficientUserProfile, HTTPStatus.FORBIDDEN)
     @hapic.handle_exception(WorkspaceNotFound, HTTPStatus.FORBIDDEN)
+    @hapic.handle_exception(InsufficientUserWorkspaceRole, HTTPStatus.FORBIDDEN)
+    @hapic.handle_exception(NotSameWorkspace, HTTPStatus.BAD_REQUEST)
     @require_workspace_role(UserRoleInWorkspace.CONTRIBUTOR)
+    @require_candidate_workspace_role(UserRoleInWorkspace.CONTRIBUTOR)
     @hapic.input_path(WorkspaceAndContentIdPathSchema())
     @hapic.input_body(ContentMoveSchema())
     @hapic.output_body(NoContentSchema())
@@ -172,6 +179,7 @@ class WorkspaceController(Controller):
         app_config = request.registry.settings['CFG']
         path_data = hapic_data.path
         move_data = hapic_data.body
+
         api = ContentApi(
             current_user=request.current_user,
             session=request.dbsession,
@@ -184,12 +192,23 @@ class WorkspaceController(Controller):
         new_parent = api.get_one(
             move_data.new_parent_id, content_type=ContentType.Any
         )
+
+        try:
+            new_workspace = request.candidate_workspace
+        except WorkspaceNotFoundInTracimRequest:
+            new_workspace = None
+
         with new_revision(
                 session=request.dbsession,
                 tm=transaction.manager,
                 content=content
         ):
-            api.move(content, new_parent=new_parent)
+            api.move(
+                content,
+                new_parent=new_parent,
+                new_workspace=new_workspace,
+                must_stay_in_same_workspace=False,
+            )
         return
 
     @hapic.with_api_doc()