Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jkmarx/add tools list landing page #3122

Merged
merged 23 commits into from
Nov 29, 2018
Merged
Show file tree
Hide file tree
Changes from 21 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 6 additions & 0 deletions refinery/tool_manager/serializers.py
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,12 @@ class Meta:
class ToolDefinitionSerializer(serializers.ModelSerializer):
file_relationship = FileRelationshipSerializer()
parameters = ParameterSerializer(many=True, allow_null=True)
workflow = serializers.SerializerMethodField()

def get_workflow(self, tool):
if tool.workflow:
return tool.workflow.uuid
return tool.workflow
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't the 2nd return tool.workflow always be Nonehere?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, am I being redundant?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe a little. How about something like

def get_workflow(self, tool):
    return tool.workflow.uuid if tool.is_workflow() else None

or

def get_workflow(self, tool):
    if tool.is_visualization():
        return None  # Vis Tools have no associated Workflows
    return tool.workflow.uuid

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see how that improves anything.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good to be explicit if we're going to be returning a None. Otherwise, someone could be left wondering why we're returning tool.workflow after already running the if tool.workflow.

Another path could be something like:

def get_workflow(self, tool):
    if tool.workflow is not None:  # Vis Tools have no associated Workflows
        return tool.workflow.uuid


class Meta:
model = ToolDefinition
Expand Down
36 changes: 17 additions & 19 deletions refinery/tool_manager/tests.py
Original file line number Diff line number Diff line change
Expand Up @@ -670,6 +670,23 @@ def test_unallowed_http_verbs(self):
self.delete_response.data['detail'], 'Method "DELETE" not '
'allowed.')

def test_request_without_any_params_returns_all(self):
get_request = self.factory.get(self.tool_defs_url_root)
force_authenticate(get_request, self.user)
get_response = self.tool_defs_view(get_request)
serialized_data = ToolDefinitionSerializer(
ToolDefinition.objects.all(), many=True
).data
self.assertEqual(get_response.status_code, 200)
self.assertEqual(get_response.data, serialized_data)

def test_request_without_data_set_uuid_returns_all(self):
serialized_data = ToolDefinitionSerializer(
ToolDefinition.objects.all(), many=True
).data
self.assertEqual(self.get_response.status_code, 200)
self.assertEqual(self.get_response.data, serialized_data)

