Skip to content

Commit

Permalink
Merge branch '53-add-provider-filter-in-search' into 'main'
Browse files Browse the repository at this point in the history
Resolve "Add provider filter in search"

Closes #53

See merge request pub/terrareg!97
  • Loading branch information
MatthewJohn committed May 9, 2022
2 parents d126792 + 7f8e9d2 commit edda9f5
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 42 deletions.
18 changes: 9 additions & 9 deletions terrareg/module_search.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ def search_module_providers(
offset: int,
limit: int,
query: str=None,
namespace: str=None,
module: str=None,
provider: str=None,
namespaces: list=None,
modules: list=None,
providers: list=None,
verified: bool=False,
namespace_trust_filters: list=NamespaceTrustFilter.UNSPECIFIED):

Expand All @@ -108,21 +108,21 @@ def search_module_providers(
select = cls._get_search_query_filter(select, query)

# If provider has been supplied, select by that
if provider:
if providers:
select = select.where(
db.module_provider.c.provider == provider
db.module_provider.c.provider.in_(providers)
)

# If namespace has been supplied, select by that
if namespace:
if namespaces:
select = select.where(
db.module_provider.c.namespace == namespace
db.module_provider.c.namespace.in_(namespaces)
)

# If namespace has been supplied, select by that
if module:
if modules:
select = select.where(
db.module_provider.c.module == module
db.module_provider.c.module.in_(modules)
)

# Filter by verified modules, if requested
Expand Down
19 changes: 11 additions & 8 deletions terrareg/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -897,7 +897,8 @@ def _get(self):
)
parser.add_argument(
'provider', type=str,
default=None, help='Limits modules to a specific provider.'
default=None, help='Limits modules to a specific provider.',
action='append', dest='providers'
)
parser.add_argument(
'verified', type=inputs.boolean,
Expand All @@ -907,7 +908,7 @@ def _get(self):
args = parser.parse_args()

search_results = ModuleSearch.search_module_providers(
provider=args.provider,
providers=args.providers,
verified=args.verified,
offset=args.offset,
limit=args.limit
Expand Down Expand Up @@ -941,11 +942,13 @@ def _get(self):
)
parser.add_argument(
'provider', type=str,
default=None, help='Limits modules to a specific provider.'
default=None, help='Limits modules to a specific provider.',
action='append', dest='providers'
)
parser.add_argument(
'namespace', type=str,
default=None, help='Limits modules to a specific namespace.'
default=None, help='Limits modules to a specific namespace.',
action='append', dest='namespaces'
)
parser.add_argument(
'verified', type=inputs.boolean,
Expand Down Expand Up @@ -977,8 +980,8 @@ def _get(self):

search_results = ModuleSearch.search_module_providers(
query=args.q,
namespace=args.namespace,
provider=args.provider,
namespaces=args.namespaces,
providers=args.providers,
verified=args.verified,
namespace_trust_filters=namespace_trust_filters,
offset=args.offset,
Expand Down Expand Up @@ -1013,8 +1016,8 @@ def _get(self, namespace, name):
search_results = ModuleSearch.search_module_providers(
offset=args.offset,
limit=args.limit,
namespace=namespace,
module=name
namespaces=[namespace],
modules=[name]
)

if not search_results.module_providers:
Expand Down
2 changes: 0 additions & 2 deletions terrareg/static/js/terrareg/search_result_card.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,8 +72,6 @@ function createSearchResultCard(parent_id, module, provider_logos) {

// Add provider TOS to results, if not already there
if ($('#provider-tos-' + module.provider).length == 0) {
console.log('ADding now');
console.log($('#provider-tos')[0]);
let tos_object = document.createElement('p');
tos_object.id = `provider-tos-${module.provider}`;
tos_object.innerHTML = provider_logo_details.tos;
Expand Down
55 changes: 53 additions & 2 deletions terrareg/templates/module_search.html
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@
<span id="search-contributed-count" class="tag"></span>
</a>
</nav>

<nav id="provider-filters" class="panel">
<p class="panel-heading">
Providers
</p>
</nav>
</div>
<div class="column is-three-fifths">

Expand Down Expand Up @@ -78,10 +84,24 @@

// Obtain map of all provider logos
provider_logos = {};
provider_filter_status = {};
$.get('/v1/terrareg/provider_logos', (data) => {
provider_logos = data;
})

function getFilterProviders() {
let providers = [];
// Iterate over provider filter checkboxes and generate
// query string
$('#provider-filters').find('input').each((index, element) => {
let j_element = $(element);
if (j_element.is(':checked')) {
providers.push(j_element.data('providerName'));
}
});
return providers;
}

let previousSearchString = '';
function performSearch(new_offset) {

Expand All @@ -104,15 +124,26 @@
}

// Clear any existing provider logo TOS
$('#provider-tos')[0].innerHTML = '';
// Check if element exists - on initial page load,
// the element will not be present
// on the page at this point.
if ($('#provider-tos')[0] !== undefined) {
$('#provider-tos')[0].innerHTML = '';
}

let query_string = '';
getFilterProviders().forEach((provider) => {
query_string += `&provider=${provider}`;
});

// Perform AJAX query to obtain results
$.get(`/v1/modules/search?` +
`q=${encodeURIComponent(searchQuery)}&` +
`offset=${new_offset}&` +
`verified=${$('#search-verified').is(':checked')}&` +
`trusted_namespaces=${$('#search-trusted-namespaces').is(':checked')}&` +
`contributed=${$('#search-contributed').is(':checked')}`,
`contributed=${$('#search-contributed').is(':checked')}` +
query_string,
function(data, status){
if (data.modules.length == 0) {
$('#results').html(`
Expand Down Expand Up @@ -147,6 +178,26 @@
$('#search-verified-count').html(data.verified);
$('#search-trusted-namespaces-count').html(data.trusted_namespaces);
$('#search-contributed-count').html(data.contributed);

let current_filtered_provider = getFilterProviders();

// Clear all providers
$('#provider-filters').find('a').remove();
let new_provider_filter_status = {};
Object.keys(data.providers).forEach((provider_name) => {

// Check if provider already exists and is checked
let checked = current_filtered_provider.indexOf(provider_name) !== -1;

// Add providers to provider filter
$('#provider-filters').append(`
<a class="panel-block" onclick="toggleChildCheckbox(event);">
<input data-provider-name="${provider_name}" id="provider-filter-${provider_name}" onchange="performSearch();" ${checked ? 'checked' : ''} type="checkbox" />
${provider_name}
<span class="tag">${data.providers[provider_name]}</span>
</a>
`);
});
});

previousSearchString = searchQuery;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -206,7 +206,7 @@ def test_namespace_search_in_filter(self, namespace, expected_module_provider_id
result = ModuleSearch.search_module_providers(
offset=0, limit=50,
query='',
namespace=namespace
namespaces=[namespace]
)

resulting_module_provider_ids = [
Expand Down Expand Up @@ -277,7 +277,7 @@ def test_module_name_search_in_filter(self, module_name_search, expected_module_
result = ModuleSearch.search_module_providers(
offset=0, limit=50,
query='',
module=module_name_search
modules=[module_name_search]
)

resulting_module_provider_ids = [
Expand Down Expand Up @@ -344,7 +344,7 @@ def test_provider_name_search_in_filter(self, provider_name_search, expected_mod
result = ModuleSearch.search_module_providers(
offset=0, limit=50,
query='',
provider=provider_name_search
providers=[provider_name_search]
)

resulting_module_provider_ids = [
Expand Down
4 changes: 2 additions & 2 deletions test/unit/terrareg/server/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ def search_results_func(
offset: int,
limit: int,
query: str=None,
namespace: str=None,
provider: str=None,
namespaces: list=None,
providers: list=None,
verified: bool=False,
namespace_trust_filters: list=NamespaceTrustFilter.UNSPECIFIED):
return ModuleSearchResults(offset=offset, limit=limit, count=0, module_providers=[])
Expand Down
8 changes: 4 additions & 4 deletions test/unit/terrareg/server/test_api_module_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ def return_results(*args, **kwargs):
assert res.status_code == 200

mocked_search_module_providers.assert_called_once_with(
offset=0, limit=10, namespace='testnamespace', module='lonelymodule')
offset=0, limit=10, namespaces=['testnamespace'], modules=['lonelymodule'])

@setup_test_data()
def test_unverified_module(self, client, mocked_server_namespace_fixture,
Expand Down Expand Up @@ -78,7 +78,7 @@ def return_results(*args, **kwargs):
assert res.status_code == 200

mocked_search_module_providers.assert_called_once_with(
offset=0, limit=10, namespace='testnamespace', module='unverifiedmodule')
offset=0, limit=10, namespaces=['testnamespace'], modules=['unverifiedmodule'])

def test_non_existent_module(self, client, mocked_server_namespace_fixture,
mocked_search_module_providers):
Expand All @@ -99,7 +99,7 @@ def return_results(*args, **kwargs):
assert res.status_code == 404

mocked_search_module_providers.assert_called_once_with(
offset=0, limit=10, namespace='doesnotexist', module='unittestdoesnotexist')
offset=0, limit=10, namespaces=['doesnotexist'], modules=['unittestdoesnotexist'])

@setup_test_data()
def test_analytics_token(self, client, mocked_server_namespace_fixture,
Expand Down Expand Up @@ -141,5 +141,5 @@ def return_results(*args, **kwargs):
}
assert res.status_code == 200
mocked_search_module_providers.assert_called_once_with(
offset=0, limit=10, namespace='testnamespace', module='lonelymodule')
offset=0, limit=10, namespaces=['testnamespace'], modules=['lonelymodule'])

10 changes: 5 additions & 5 deletions test/unit/terrareg/server/test_api_module_list.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def test_with_no_params(self, client, mocked_search_module_providers):
'meta': {'current_offset': 0, 'limit': 10}, 'modules': []
}

ModuleSearch.search_module_providers.assert_called_with(provider=None, verified=False, offset=0, limit=10)
ModuleSearch.search_module_providers.assert_called_with(providers=None, verified=False, offset=0, limit=10)

def test_with_limit_offset(self, client, mocked_search_module_providers):
"""Call with limit and offset"""
Expand All @@ -31,7 +31,7 @@ def test_with_limit_offset(self, client, mocked_search_module_providers):
'meta': {'current_offset': 23, 'limit': 12, 'prev_offset': 11}, 'modules': []
}

ModuleSearch.search_module_providers.assert_called_with(provider=None, verified=False, offset=23, limit=12)
ModuleSearch.search_module_providers.assert_called_with(providers=None, verified=False, offset=23, limit=12)

def test_with_provider_filter(self, client, mocked_search_module_providers):
"""Call with provider limit"""
Expand All @@ -42,7 +42,7 @@ def test_with_provider_filter(self, client, mocked_search_module_providers):
'meta': {'current_offset': 0, 'limit': 10}, 'modules': []
}

ModuleSearch.search_module_providers.assert_called_with(provider='testprovider', verified=False, offset=0, limit=10)
ModuleSearch.search_module_providers.assert_called_with(providers=['testprovider'], verified=False, offset=0, limit=10)

def test_with_verified_false(self, client, mocked_search_module_providers):
"""Call with verified flag as false"""
Expand All @@ -52,7 +52,7 @@ def test_with_verified_false(self, client, mocked_search_module_providers):
assert res.json == {
'meta': {'current_offset': 0, 'limit': 10}, 'modules': []
}
ModuleSearch.search_module_providers.assert_called_with(provider=None, verified=False, offset=0, limit=10)
ModuleSearch.search_module_providers.assert_called_with(providers=None, verified=False, offset=0, limit=10)


def test_with_verified_true(self, client, mocked_search_module_providers):
Expand All @@ -63,7 +63,7 @@ def test_with_verified_true(self, client, mocked_search_module_providers):
assert res.json == {
'meta': {'current_offset': 0, 'limit': 10}, 'modules': []
}
ModuleSearch.search_module_providers.assert_called_with(provider=None, verified=True, offset=0, limit=10)
ModuleSearch.search_module_providers.assert_called_with(providers=None, verified=True, offset=0, limit=10)

@setup_test_data()
def test_with_module_response(self, client, mocked_search_module_providers):
Expand Down
Loading

0 comments on commit edda9f5

Please sign in to comment.