Merge pull request #346 from tracim/fix/302/role_api_notification_parameter

tracim/tracim/controllers/admin/workspace.py

@@ -82,17 +82,28 @@ class RoleInWorkspaceRestController(TIMRestController, BaseController):
     def undelete(self, user_id, old_role):
         user_id = int(user_id)
         role_id = int(old_role)
-        self._add_user_with_role(user_id, role_id, None, _('User {} restored in workspace {} as {}'))
+        self._add_user_with_role(user_id, role_id, False, _('User {} restored in workspace {} as {}'))
         tg.redirect(self.parent_controller.url(tg.tmpl_context.workspace_id))
 
     @tg.expose()
-    def post(self, user_id, role_id, with_notif=False):
+    def post(self, user_id: str, role_id: str, with_notif: str='off'):
         user_id = int(user_id)
         role_id = int(role_id)
-        self._add_user_with_role(user_id, role_id, with_notif, _('User {} added to workspace {} as {}'))
+        self._add_user_with_role(
+            user_id,
+            role_id,
+            on_off_to_boolean(with_notif),
+            _('User {} added to workspace {} as {}')
+        )
         tg.redirect(self.parent_controller.url(tg.tmpl_context.workspace_id))
 
-    def _add_user_with_role(self, user_id: int, role_id: int, with_notif: bool, flash_msg_template)-> UserRoleInWorkspace:
+    def _add_user_with_role(
+            self,
+            user_id: int,
+            role_id: int,
+            with_notif: bool,
+            flash_msg_template: str
+    )-> UserRoleInWorkspace:
         user_api = UserApi(tg.tmpl_context.current_user)
         user = user_api.get_one(user_id)
 

tracim/tracim/lib/userworkspace.py

@@ -86,16 +86,12 @@ class RoleApi(object):
     def get_one(self, user_id, workspace_id):
         return self._get_one_rsc(user_id, workspace_id).one()
 
-    # TODO - A.P - fix with_notif: bool vs with_notif: str
-    # https://github.com/tracim/tracim/issues/302
     def create_one(self, user: User, workspace: Workspace, role_level: int, with_notif: bool, flush: bool=True) -> UserRoleInWorkspace:
         role = self.create_role()
         role.user_id = user.user_id
         role.workspace = workspace
         role.role = role_level
-        if with_notif is not None:
-            from tracim.lib.helpers import on_off_to_boolean
-            role.do_notify = on_off_to_boolean(with_notif)
+        role.do_notify = with_notif
         if flush:
             DBSession.flush()
         return role

tracim/tracim/tests/library/test_workspace.py

@@ -39,7 +39,7 @@ class TestThread(BaseTestThread, TestStandard):
         u = uapi.create_user(email='u.u@u.u', save_now=True)
         eq_([], wapi.get_notifiable_roles(workspace=w))
         rapi = RoleApi(u)
-        r = rapi.create_one(u, w, UserRoleInWorkspace.READER, with_notif='on')
+        r = rapi.create_one(u, w, UserRoleInWorkspace.READER, with_notif=True)
         eq_([r, ], wapi.get_notifiable_roles(workspace=w))
         u.is_active = False
         eq_([], wapi.get_notifiable_roles(workspace=w))
@@ -62,14 +62,13 @@ class TestThread(BaseTestThread, TestStandard):
         u = uapi.create_user('u.s@e.r', [gapi.get_one(Group.TIM_USER)], True)
         wapi = WorkspaceApi(current_user=u)
         rapi = RoleApi(current_user=u)
-        off = 'off'
-        rapi.create_one(u, w4, UserRoleInWorkspace.READER, off)
-        rapi.create_one(u, w3, UserRoleInWorkspace.CONTRIBUTOR, off)
-        rapi.create_one(u, w2, UserRoleInWorkspace.CONTENT_MANAGER, off)
-        rapi.create_one(u, w1, UserRoleInWorkspace.WORKSPACE_MANAGER, off)
+        rapi.create_one(u, w4, UserRoleInWorkspace.READER, False)
+        rapi.create_one(u, w3, UserRoleInWorkspace.CONTRIBUTOR, False)
+        rapi.create_one(u, w2, UserRoleInWorkspace.CONTENT_MANAGER, False)
+        rapi.create_one(u, w1, UserRoleInWorkspace.WORKSPACE_MANAGER, False)
         eq_([], wapi.get_all_manageable())
         # Checks a manager gets only its own workspaces.
         u.groups.append(gapi.get_one(Group.TIM_MANAGER))
         rapi.delete_one(u.user_id, w2.workspace_id)
-        rapi.create_one(u, w2, UserRoleInWorkspace.WORKSPACE_MANAGER, off)
+        rapi.create_one(u, w2, UserRoleInWorkspace.WORKSPACE_MANAGER, False)
         eq_([w1, w2], wapi.get_all_manageable())