Repository: Duke-GCB/DukeDSClient

Issue:
Add option to filter projects by role
Me:

> ... Duke DS team has developed a UI change to include the project role in the project listing (screenshot attached). This is on their dev server (https://dev.dataservice.duke.edu), and they’ve asked for feedback.
> 
> One suggestion I’d have would be a select box to let you filter the list based on your role (e.g. show me only projects where I am an admin).
> 
> Would such a change be useful in the command-line tool also? For example, running `ddsclient list —role admin` could display only the projects for which you have admin role.
> 

User:
> Being able to filter by it on both the GUI and the command line tool would be very helpful for us.  The mockup that you presented looks nice and will be very helpful as we sort through all of our projects (especially until we’re able to take ourselves off of old projects).


Discussion:

Patch:
diff --git a/ddsc/cmdparser.py b/ddsc/cmdparser.py
index b64cb30..9bf2a7b 100644
--- a/ddsc/cmdparser.py
+++ b/ddsc/cmdparser.py
@@ -160,6 +160,21 @@ def _add_auth_role_arg(arg_parser, default_permissions):
                             default=default_permissions)
 
 
+def _add_project_filter_auth_role_arg(arg_parser):
+    """
+    Adds optional auth_role filtering parameter to a parser.
+    :param arg_parser: ArgumentParser parser to add this argument to.
+    """
+    help_text = "Filters project listing to just those projects with the specified role. "
+    help_text += "See command list_auth_roles for AuthRole values."
+    arg_parser.add_argument("--auth-role",
+                            metavar='AuthRole',
+                            type=to_unicode,
+                            dest='auth_role',
+                            help=help_text,
+                            default=None)
+
+
 def _add_copy_project_arg(arg_parser):
     """
     Adds optional copy_project parameter to a parser.
@@ -378,7 +393,9 @@ class CommandParser(object):
         """
         description = "Show a list of project names or folders/files of a single project."
         list_parser = self.subparsers.add_parser('list', description=description)
-        add_project_name_arg(list_parser, required=False, help_text="Name of the project to show details for.")
+        project_name_or_auth_role = list_parser.add_mutually_exclusive_group(required=False)
+        _add_project_filter_auth_role_arg(project_name_or_auth_role)
+        add_project_name_arg(project_name_or_auth_role, required=False, help_text="Name of the project to show details for.")
         list_parser.set_defaults(func=list_func)
 
     def register_delete_command(self, delete_func):
diff --git a/ddsc/ddsclient.py b/ddsc/ddsclient.py
index 0f2d9b1..ff2449a 100644
--- a/ddsc/ddsclient.py
+++ b/ddsc/ddsclient.py
@@ -305,20 +305,30 @@ class ListCommand(object):
         Lists project names.
         :param args Namespace arguments parsed from the command line
         """
+        # project_name and auth_role args are mutually exclusive
         if args.project_name:
             project = self.remote_store.fetch_remote_project(args.project_name, must_exist=True)
             self.print_project_details(project)
         else:
-            self.print_project_names()
+            self.print_project_names(args.auth_role)
 
-    def print_project_details(self, project):
+    @staticmethod
+    def print_project_details(project):
         filename_list = ProjectFilenameList()
         filename_list.walk_project(project)
         for info in filename_list.details:
             print(info)
 
-    def print_project_names(self):
-        names = self.remote_store.get_project_names()
+    def print_project_names(self, filter_auth_role):
+        """
+        Prints project names to stdout for all projects or just those with the specified auth_role
+        :param filter_auth_role: str: optional auth_role to filter project list
+        """
+        if filter_auth_role:
+            projects = self.remote_store.get_projects_with_auth_role(auth_role=filter_auth_role)
+            names = [project['name'] for project in projects]
+        else:
+            names = self.remote_store.get_project_names()
         if names:
             for name in names:
                 print(pipes.quote(name))


Test patch:
diff --git a/ddsc/core/remotestore.py b/ddsc/core/remotestore.py
index b9f3e2a..6eeee03 100644
--- a/ddsc/core/remotestore.py
+++ b/ddsc/core/remotestore.py
@@ -254,6 +254,23 @@ class RemoteStore(object):
             names.append(project['name'])
         return names
 
+    def get_projects_with_auth_role(self, auth_role):
+        """
+        Return the list of projects that have the specified auth role from the list that the current user has access to.
+        :param auth_role: str: auth role we are filtering for
+        :return: [dict]: list of projects that have auth_role permissions for the current user
+        """
+        user = self.get_current_user()
+        # user.id
+        projects = []
+        response = self.data_service.get_projects().json()
+        for project in response['results']:
+            project_id = project['id']
+            permissions = self.data_service.get_user_project_permission(project_id, user.id).json()
+            if auth_role == permissions['auth_role']['id']:
+                projects.append(project)
+        return projects
+
     def delete_project_by_name(self, project_name):
         """
         Find the project named project_name and delete it raise error if not found.
diff --git a/ddsc/core/tests/test_remotestore.py b/ddsc/core/tests/test_remotestore.py
index b771f30..23d56ea 100644
--- a/ddsc/core/tests/test_remotestore.py
+++ b/ddsc/core/tests/test_remotestore.py
@@ -1,6 +1,6 @@
 import json
 from unittest import TestCase
-from mock import MagicMock
+from mock import MagicMock, Mock
 from mock.mock import patch
 from ddsc.core.remotestore import RemoteProject, RemoteFolder, RemoteFile, RemoteUser
 from ddsc.core.remotestore import RemoteStore
