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

Add AIPs search (#335) #336

Merged
merged 1 commit into from
Aug 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions AIPscan/Reporter/templates/aips.html
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,12 @@

{% if storage_services %}

<div class="float-right">
<form action="{{ url_for('reporter.view_aips') }}" method="GET" class="pr-3" id="queryform">
<i class="fas fa-search"></i> Search <input name="query" id="query"/><input type="submit" hidden/>
</form>
</div>

<div class="report-opts" style="margin-bottom: 0.2em;">
<div>
<span class="fixed-width-menu-span-left">
Expand Down
79 changes: 63 additions & 16 deletions AIPscan/Reporter/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,26 +38,73 @@ class MockResponse(object):
assert response.status_code == return_response.status_code


@pytest.mark.parametrize("page,pager_page", [(1, 1), ("bad", 1)])
def test_get_aip_pager(app_instance, mocker, page, pager_page):
paginate_mock = mocker.Mock()
filter_by_mock = mocker.Mock()
filter_by_mock.paginate.return_value = paginate_mock
@pytest.mark.parametrize(
"query,page,pages,total,prev_num,next_num,set_location",
[
(None, 1, 2, 3, None, 2, False), # No search query
("Another Test AIP", 1, 1, 1, None, None, False), # Search matches one AIP
(None, 1, 1, 1, None, None, True), # Use non-default storage location
("Does Not Exist", 1, 0, 0, None, None, False), # Search doesn't match any AIP
("Test AIP", 1, 2, 3, None, 2, False), # Search matches all AIPs, page 1
("Test AIP", 2, 2, 3, 1, None, False), # Search matches all AIPs, page 2
],
)
def test_get_aip_pager(
app_instance,
mocker,
query,
page,
pages,
total,
prev_num,
next_num,
set_location,
):
storage_service = test_helpers.create_test_storage_service()
default_storage_location = test_helpers.create_test_storage_location(
default_storage_service_id=storage_service.id
)
other_storage_location = test_helpers.create_test_storage_location(
storage_service_id=storage_service.id,
current_location="/other/location",
)

query_mock = mocker.Mock()
query_mock.filter_by.return_value = filter_by_mock
test_helpers.create_test_aip(
transfer_name="Test AIP",
storage_service_id=storage_service.id,
storage_location_id=default_storage_location.id,
)
test_helpers.create_test_aip(
transfer_name="Another Test AIP",
storage_service_id=storage_service.id,
storage_location_id=default_storage_location.id,
)
test_helpers.create_test_aip(
transfer_name="This Is Also A Test AIP",
storage_service_id=storage_service.id,
storage_location_id=other_storage_location.id,
)

storage_service = test_helpers.create_test_storage_service()
storage_location = test_helpers.create_test_storage_location(
storage_service_id=storage_service.id
location = None
if set_location:
location = other_storage_location

pager = views.get_aip_pager(
page, 2, storage_service, storage_location=location, query=query
)
pager = views.get_aip_pager(page, 2, storage_service, storage_location)

assert pager.page == pager_page
assert pager.pages == 0
assert pager.per_page == 2
assert pager.prev_num is None
assert pager.next_num is None
assert pager.pages == pages
assert pager.total == total

if prev_num is None:
assert pager.prev_num is None
else:
assert pager.prev_num == prev_num

if next_num is None:
assert pager.next_num is None
else:
assert pager.next_num == next_num


@pytest.mark.parametrize("page,pager_page", [(1, 1), ("bad", 1)])
Expand Down
47 changes: 33 additions & 14 deletions AIPscan/Reporter/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -86,25 +86,36 @@
return [loc for loc in storage_locations if loc.aips]


def get_aip_pager(page, per_page, storage_service, storage_location):
try:
page = int(page)
except ValueError:
page = 1

def get_aip_pager(page, per_page, storage_service, **kwargs):
storage_service_id = None
if storage_service is not None:
storage_service_id = storage_service.id

pager = AIP.query.filter_by(storage_service_id=storage_service_id).paginate(
page=page, per_page=per_page, error_out=False
)
storage_location_id = None
if kwargs.get("storage_location", None):
storage_location_id = kwargs["storage_location"].id

if storage_location:
pager = pager.query.filter_by(storage_location_id=storage_location.id).paginate(
page=page, per_page=per_page, error_out=False
query = kwargs.get("query", None)

aips = AIP.query

# Filter by storage service
aips = aips.filter_by(storage_service_id=storage_service_id)

# Optionally filter by storage location
if storage_location_id:
aips = aips.filter_by(storage_location_id=storage_location_id)

# Optionally filter by text search
if query is not None:
aips = aips.filter(
AIP.uuid.like(f"%{query}%")
| AIP.transfer_name.like(f"%{query}%")
| AIP.create_date.like(f"%{query}%")
)
Comment on lines +111 to 115
Copy link
Member

Choose a reason for hiding this comment

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

This guarantees a full table scan, it won't be very efficient. I guess that's ok for now?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, this is just to get something in place to replace functionality that seemingly once existed (a client had made their own documentation of it with screenshots), yet weirdly doesn't seem to exist in the code (I scoured the commit history and could find any trade). Very weird!

If it's slow for them I might re-implement it with Typesense.


pager = aips.paginate(page=page, per_page=per_page, error_out=False)

return pager


Expand All @@ -131,8 +142,16 @@
except Exception as e:
print(e)

page = request.args.get(request_params.PAGE, default="1")
pager = get_aip_pager(page, 10, storage_service, storage_location)
query = request.args.get("query", "")

Check warning on line 145 in AIPscan/Reporter/views.py

View check run for this annotation

Codecov / codecov/patch

AIPscan/Reporter/views.py#L145

Added line #L145 was not covered by tests

try:
page = int(request.args.get(request_params.PAGE, default=1))
except ValueError:
page = 1

Check warning on line 150 in AIPscan/Reporter/views.py

View check run for this annotation

Codecov / codecov/patch

AIPscan/Reporter/views.py#L147-L150

Added lines #L147 - L150 were not covered by tests

pager = get_aip_pager(

Check warning on line 152 in AIPscan/Reporter/views.py

View check run for this annotation

Codecov / codecov/patch

AIPscan/Reporter/views.py#L152

Added line #L152 was not covered by tests
page, 10, storage_service, storage_location=storage_location, query=query
)

first_item, last_item = calculate_paging_window(pager)

Expand Down
11 changes: 11 additions & 0 deletions AIPscan/static/js/reporter/aips.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ $(document).ready(function () {
function reloadPage(ignoreLocation) {
var storageServiceId = $("#ss").val();
var storageLocationId = $("#sl").val();
var query = $("#query").val();

var url = new URL("reporter/aips", $("body").data("url-root"));
var params = {
Expand All @@ -15,6 +16,10 @@ $(document).ready(function () {
params["storage_location"] = storageLocationId;
}

if (query !== "") {
params["query"] = query;
}

url.search = new URLSearchParams(params).toString();
window.location.href = url.href;
}
Expand All @@ -26,4 +31,10 @@ $(document).ready(function () {
$("#sl").on("change", function () {
reloadPage(false);
});

$("#queryform").on("submit", function () {
reloadPage(false);

return false;
});
});
Loading