From d8616b7126de7e234b901b9bb166b5e4203da3c0 Mon Sep 17 00:00:00 2001 From: radhika Date: Tue, 28 Jun 2016 15:58:27 -0400 Subject: [PATCH] 9498: show top-level container requests in project#Pipelines_and_processes tab. --- .../app/assets/javascripts/infinite_scroll.js | 3 +- .../app/controllers/application_controller.rb | 2 +- .../container_requests_controller.rb | 5 +++ .../app/controllers/containers_controller.rb | 5 +++ .../app/controllers/projects_controller.rb | 10 ++++-- .../workbench/app/models/container_request.rb | 4 +++ .../_show_jobs_and_pipelines.html.erb | 5 --- .../_show_pipelines_and_processes.html.erb | 5 +++ .../controllers/projects_controller_test.rb | 2 +- .../test/integration/anonymous_access_test.rb | 24 ++++++-------- .../integration/pipeline_instances_test.rb | 2 +- .../test/integration/projects_test.rb | 22 ++++++------- doc/api/methods/groups.html.textile.liquid | 2 ++ .../arvados/v1/groups_controller.rb | 27 +++++++++++++-- .../api/test/fixtures/container_requests.yml | 33 +++++++++++++++++++ services/api/test/fixtures/groups.yml | 6 ++-- services/api/test/fixtures/jobs.yml | 16 --------- .../api/test/fixtures/pipeline_instances.yml | 6 ++-- .../arvados/v1/groups_controller_test.rb | 25 ++++++++++++++ 19 files changed, 142 insertions(+), 62 deletions(-) delete mode 100644 apps/workbench/app/views/projects/_show_jobs_and_pipelines.html.erb create mode 100644 apps/workbench/app/views/projects/_show_pipelines_and_processes.html.erb diff --git a/apps/workbench/app/assets/javascripts/infinite_scroll.js b/apps/workbench/app/assets/javascripts/infinite_scroll.js index 047858c5a0..a0c9efc523 100644 --- a/apps/workbench/app/assets/javascripts/infinite_scroll.js +++ b/apps/workbench/app/assets/javascripts/infinite_scroll.js @@ -151,7 +151,8 @@ function mergeInfiniteContentParams($container) { // For example, filterable.js writes filters in // infiniteContentParamsFilterable ("search for text foo") // without worrying about clobbering the filters set up by the - // tab pane ("only show jobs and pipelines in this tab"). + // tab pane ("only show container requests and pipeline instances + // in this tab"). $.each($container.data(), function(datakey, datavalue) { // Note: We attach these data to DOM elements using // . We store/retrieve them diff --git a/apps/workbench/app/controllers/application_controller.rb b/apps/workbench/app/controllers/application_controller.rb index 0ed629403c..648cae85a6 100644 --- a/apps/workbench/app/controllers/application_controller.rb +++ b/apps/workbench/app/controllers/application_controller.rb @@ -115,7 +115,7 @@ class ApplicationController < ActionController::Base # Column names should always be qualified by a table name and a direction is optional, defaulting to asc # (e.g. "collections.name" or "collections.name desc"). # If a column name is specified, that table will be sorted by that column. - # If there are objects from different models that will be shown (such as in Jobs and Pipelines tab), + # If there are objects from different models that will be shown (such as in Pipelines and processes tab), # then a sort column name can optionally be specified for each model, passed as an comma-separated list (e.g. "jobs.script, pipeline_instances.name") # Currently only one sort column name and direction can be specified for each model. def load_filters_and_paging_params diff --git a/apps/workbench/app/controllers/container_requests_controller.rb b/apps/workbench/app/controllers/container_requests_controller.rb index 4a32cd8171..f5a68fec27 100644 --- a/apps/workbench/app/controllers/container_requests_controller.rb +++ b/apps/workbench/app/controllers/container_requests_controller.rb @@ -1,4 +1,9 @@ class ContainerRequestsController < ApplicationController + skip_around_filter :require_thread_api_token, if: proc { |ctrl| + Rails.configuration.anonymous_user_token and + 'show' == ctrl.action_name + } + def show_pane_list %w(Status Log Advanced) end diff --git a/apps/workbench/app/controllers/containers_controller.rb b/apps/workbench/app/controllers/containers_controller.rb index 86582dff4f..1df2c3acb0 100644 --- a/apps/workbench/app/controllers/containers_controller.rb +++ b/apps/workbench/app/controllers/containers_controller.rb @@ -1,4 +1,9 @@ class ContainersController < ApplicationController + skip_around_filter :require_thread_api_token, if: proc { |ctrl| + Rails.configuration.anonymous_user_token and + 'show' == ctrl.action_name + } + def show_pane_list %w(Status Log Advanced) end diff --git a/apps/workbench/app/controllers/projects_controller.rb b/apps/workbench/app/controllers/projects_controller.rb index e49ed1fab6..3674e314a8 100644 --- a/apps/workbench/app/controllers/projects_controller.rb +++ b/apps/workbench/app/controllers/projects_controller.rb @@ -63,8 +63,8 @@ class ProjectsController < ApplicationController } pane_list << { - :name => 'Jobs_and_pipelines', - :filters => [%w(uuid is_a) + [%w(arvados#job arvados#pipelineInstance)]] + :name => 'Pipelines_and_processes', + :filters => [%w(uuid is_a) + [%w(arvados#containerRequest arvados#pipelineInstance)]] } pane_list << { @@ -213,9 +213,13 @@ class ProjectsController < ApplicationController @name_link_for = {} kind_filters.each do |attr,op,val| (val.is_a?(Array) ? val : [val]).each do |type| + filters = @filters - kind_filters + [['uuid', 'is_a', type]] + if type == 'arvados#containerRequest' + filters = filters + [['container_requests.requesting_container_uuid', '=', nil]] + end objects = @object.contents(order: @order, limit: @limit, - filters: (@filters - kind_filters + [['uuid', 'is_a', type]]), + filters: filters, ) objects.each do |object| @name_link_for[object.andand.uuid] = objects.links_for(object, 'name').first diff --git a/apps/workbench/app/models/container_request.rb b/apps/workbench/app/models/container_request.rb index 62d8bff042..0148de51f7 100644 --- a/apps/workbench/app/models/container_request.rb +++ b/apps/workbench/app/models/container_request.rb @@ -7,6 +7,10 @@ class ContainerRequest < ArvadosBase [ 'description' ] end + def self.goes_in_projects? + true + end + def work_unit(label=nil) ContainerWorkUnit.new(self, label) end diff --git a/apps/workbench/app/views/projects/_show_jobs_and_pipelines.html.erb b/apps/workbench/app/views/projects/_show_jobs_and_pipelines.html.erb deleted file mode 100644 index 3637ef43ca..0000000000 --- a/apps/workbench/app/views/projects/_show_jobs_and_pipelines.html.erb +++ /dev/null @@ -1,5 +0,0 @@ -<%= render_pane 'tab_contents', to_string: true, locals: { - limit: 50, - filters: [['uuid', 'is_a', ["arvados#job", "arvados#pipelineInstance"]]], - sortable_columns: { 'name' => 'jobs.script, pipeline_instances.name', 'description' => 'jobs.description, pipeline_instances.description' } - }.merge(local_assigns) %> diff --git a/apps/workbench/app/views/projects/_show_pipelines_and_processes.html.erb b/apps/workbench/app/views/projects/_show_pipelines_and_processes.html.erb new file mode 100644 index 0000000000..1ee3070941 --- /dev/null +++ b/apps/workbench/app/views/projects/_show_pipelines_and_processes.html.erb @@ -0,0 +1,5 @@ +<%= render_pane 'tab_contents', to_string: true, locals: { + limit: 50, + filters: [['uuid', 'is_a', ["arvados#containerRequest", "arvados#pipelineInstance"]]], + sortable_columns: { 'name' => 'container_requests.name, pipeline_instances.name', 'description' => 'container_requests.description, pipeline_instances.description' } + }.merge(local_assigns) %> diff --git a/apps/workbench/test/controllers/projects_controller_test.rb b/apps/workbench/test/controllers/projects_controller_test.rb index 58914a84ac..c0519bcedf 100644 --- a/apps/workbench/test/controllers/projects_controller_test.rb +++ b/apps/workbench/test/controllers/projects_controller_test.rb @@ -421,7 +421,7 @@ class ProjectsControllerTest < ActionController::TestCase [ ["active", 5, ["aproject", "asubproject"], "anonymously_accessible_project"], - ["user1_with_load", 2, ["project_with_10_collections"], "project_with_2_pipelines_and_60_jobs"], + ["user1_with_load", 2, ["project_with_10_collections"], "project_with_2_pipelines_and_60_crs"], ["admin", 5, ["anonymously_accessible_project", "subproject_in_anonymous_accessible_project"], "aproject"], ].each do |user, page_size, tree_segment, unexpected| test "build my projects tree for #{user} user and verify #{unexpected} is omitted" do diff --git a/apps/workbench/test/integration/anonymous_access_test.rb b/apps/workbench/test/integration/anonymous_access_test.rb index d58a0315ee..6e28e4efb4 100644 --- a/apps/workbench/test/integration/anonymous_access_test.rb +++ b/apps/workbench/test/integration/anonymous_access_test.rb @@ -68,7 +68,7 @@ class AnonymousAccessTest < ActionDispatch::IntegrationTest assert_selector 'a', text: 'Description' assert_selector 'a', text: 'Data collections' - assert_selector 'a', text: 'Jobs and pipelines' + assert_selector 'a', text: 'Pipelines and processes' assert_selector 'a', text: 'Pipeline templates' assert_selector 'a', text: 'Subprojects' assert_selector 'a', text: 'Advanced' @@ -123,39 +123,35 @@ class AnonymousAccessTest < ActionDispatch::IntegrationTest end [ - 'running_job', - 'completed_job', + 'running anonymously accessible cr', 'pipelineInstance' - ].each do |type| - test "anonymous user accesses jobs and pipelines tab in shared project and clicks on #{type}" do + ].each do |proc| + test "anonymous user accesses pipelines and processes tab in shared project and clicks on '#{proc}'" do visit PUBLIC_PROJECT click_link 'Data collections' assert_text 'GNU General Public License' - click_link 'Jobs and pipelines' + click_link 'Pipelines and processes' assert_text 'Pipeline in publicly accessible project' - # click on the specified job - if type.include? 'job' - verify_job_row type - else + if proc.include? 'pipeline' verify_pipeline_instance_row + else + verify_container_request_row proc end end end - def verify_job_row look_for + def verify_container_request_row look_for within first('tr', text: look_for) do click_link 'Show' end assert_text 'Public Projects Unrestricted public data' - assert_text 'script_version' + assert_text 'command' assert_text 'zzzzz-tpzed-xurymjxw79nv3jz' # modified by user assert_no_selector 'a', text: 'zzzzz-tpzed-xurymjxw79nv3jz' - assert_no_selector 'a', text: 'Move job' assert_no_selector 'button', text: 'Cancel' - assert_no_selector 'button', text: 'Re-run job' end def verify_pipeline_instance_row diff --git a/apps/workbench/test/integration/pipeline_instances_test.rb b/apps/workbench/test/integration/pipeline_instances_test.rb index 2ab8beb294..3d8cbf0b63 100644 --- a/apps/workbench/test/integration/pipeline_instances_test.rb +++ b/apps/workbench/test/integration/pipeline_instances_test.rb @@ -82,7 +82,7 @@ class PipelineInstancesTest < ActionDispatch::IntegrationTest wait_for_ajax end - click_link 'Jobs and pipelines' + click_link 'Pipelines and processes' find('tr[data-kind="arvados#pipelineInstance"]', text: '(none)'). find('a', text: 'Show'). click diff --git a/apps/workbench/test/integration/projects_test.rb b/apps/workbench/test/integration/projects_test.rb index 01e84b1c02..1c18a436fd 100644 --- a/apps/workbench/test/integration/projects_test.rb +++ b/apps/workbench/test/integration/projects_test.rb @@ -514,23 +514,23 @@ class ProjectsTest < ActionDispatch::IntegrationTest [ ['project_with_10_pipelines', 10, 0], - ['project_with_2_pipelines_and_60_jobs', 2, 60], + ['project_with_2_pipelines_and_60_crs', 2, 60], ['project_with_25_pipelines', 25, 0], - ].each do |project_name, num_pipelines, num_jobs| - test "scroll pipeline instances tab for #{project_name} with #{num_pipelines} pipelines and #{num_jobs} jobs" do - item_list_parameter = "Jobs_and_pipelines" + ].each do |project_name, num_pipelines, num_crs| + test "scroll pipeline instances tab for #{project_name} with #{num_pipelines} pipelines and #{num_crs} container requests" do + item_list_parameter = "Pipelines_and_processes" scroll_setup project_name, - num_pipelines + num_jobs, + num_pipelines + num_crs, item_list_parameter # check the general scrolling and the pipelines scroll_items_check num_pipelines, "pipeline_", item_list_parameter, 'tr[data-kind="arvados#pipelineInstance"]' - # Check job count separately - jobs_found = page.all('tr[data-kind="arvados#job"]') - found_job_count = jobs_found.count - assert_equal num_jobs, found_job_count, 'Did not find expected number of jobs' + # Check container request count separately + crs_found = page.all('tr[data-kind="arvados#containerRequest"]') + found_cr_count = crs_found.count + assert_equal num_crs, found_cr_count, 'Did not find expected number of container requests' end end @@ -618,8 +618,8 @@ class ProjectsTest < ActionDispatch::IntegrationTest assert_no_selector 'li.disabled', text: 'Copy selected' end - # Go to Jobs and pipelines tab and assert none selected - click_link 'Jobs and pipelines' + # Go to Pipelines and processes tab and assert none selected + click_link 'Pipelines and processes' wait_for_ajax # Since this is the first visit to this tab, all selection options should be disabled diff --git a/doc/api/methods/groups.html.textile.liquid b/doc/api/methods/groups.html.textile.liquid index 9f20a88a95..cd9633db42 100644 --- a/doc/api/methods/groups.html.textile.liquid +++ b/doc/api/methods/groups.html.textile.liquid @@ -29,6 +29,8 @@ table(table table-bordered table-condensed). Note: Because adding access tokens to manifests can be computationally expensive, the @manifest_text@ field is not included in listed collections. If you need it, request a "list of collections":{{site.baseurl}}/api/methods/collections.html with the filter @["owner_uuid", "=", GROUP_UUID]@, and @"manifest_text"@ listed in the select parameter. +Note: Use filters with the attribute format @.@ to filter items of a specific type. For example: @["pipeline_instances.state", "=", "Complete"]@ to filter @pipeline_instances@ where @state@ is @Complete@. All other types of items owned by this group will be unimpacted by this filter and will still be included. + h2. create Create a new Group. diff --git a/services/api/app/controllers/arvados/v1/groups_controller.rb b/services/api/app/controllers/arvados/v1/groups_controller.rb index eae6dca8c0..a1bfb8bc5e 100644 --- a/services/api/app/controllers/arvados/v1/groups_controller.rb +++ b/services/api/app/controllers/arvados/v1/groups_controller.rb @@ -61,10 +61,21 @@ class Arvados::V1::GroupsController < ApplicationController request_orders = @orders.clone @orders = [] - [Group, - Job, PipelineInstance, PipelineTemplate, + request_filters = @filters + + klasses = [Group, + Job, PipelineInstance, PipelineTemplate, ContainerRequest, Collection, - Human, Specimen, Trait].each do |klass| + Human, Specimen, Trait] + + table_names = klasses.map(&:table_name) + request_filters.each do |col, op, val| + if col.index('.') && !table_names.include?(col.split('.', 2)[0]) + raise ArgumentError.new("Invalid attribute '#{col}' in filter") + end + end + + klasses.each do |klass| # If the currently requested orders specifically match the # table_name for the current klass, apply that order. # Otherwise, order by recency. @@ -81,6 +92,16 @@ class Arvados::V1::GroupsController < ApplicationController where_conds[:group_class] = "project" end + @filters = request_filters.map do |col, op, val| + if !col.index('.') + [col, op, val] + elsif (col = col.split('.', 2))[0] == klass.table_name + [col[1], op, val] + else + nil + end + end.compact + @objects = klass.readable_by(*@read_users). order(request_order).where(where_conds) @limit = limit_all - all_objects.count diff --git a/services/api/test/fixtures/container_requests.yml b/services/api/test/fixtures/container_requests.yml index 1e3d773550..04746d3abb 100644 --- a/services/api/test/fixtures/container_requests.yml +++ b/services/api/test/fixtures/container_requests.yml @@ -110,3 +110,36 @@ cr_for_requester2: output_path: test command: ["echo", "hello"] requesting_container_uuid: zzzzz-dz642-requestercntnr1 + +running_anonymous_accessible: + uuid: zzzzz-xvhdp-runninganonaccs + owner_uuid: zzzzz-j7d0g-zhxawtyetzwc5f0 + name: running anonymously accessible cr + state: Committed + priority: 1 + created_at: 2016-01-11 11:11:11.111111111 Z + updated_at: 2016-01-11 11:11:11.111111111 Z + modified_at: 2016-01-11 11:11:11.111111111 Z + modified_by_user_uuid: zzzzz-tpzed-xurymjxw79nv3jz + container_image: test + cwd: test + output_path: test + command: ["echo", "hello"] + container_uuid: zzzzz-dz642-runningcontain2 + +# Test Helper trims the rest of the file + +# Do not add your fixtures below this line as the rest of this file will be trimmed by test_helper + +# container requests in project_with_2_pipelines_and_60_crs +<% for i in 1..60 do %> +cr_<%=i%>_of_60: + uuid: zzzzz-xvhdp-oneof60crs<%= i.to_s.rjust(5, '0') %> + created_at: <%= ((i+5)/5).hour.ago.to_s(:db) %> + owner_uuid: zzzzz-j7d0g-nnncrspipelines + name: cr-<%= i.to_s %> + output_path: test + command: ["echo", "hello"] +<% end %> + +# Do not add your fixtures below this line as the rest of this file will be trimmed by test_helper diff --git a/services/api/test/fixtures/groups.yml b/services/api/test/fixtures/groups.yml index 4029846484..b90a25ced8 100644 --- a/services/api/test/fixtures/groups.yml +++ b/services/api/test/fixtures/groups.yml @@ -196,15 +196,15 @@ project_with_10_pipelines: description: project with 10 pipelines group_class: project -project_with_2_pipelines_and_60_jobs: - uuid: zzzzz-j7d0g-nnjobspipelines +project_with_2_pipelines_and_60_crs: + uuid: zzzzz-j7d0g-nnncrspipelines owner_uuid: zzzzz-tpzed-user1withloadab created_at: 2014-04-21 15:37:48 -0400 modified_by_client_uuid: zzzzz-ozdt8-brczlopd8u8d0jr modified_by_user_uuid: zzzzz-tpzed-user1withloadab modified_at: 2014-04-21 15:37:48 -0400 updated_at: 2014-04-21 15:37:48 -0400 - name: project with 2 pipelines and 60 jobs + name: project with 2 pipelines and 60 crs description: This will result in two pages in the display group_class: project diff --git a/services/api/test/fixtures/jobs.yml b/services/api/test/fixtures/jobs.yml index d0c22d3059..95cb967ffc 100644 --- a/services/api/test/fixtures/jobs.yml +++ b/services/api/test/fixtures/jobs.yml @@ -527,19 +527,3 @@ running_job_with_components: components: component1: zzzzz-8i9sb-jyq01m7in1jlofj component2: zzzzz-d1hrv-partdonepipelin - -# Test Helper trims the rest of the file - -# Do not add your fixtures below this line as the rest of this file will be trimmed by test_helper - -# jobs in project_with_2_pipelines_and_60_jobs -<% for i in 1..60 do %> -job_<%=i%>_of_60: - uuid: zzzzz-8i9sb-oneof100jobs<%= i.to_s.rjust(3, '0') %> - created_at: <%= ((i+5)/5).minute.ago.to_s(:db) %> - owner_uuid: zzzzz-j7d0g-nnjobspipelines - script_version: 7def43a4d3f20789dda4700f703b5514cc3ed250 - state: Complete -<% end %> - -# Do not add your fixtures below this line as the rest of this file will be trimmed by test_helper diff --git a/services/api/test/fixtures/pipeline_instances.yml b/services/api/test/fixtures/pipeline_instances.yml index 04a200ddb0..34dbe9603b 100644 --- a/services/api/test/fixtures/pipeline_instances.yml +++ b/services/api/test/fixtures/pipeline_instances.yml @@ -445,13 +445,13 @@ pipeline_<%=i%>_of_10: title: foo instance input <% end %> -# pipelines in project_with_2_pipelines_and_100_jobs +# pipelines in project_with_2_pipelines_and_60_crs <% for i in 1..2 do %> -pipeline_<%=i%>_of_2_pipelines_and_100_jobs: +pipeline_<%=i%>_of_2_pipelines_and_60_crs: name: pipeline_<%= i %> state: New uuid: zzzzz-d1hrv-abcgneyn6brx<%= i.to_s.rjust(3, '0') %> - owner_uuid: zzzzz-j7d0g-nnjobspipelines + owner_uuid: zzzzz-j7d0g-nnncrspipelines created_at: <%= i.minute.ago.to_s(:db) %> components: foo: diff --git a/services/api/test/functional/arvados/v1/groups_controller_test.rb b/services/api/test/functional/arvados/v1/groups_controller_test.rb index 00846795b4..10534a7061 100644 --- a/services/api/test/functional/arvados/v1/groups_controller_test.rb +++ b/services/api/test/functional/arvados/v1/groups_controller_test.rb @@ -423,4 +423,29 @@ class Arvados::V1::GroupsControllerTest < ActionController::TestCase end assert_equal true, found_projects.include?(groups(:starred_and_shared_active_user_project).uuid) end + + [ + [['owner_uuid', '!=', 'zzzzz-tpzed-xurymjxw79nv3jz'], 200, + 'zzzzz-d1hrv-subprojpipeline', 'zzzzz-d1hrv-1xfj6xkicf2muk2'], + [["pipeline_instances.state", "not in", ["Complete", "Failed"]], 200, + 'zzzzz-d1hrv-1xfj6xkicf2muk2', 'zzzzz-d1hrv-i3e77t9z5y8j9cc'], + [['container_requests.requesting_container_uuid', '=', nil], 200, + 'zzzzz-xvhdp-cr4queuedcontnr', 'zzzzz-xvhdp-cr4requestercn2'], + [['container_requests.no_such_column', '=', nil], 422], + [['container_requests.', '=', nil], 422], + [['.requesting_container_uuid', '=', nil], 422], + [['no_such_table.uuid', '!=', 'zzzzz-tpzed-xurymjxw79nv3jz'], 422], + ].each do |filter, expect_code, expect_uuid, not_expect_uuid| + test "get contents with '#{filter}' filter" do + authorize_with :active + get :contents, filters: [filter], format: :json + assert_response expect_code + if expect_code == 200 + assert_not_empty json_response['items'] + item_uuids = json_response['items'].collect {|item| item['uuid']} + assert_includes(item_uuids, expect_uuid) + assert_not_includes(item_uuids, not_expect_uuid) + end + end + end end -- 2.30.2