def test_for_proper_parameters_in_response(self):
"""ToolDefinitions for Workflows will have an extra field on their
parameter objects
Expand Down Expand Up @@ -710,25 +727,6 @@ def test_request_from_public_dataset_shows_vis_tools_only(self):
)
)

def test_no_query_params_in_get_yields_bad_request(self):
get_request = self.factory.get(self.tool_defs_url_root)
force_authenticate(get_request, self.user)
get_response = self.tool_defs_view(get_request)
self.assertEqual(get_response.status_code, 400)
self.assertIn("Must specify a DataSet UUID", get_response.content)

def test_bad_query_params_in_get_yields_bad_request(self):
get_request = self.factory.get(
"{}?coffee={}".format(
self.tool_defs_url_root,
self.dataset.uuid
)
)
force_authenticate(get_request, self.user)
get_response = self.tool_defs_view(get_request)
self.assertEqual(get_response.status_code, 400)
self.assertIn("Must specify a DataSet UUID", get_response.content)

def test_missing_dataset_in_get_yields_bad_request(self):
dataset_uuid = str(uuid.uuid4())

Expand Down
9 changes: 9 additions & 0 deletions refinery/tool_manager/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
from django_docker_engine.proxy import Proxy
from rest_framework.decorators import detail_route
from rest_framework.generics import get_object_or_404
from rest_framework.response import Response
from rest_framework.viewsets import ModelViewSet

from core.models import DataSet
Expand Down Expand Up @@ -56,6 +57,14 @@ class ToolDefinitionsViewSet(ToolManagerViewSetBase):
lookup_field = 'uuid'
http_method_names = ['get']

def list(self, request):
if request.query_params.get('data_set_uuid'):
return super(ToolDefinitionsViewSet, self).list(request)

serializer = ToolDefinitionSerializer(ToolDefinition.objects.all(),
many=True)
return Response(serializer.data)

def get_queryset(self):
if self.request.user.has_perm('core.change_dataset', self.data_set):
return ToolDefinition.objects.all()
Expand Down
2 changes: 2 additions & 0 deletions refinery/ui/Gruntfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -647,6 +647,8 @@ module.exports = function (grunt) {
'<%= cfg.basePath.ui.src %>/styles/treemap.less',
'<%= cfg.basePath.ui.compile %>/styles/dashboard.css':
'<%= cfg.basePath.ui.src %>/styles/dashboard.less',
'<%= cfg.basePath.ui.compile %>/styles/home.css':
'<%= cfg.basePath.ui.src %>/styles/home.less',
'<%= cfg.basePath.ui.compile %>/styles/provenance-visualization.css':
'<%= cfg.basePath.ui.src %>/styles/provenance-visualization.less',
'<%= cfg.basePath.ui.compile %>/styles/file-browser.css':
Expand Down
18 changes: 0 additions & 18 deletions refinery/ui/source/js/home/ctrls/home-ctrl.js

This file was deleted.

24 changes: 0 additions & 24 deletions refinery/ui/source/js/home/ctrls/home-ctrl.spec.js

This file was deleted.

35 changes: 35 additions & 0 deletions refinery/ui/source/js/home/ctrls/tool-list-ctrl.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
/**
* Tool List Ctrl
* @namespace ToolListCtrl
* @desc Component controller for the tool list partial.
* @memberOf refineryApp.refineryHome
*/
(function () {
'use strict';
angular
.module('refineryHome')
.controller('ToolListCtrl', ToolListCtrl);

ToolListCtrl.$inject = ['toolListService'];

function ToolListCtrl (toolListService) {
var vm = this;
vm.toolList = toolListService.toolList;

activate();
/*
* -----------------------------------------------------------------------------
* Methods
* -----------------------------------------------------------------------------
*/
function activate () {
// tool list should won't be updated often, so no need for api call
// if list is already populated
if (!vm.toolList.length) {
toolListService.getTools().then(function () {
vm.toolList = toolListService.toolList;
});
}
}
}
})();
65 changes: 65 additions & 0 deletions refinery/ui/source/js/home/ctrls/tool-list-ctrl.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
(function () {
'use strict';

describe('Controller: Tool List Ctrl', function () {
var ctrl;
var genCtrl;
var mockService;
var mockServiceResponse = false;
var scope;
var service;
var rootScope;

beforeEach(module('refineryApp'));
beforeEach(module('refineryHome'));
beforeEach(inject(function (
$rootScope,
$controller,
$httpBackend,
settings,
toolListService
) {
service = toolListService;
mockService = spyOn(service, 'getTools').and.callFake(function () {
return {
then: function () {
mockServiceResponse = true;
}
};
});
rootScope = $rootScope;
scope = rootScope.$new();
genCtrl = $controller;
ctrl = $controller('ToolListCtrl', {
$scope: scope
});
}));

it('Tool List Ctrl ctrl should exist', function () {
expect(ctrl).toBeDefined();
});

it('data variables should exist for views', function () {
expect(ctrl.toolList).toEqual([]);
});

it('should call service when initalized', function () {
scope.$apply();
expect(mockService).toHaveBeenCalled();
expect(mockServiceResponse).toEqual(true);
});

it('should not call service when list exists', function () {
mockServiceResponse = false;
mockService.calls.reset();
angular.copy([{ name: 'FastQC' }], service.toolList);
var newScope = rootScope.$new();
genCtrl('ToolListCtrl', {
$scope: newScope
});
newScope.$apply();
expect(mockService).not.toHaveBeenCalled();
expect(mockServiceResponse).toEqual(false);
});
});
})();
2 changes: 0 additions & 2 deletions refinery/ui/source/js/home/directives/home.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,14 +13,12 @@
$templateCache,
$window
) {
// Parent component contains the input-group (child) component
$templateCache.put(
$window.getStaticUrl('partials/home/views/home.html'),
'<div id="home-main">/div>'
);

var scope = $rootScope.$new();
// Parent component
var template = '<rp-home></rp-home>';
directiveElement = $compile(template)(scope);
scope.$digest();
Expand Down
17 changes: 17 additions & 0 deletions refinery/ui/source/js/home/directives/tool-list.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Tool List Component
* @namespace rpToolList
* @desc Component to show a list of all the tools
* @memberOf refineryApp.refineryHome
*/
(function () {
'use strict';
angular
.module('refineryHome')
.component('rpToolList', {
controller: 'ToolListCtrl',
templateUrl: ['$window', function ($window) {
return $window.getStaticUrl('partials/home/partials/tool-list.html');
}]
});
})();
40 changes: 40 additions & 0 deletions refinery/ui/source/js/home/directives/tool-list.spec.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
(function () {
'use strict';

describe('rpToolList component unit test', function () {
beforeEach(module('refineryApp'));
beforeEach(module('refineryHome'));

var directiveElement;

beforeEach(inject(function (
$compile,
$httpBackend,
$rootScope,
$templateCache,
$window,
settings
) {
// Mock api call due to ctrl activate method
$httpBackend
.whenGET(
settings.appRoot +
settings.refineryApiV2 + '/tool_definitions/?data_set_uuid=').respond(200, []);

$templateCache.put(
$window.getStaticUrl('partials/home/partials/tool-list.html'),
'<div id="tool-list">/div>'
);

var scope = $rootScope.$new();
var template = '<rp-tool-list></rp-tool-list>';
directiveElement = $compile(template)(scope);
scope.$digest();
}));

it('generates the appropriate HTML', function () {
expect(directiveElement.html()).toContain('tool-list');
expect(directiveElement.html()).toContain('</div>');
});
});
})();
36 changes: 17 additions & 19 deletions refinery/ui/source/js/home/partials/data-set-chart.html
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
<div class="row p-t-1">
<div class="col-md-4 col-md-offset-7">
<ui-select
append-to-body="true"
ng-model="$ctrl.selectedAttribute.select"
on-select="$ctrl.updateAttribute($item)"
theme="select2"
class="text-left p-l-1 p-b-1">
<ui-select-match placeholder="Select an attribute">
{{$select.selected.name }}
</ui-select-match>
<ui-select-choices
position="down"
repeat="attribute in $ctrl.attributes | orderBy:'name' track by $index">
<span ng-bind-html="attribute.name"></span>
</ui-select-choices>
</ui-select>
<canvas id="files-bar-chart"></canvas>
</div>
<div>
<ui-select
append-to-body="true"
ng-model="$ctrl.selectedAttribute.select"
on-select="$ctrl.updateAttribute($item)"
theme="select2"
class="text-left p-l-1 p-b-1">
<ui-select-match placeholder="Select an attribute">
{{$select.selected.name }}
</ui-select-match>
<ui-select-choices
position="down"
repeat="attribute in $ctrl.attributes | orderBy:'name' track by $index">
<span ng-bind-html="attribute.name"></span>
</ui-select-choices>
</ui-select>
<canvas id="files-bar-chart"></canvas>
</div>
21 changes: 21 additions & 0 deletions refinery/ui/source/js/home/partials/tool-list.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
<div>
<div class="refinery-header">
<h3>
Analysis and Visualization Tools
</h3>
</div>
<ul class="list-unstyled">
<li ng-repeat="tool in $ctrl.toolList" class="p-b-1-2">
<i class="{{tool.tool_type|toolTypeIcon}} p-r-1-4" aria-hidden="true"></i>
<span ng-if="tool.tool_type=='WORKFLOW'">
<a ng-href="/workflows/{{tool.workflow}}">
{{ tool.name }}
</a>
</span>
<span ng-if="tool.tool_type!=='WORKFLOW'">
{{ tool.name }}
</span>
- {{ tool.description }}
</li>
</ul>
</div>