Increase consistency between job index and show.

Add tests for new date range and history filtering as well as to ensure only admins get to see external_id and command_line and that users cannot see each other's jobs.

Always allow admins to views all jobs on index (instead of only when user_details is specified) and show. Add user_email and external_id to show (for admins) to bring it inline with index and add command_line to index (for admins) to bring it in line with show.

Show still allow more details including job standard error and output as well as job metrics.

lib/galaxy/model/__init__.py

@@ -520,9 +520,15 @@ class Job( object, HasJobMetrics, Dictifiable ):
                 dataset.blurb = 'deleted'
                 dataset.peek = 'Job deleted'
                 dataset.info = 'Job output deleted by user before job completed'
-    def to_dict( self, view='collection' ):
+
+    def to_dict( self, view='collection', system_details=False ):
         rval = super( Job, self ).to_dict( view=view )
         rval['tool_id'] = self.tool_id
+        if system_details:
+            # System level details that only admins should have.
+            rval['external_id'] = self.job_runner_external_id
+            rval['command_line'] = self.command_line
+
         if view == 'element':
             param_dict = dict( [ ( p.name, p.value ) for p in self.parameters ] )
             rval['params'] = param_dict

lib/galaxy/web/security/__init__.py

@@ -73,7 +73,7 @@ class SecurityHelper( object ):
         if not isinstance( rval, dict ):
             return rval
         for k, v in rval.items():
-            if ( k == 'id' or k.endswith( '_id' ) ) and v is not None and k not in [ 'tool_id' ]:
+            if ( k == 'id' or k.endswith( '_id' ) ) and v is not None and k not in [ 'tool_id', 'external_id' ]:
                 try:
                     rval[ k ] = self.encode_id( v )
                 except Exception:

lib/galaxy/webapps/galaxy/api/jobs.py