@@ -416,6 +416,39 @@ class TestRemoteStore(TestCase):
         expected_ids = set(["project_admin", "project_viewer"])
         self.assertEqual(expected_ids, ids)
 
+    @patch("ddsc.core.remotestore.DataServiceApi")
+    def test_get_projects_with_auth_role(self, mock_data_service_api):
+        projects_resp = Mock()
+        projects_resp.json.return_value = {
+            'results': [
+                {
+                    'id': '123'
+                },
+                {
+                    'id': '456'
+                }
+            ]
+        }
+        mock_data_service_api.return_value.get_projects.return_value = projects_resp
+        permission_resp = Mock()
+        permission_resp.json.side_effect = [
+            {
+                'auth_role': {
+                    'id': 'project_admin'
+                }
+            }, {
+                'auth_role': {
+                    'id': 'file_downloader'
+                }
+            }
+        ]
+        mock_data_service_api.return_value.get_user_project_permission.return_value = permission_resp
+        remote_store = RemoteStore(config=MagicMock())
+        result = remote_store.get_projects_with_auth_role(auth_role='project_admin')
+        mock_data_service_api.return_value.get_projects.assert_called()
+        self.assertEqual(1, len(result))
+        self.assertEqual('123', result[0]['id'])
+
 
 class TestRemoteProjectChildren(TestCase):
     def test_simple_case(self):
diff --git a/ddsc/tests/test_cmdparser.py b/ddsc/tests/test_cmdparser.py
index 6f66581..34523cb 100644
--- a/ddsc/tests/test_cmdparser.py
+++ b/ddsc/tests/test_cmdparser.py
@@ -1,6 +1,7 @@
 from __future__ import absolute_import
 from unittest import TestCase
 from ddsc.cmdparser import CommandParser
+from mock import Mock
 
 
 def no_op():
@@ -51,3 +52,31 @@ class TestCommandParser(TestCase):
         self.assertEqual(['share'], list(command_parser.subparsers.choices.keys()))
         command_parser.run_command(['share', '-p', 'someproject', '--user', 'joe123', '--msg-file', 'setup.py'])
         self.assertIn('setup(', self.parsed_args.msg_file.read())
+
+    def test_list_command(self):
+        func = Mock()
+        command_parser = CommandParser()
+        command_parser.register_list_command(func)
+        self.assertEqual(['list'], list(command_parser.subparsers.choices.keys()))
+
+        # Test simple listing
+        command_parser.run_command(['list'])
+        func.assert_called()
+        args, kwargs = func.call_args
+        self.assertEqual(args[0].auth_role, None)
+        self.assertEqual(args[0].project_name, None)
+        func.reset_mock()
+
+        # Test simple listing single project
+        command_parser.run_command(['list', '-p', 'mouse'])
+        func.assert_called()
+        args, kwargs = func.call_args
+        self.assertEqual(args[0].auth_role, None)
+        self.assertEqual(args[0].project_name, 'mouse')
+
+        # Test simple listing auth_role
+        command_parser.run_command(['list', '--auth-role', 'project_admin'])
+        func.assert_called()
+        args, kwargs = func.call_args
+        self.assertEqual(args[0].auth_role, 'project_admin')
+        self.assertEqual(args[0].project_name, None)
diff --git a/ddsc/tests/test_ddsclient.py b/ddsc/tests/test_ddsclient.py
index d612b03..5e0c37d 100644
--- a/ddsc/tests/test_ddsclient.py
+++ b/ddsc/tests/test_ddsclient.py
@@ -1,8 +1,8 @@
 from __future__ import absolute_import
 from unittest import TestCase
-from ddsc.ddsclient import UploadCommand
+from ddsc.ddsclient import UploadCommand, ListCommand
 from ddsc.ddsclient import ShareCommand, DeliverCommand, read_argument_file_contents
-from mock import patch, MagicMock, Mock
+from mock import patch, MagicMock, Mock, call
 
 
 class TestUploadCommand(TestCase):
@@ -106,3 +106,38 @@ class TestDDSClient(TestCase):
         self.assertEqual('', read_argument_file_contents(None))
         with open("setup.py") as infile:
             self.assertIn("setup(", read_argument_file_contents(infile))
+
+
+class TestListCommand(TestCase):
+    @patch('sys.stdout.write')
+    @patch('ddsc.ddsclient.RemoteStore')
+    def test_print_project_names_no_auth_role(self, mock_remote_store, mock_print):
+        mock_remote_store.return_value.get_project_names.return_value = ['one', 'two', 'three']
+        cmd = ListCommand(MagicMock())
+        cmd.print_project_names(filter_auth_role=None)
+        expected_calls = [
+            call("one"),
+            call("\n"),
+            call("two"),
+            call("\n"),
+            call("three"),
+            call("\n")
+        ]
+        self.assertEqual(expected_calls, mock_print.call_args_list)
+
+    @patch('sys.stdout.write')
+    @patch('ddsc.ddsclient.RemoteStore')
+    def test_print_project_names_with_auth_role(self, mock_remote_store, mock_print):
+        mock_remote_store.return_value.get_projects_with_auth_role.return_value = [
+            {'name': 'mouse'},
+            {'name': 'ant'},
+        ]
+        cmd = ListCommand(MagicMock())
+        cmd.print_project_names(filter_auth_role='project_admin')
+        expected_calls = [
+            call("mouse"),
+            call("\n"),
+            call("ant"),
+            call("\n")
+        ]
+        self.assertEqual(expected_calls, mock_print.call_args_list)