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

Mccalluc/user files sorting #1950

Merged
merged 12 commits into from
Aug 1, 2017
23 changes: 17 additions & 6 deletions refinery/data_set_manager/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
from django.utils.http import urlquote, urlunquote

import requests
from requests.exceptions import HTTPError

import core

Expand Down Expand Up @@ -826,11 +825,23 @@ def search_solr(encoded_params, core):
"""
url_portion = '/'.join([core, "select"])
url = urlparse.urljoin(settings.REFINERY_SOLR_BASE_URL, url_portion)
try:
full_response = requests.get(url, params=encoded_params)
full_response.raise_for_status()
except HTTPError as e:
logger.error(e)
full_response = requests.get(url, params=encoded_params)
if not full_response.ok:
try:
response_obj = json.loads(full_response.content)
except ValueError:
raise RuntimeError(
'Expected Solr JSON, not: {}'.format(full_response.content)
)
try:
raise RuntimeError('Solr error: {}\nin context: {}'.format(
response_obj['error']['msg'],
response_obj
))
except KeyError:
raise RuntimeError(
'Not expected response structure: {}'.format(response_obj)
)

response = full_response.content

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,17 @@
'$log',
'$q',
'userFileBrowserFactory',
'userFileSortsService',
'gridOptionsService'
];

function UserFileBrowserFilesCtrl ($log, $q, userFileBrowserFactory, gridOptionsService) {
function UserFileBrowserFilesCtrl (
$log,
$q,
userFileBrowserFactory,
userFileSortsService,
gridOptionsService
) {
var vm = this;
var promise = $q.defer();
var getUserFiles = userFileBrowserFactory.getUserFiles;
Expand All @@ -25,8 +32,38 @@
promise.reject();
});

vm.sortChanged = function (grid, sortColumns) {
if (typeof sortColumns !== 'undefined') {
for (var i = 0; i < sortColumns.length; i++) {
var column = sortColumns[i];
userFileSortsService.fields[i] = {
name: column.field,
direction: column.sort.direction
// NOTE: UI Grid uiGridConstants.ASC and DESC happen to match
// "asc" and "desc" for solr, but if that changed, this breaks.
// NOTE: column.sort.priority seems to be redundant with array order,
// but I don't think we have this guaranteed.
};
}

// TODO: This is copy-and-paste
getUserFiles().then(function (solr) {
// TODO: Should there be something that wraps up this "then"? It is repeated.
// gridOptionsService.columnDefs = userFileBrowserFactory.createColumnDefs();
gridOptionsService.data = userFileBrowserFactory.createData(solr.nodes);
promise.resolve();
}, function () {
$log.error('/user/files/ request failed');
promise.reject();
});
}
};

gridOptionsService.appScopeProvider = vm;
vm.gridOptions = gridOptionsService;
vm.gridOptions.onRegisterApi = function (api) {
api.core.on.sortChanged(null, vm.sortChanged);
};
}
})();

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
(function () {
'use strict';
angular
.module('refineryApp')
.factory('userFileSortsService', userFileSortsService);

userFileSortsService.$inject = [];

function userFileSortsService () {
var userFileSorts = {
fields: []
};
return userFileSorts;
}
})();
21 changes: 19 additions & 2 deletions refinery/ui/source/js/user-file-browser/services/user-file.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,19 @@
.factory('userFileService', userFileService);

// TODO: userFileBrowserFiltersCtrl.filters is what I need, but maybe I need a new service?
userFileService.$inject = ['$resource', 'settings', 'userFileFiltersService'];
userFileService.$inject = [
'$resource',
'settings',
'userFileFiltersService',
'userFileSortsService'
];

function userFileService ($resource, settings, userFileFiltersService) {
function userFileService (
$resource,
settings,
userFileFiltersService,
userFileSortsService
) {
var userFile = $resource(
settings.appRoot + settings.refineryApiV2 + '/user/files/',
{},
Expand All @@ -27,6 +37,13 @@
});
// TODO: Repeated fq params may be more efficient, but not a big deal
return filters.join(' AND ');
},
sort: function () {
var sort = userFileSortsService.fields.map(function (field) {
return field.name + '_Characteristics_generic_s ' + field.direction
+ ', ' + field.name + '_Factor_Value_s ' + field.direction;
}).join(', ');
return sort;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@
.expectGET(
settings.appRoot +
settings.refineryApiV2 +
'/user/files/?fq=&limit=100'
'/user/files/?fq=&limit=100&sort='
Copy link
Member

Choose a reason for hiding this comment

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

Test for appropriate structure after sort=

Copy link
Member Author

Choose a reason for hiding this comment

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

actually, now that I look at it, that is already being done with fakeResponse.

).respond(200, fakeResponse);
});
});
Expand Down