@@ -55,12 +55,10 @@ class JobController( BaseAPIController, UsesHistoryDatasetAssociationMixin, Uses
         :returns:   list of dictionaries containing summary job information
         """
         state = kwd.get( 'state', None )
-        if trans.user_is_admin() and kwd.get('user_details', False):
-            is_extended_service = True
-        else:
-            is_extended_service = False
+        is_admin = trans.user_is_admin()
+        user_details = kwd.get('user_details', False)
 
-        if is_extended_service:
+        if is_admin:
             query = trans.sa_session.query( trans.app.model.Job )
         else:
             query = trans.sa_session.query( trans.app.model.Job ).filter(trans.app.model.Job.user == trans.user)
@@ -98,9 +96,9 @@ class JobController( BaseAPIController, UsesHistoryDatasetAssociationMixin, Uses
         else:
             order_by = trans.app.model.Job.update_time.desc()
         for job in query.order_by( order_by ).all():
-            j = self.encode_all_ids( trans, job.to_dict( 'collection' ), True )
-            if is_extended_service:
-                j['external_id'] = job.job_runner_external_id
+            job_dict = job.to_dict( 'collection', system_details=is_admin )
+            j = self.encode_all_ids( trans, job_dict, True )
+            if user_details:
                 j['user_email'] = job.user.email
             out.append(j)
 
@@ -123,12 +121,13 @@ class JobController( BaseAPIController, UsesHistoryDatasetAssociationMixin, Uses
         :returns:   dictionary containing full description of job data
         """
         job = self.__get_job( trans, id )
-        job_dict = self.encode_all_ids( trans, job.to_dict( 'element' ), True )
+        is_admin = trans.user_is_admin()
+        job_dict = self.encode_all_ids( trans, job.to_dict( 'element', system_details=is_admin ), True )
         full_output = util.asbool( kwd.get( 'full', 'false' ) )
         if full_output:
             job_dict.update( dict( stderr=job.stderr, stdout=job.stdout ) )
-            if trans.user_is_admin():
-                job_dict['command_line'] = job.command_line
+            if is_admin:
+                job_dict['user_email'] = job.user.email
 
                 def metric_to_dict(metric):
                     metric_name = metric.metric_name
@@ -199,10 +198,16 @@ class JobController( BaseAPIController, UsesHistoryDatasetAssociationMixin, Uses
             decoded_job_id = trans.security.decode_id( id )
         except Exception:
             raise exceptions.MalformedId()
-        query = trans.sa_session.query( trans.app.model.Job ).filter(
-            trans.app.model.Job.user == trans.user,
-            trans.app.model.Job.id == decoded_job_id
-        )
+        query = trans.sa_session.query( trans.app.model.Job )
+        if trans.user_is_admin():
+            query = query.filter(
+                trans.app.model.Job.id == decoded_job_id
+            )
+        else:
+            query = query.filter(
+                trans.app.model.Job.user == trans.user,
+                trans.app.model.Job.id == decoded_job_id
+            )
         job = query.first()
         if job is None:
             raise exceptions.ObjectNotFound()

test/api/test_jobs.py

@@ -1,4 +1,6 @@
+import datetime
 import json
+import time
 from operator import itemgetter
 
 from base import api
@@ -11,14 +13,19 @@ class JobsApiTestCase( api.ApiTestCase, TestsDatasets ):
     def test_index( self ):
         # Create HDA to ensure at least one job exists...
         self.__history_with_new_dataset()
-        jobs_response = self._get( "jobs" )
-
-        self._assert_status_code_is( jobs_response, 200 )
-
-        jobs = jobs_response.json()
-        assert isinstance( jobs, list )
+        jobs = self.__jobs_index()
         assert "upload1" in map( itemgetter( "tool_id" ), jobs )
 
+    def test_system_details_admin_only( self ):
+        self.__history_with_new_dataset()
+        jobs = self.__jobs_index( admin=False )
+        job = jobs[0]
+        self._assert_not_has_keys( job, "command_line", "external_id" )
+
+        jobs = self.__jobs_index( admin=True )
+        job = jobs[0]
+        self._assert_has_keys( job, "command_line", "external_id" )
+
     def test_index_state_filter( self ):
         # Initial number of ok jobs
         original_count = len( self.__uploads_with_state( "ok" ) )
@@ -31,6 +38,33 @@ class JobsApiTestCase( api.ApiTestCase, TestsDatasets ):
         new_count = len( self.__uploads_with_state( "ok" ) )
         assert original_count < new_count
 
+    def test_index_date_filter( self ):
+        self.__history_with_new_dataset()
+        two_weeks_ago = (datetime.datetime.utcnow() - datetime.timedelta(7)).isoformat()
+        last_week = (datetime.datetime.utcnow() - datetime.timedelta(7)).isoformat()
+        next_week = (datetime.datetime.utcnow() + datetime.timedelta(7)).isoformat()
+        today = datetime.datetime.utcnow().isoformat()
+        tomorrow = (datetime.datetime.utcnow() + datetime.timedelta(1)).isoformat()
+
+        jobs = self.__jobs_index( data={"date_range_min": today[0:10], "date_range_max": tomorrow[0:10]} )
+        assert len( jobs ) > 0
+        today_job_id = jobs[0]["id"]
+
+        jobs = self.__jobs_index( data={"date_range_min": two_weeks_ago, "date_range_max": last_week} )
+        assert today_job_id not in map(itemgetter("id"), jobs)
+
+        jobs = self.__jobs_index( data={"date_range_min": last_week, "date_range_max": next_week} )
+        assert today_job_id in map(itemgetter("id"), jobs)
+
+    def test_index_history( self ):
+        history_id, _ = self.__history_with_new_dataset()
+        jobs = self.__jobs_index( data={"history_id": history_id} )
+        assert len( jobs ) > 0
+
+        history_id = self._new_history()
+        jobs = self.__jobs_index( data={"history_id": history_id} )
+        assert len( jobs ) == 0
+
     def test_index_multiple_states_filter( self ):
         # Initial number of ok jobs
         original_count = len( self.__uploads_with_state( "ok", "new" ) )
@@ -58,6 +92,22 @@ class JobsApiTestCase( api.ApiTestCase, TestsDatasets ):
         job_details = show_jobs_response.json()
         self._assert_has_key( job_details, 'id', 'state', 'exit_code', 'update_time', 'create_time' )
 
+    def test_show_security( self ):
+        history_id, _ = self.__history_with_new_dataset()
+        jobs_response = self._get( "jobs", data={"history_id": history_id} )
+        job = jobs_response.json()[ 0 ]
+        job_id = job[ "id" ]
+
+        show_jobs_response = self._get( "jobs/%s" % job_id, admin=False )
+        self._assert_not_has_keys( show_jobs_response.json(), "command_line", "external_id" )
+
+        with self._different_user():
+            show_jobs_response = self._get( "jobs/%s" % job_id, admin=False )
+            self._assert_status_code_is( show_jobs_response, 404 )
+
+        show_jobs_response = self._get( "jobs/%s" % job_id, admin=True )
+        self._assert_has_keys( show_jobs_response.json(), "command_line", "external_id" )
+
     def test_search( self ):
         history_id, dataset_id = self.__history_with_ok_dataset()
 
@@ -133,3 +183,10 @@ class JobsApiTestCase( api.ApiTestCase, TestsDatasets ):
         history_id, dataset_id = self.__history_with_new_dataset()
         self._wait_for_history( history_id, assert_ok=True )
         return history_id, dataset_id
+
+    def __jobs_index( self, **kwds ):
+        jobs_response = self._get( "jobs", **kwds )
+        self._assert_status_code_is( jobs_response, 200 )
+        jobs = jobs_response.json()
+        assert isinstance( jobs, list )
+        return jobs

test/base/api.py

@@ -11,6 +11,7 @@ from .api_util import get_user_api_key
 from .api_asserts import (
     assert_status_code_is,
     assert_has_keys,
+    assert_not_has_keys,
     assert_error_code_is,
 )
 
@@ -83,6 +84,9 @@ class ApiTestCase( TwillTestCase ):
     def _assert_has_keys( self, response, *keys ):
         assert_has_keys( response, *keys )
 
+    def _assert_not_has_keys( self, response, *keys ):
+        assert_not_has_keys( response, *keys )
+
     def _assert_error_code_is( self, response, error_code ):
         assert_error_code_is( response, error_code )
 

test/base/api_asserts.py

@@ -20,6 +20,11 @@ def assert_has_keys( response, *keys ):
         assert key in response, "Response [%s] does not contain key [%s]" % ( response, key )
 
 
+def assert_not_has_keys( response, *keys ):
+    for key in keys:
+        assert key not in response, "Response [%s] contains invalid key [%s]" % ( response, key )
+
+
 def assert_error_code_is( response, error_code ):
     if hasattr( response, "json" ):
         response = response.json()