-
Notifications
You must be signed in to change notification settings - Fork 44
feat(webportal): Add export to Web Portal function #7994
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
base: main
Are you sure you want to change the base?
Changes from all commits
25612c2
ee80b32
93bacdb
7969550
f60e0d1
f3e181a
0a83119
a2bbdf8
c204bd0
8fad6e4
5642fe4
158b0bc
8d16a3b
f8ea2d9
82e26b1
e05f02d
356cd9b
22e4cc0
540a55d
4c29e59
558cf4b
7141458
14d4711
5c6459f
7956b26
87c70a5
f742c10
51730a4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| from unittest.mock import MagicMock, Mock, patch | ||
|
|
||
| from django.test import Client | ||
|
|
||
| from specifyweb.backend.stored_queries.tests.tests import SQLAlchemySetup | ||
|
|
||
| from .raw_query import get_simple_query | ||
|
|
||
|
|
||
| class TestExportWebPortal(SQLAlchemySetup): | ||
| @patch("specifyweb.backend.stored_queries.views.Thread") | ||
| def test_export(self, thread: Mock): | ||
| c = Client() | ||
| c.force_login(self.specifyuser) | ||
|
|
||
| response = c.post( | ||
| "/stored_query/exportwebportal/", | ||
| get_simple_query(self.specifyuser), | ||
| content_type="application/json", | ||
| ) | ||
|
|
||
| self._assertStatusCodeEqual(response, 200) | ||
| thread.assert_called_once() | ||
| self.assertTrue(thread.return_value.daemon) | ||
| thread.return_value.start.assert_called_once() | ||
| self._assertContentEqual(response, "OK") | ||
|
|
||
| def test_portal_attachment_map(self): | ||
| from specifyweb.backend.stored_queries import execution | ||
|
|
||
| class FakeAttachment: | ||
| id = 5291 | ||
| attachmentlocation = "sp6896513492722436219.att.JPG" | ||
| origfilename = "29432.JPG" | ||
| title = "Figure 1" | ||
|
|
||
| class FakeJoinRecord: | ||
| collectionobject_id = 123 | ||
| attachment = FakeAttachment() | ||
|
|
||
| class FakeJoinQuery: | ||
| def select_related(self, *_args, **_kwargs): | ||
| return [FakeJoinRecord()] | ||
|
|
||
| class FakeJoinManager: | ||
| def __init__(self): | ||
| self.filter_kwargs = None | ||
|
|
||
| def filter(self, **kwargs): | ||
| self.filter_kwargs = kwargs | ||
| return FakeJoinQuery() | ||
|
|
||
| fake_join_manager = FakeJoinManager() | ||
| fake_base_model = type("Collectionobject", (), {"_meta": MagicMock(app_label="specifyweb")}) | ||
| fake_table = MagicMock() | ||
| fake_table.attachments_field = MagicMock() | ||
|
|
||
| with patch.object(execution.datamodel, "get_table_by_id", return_value=fake_table), patch.object( | ||
| execution, "get_model_by_table_id", return_value=fake_base_model | ||
| ), patch.object(execution.apps, "get_model", return_value=type("Collectionobjectattachment", (), {"objects": fake_join_manager})): | ||
| result = execution._portal_attachment_map(1, [123]) | ||
|
|
||
| self.assertEqual( | ||
| fake_join_manager.filter_kwargs, | ||
| {"collectionobject_id__in": [123], "attachment__ispublic": True}, | ||
| ) | ||
| self.assertEqual( | ||
| result["123"], | ||
| '[{AttachmentID:5291,AttachmentLocation:"sp6896513492722436219.att.JPG",Title:"Figure 1"}]', | ||
| ) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,6 +34,7 @@ class QueryBuilderPt(PermissionTarget): | |
| execute = PermissionTargetAction() | ||
| export_csv = PermissionTargetAction() | ||
| export_kml = PermissionTargetAction() | ||
| export_to_web_portal = PermissionTargetAction() | ||
| create_recordset = PermissionTargetAction() | ||
|
|
||
| def value_from_request(field, get): | ||
|
|
@@ -150,8 +151,6 @@ def batch_edit(request): | |
| @never_cache | ||
| def export_csv(request): | ||
| """Executes and return as CSV the results of the query provided as JSON in the POST body.""" | ||
| check_permission_targets(request.specify_collection.id, request.specify_user.id, [ | ||
| QueryBuilderPt.execute, QueryBuilderPt.export_csv]) | ||
| try: | ||
| spquery = json.load(request) | ||
| except ValueError as e: | ||
|
|
@@ -164,6 +163,9 @@ def export_csv(request): | |
| logger.debug('forcing collection to %s', collection.collectionname) | ||
| else: | ||
| collection = request.specify_collection | ||
|
|
||
| check_permission_targets(collection.id, request.specify_user.id, [ | ||
| QueryBuilderPt.execute, QueryBuilderPt.export_csv]) | ||
|
Comment on lines
+167
to
+168
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Conceptually, aren't we exporting the CSV in the collection the user is signed into, and running the query in the Collection specified by the query? Say the user was signed into Collection A and exported a Query that was run in Collection B. This is definitely a question regarding the scope of the permission. I can see arguments for all three sides:
From a strictly data-origin standpoint, I think it probably makes the most sense to keep this as-is: that is, scope the permission check to the Collection in which the query is scoped to. |
||
|
|
||
| file_name = format_export_file_name(spquery, "csv") | ||
|
|
||
|
|
@@ -177,8 +179,6 @@ def export_csv(request): | |
| @never_cache | ||
| def export_kml(request): | ||
| """Executes and return as KML the results of the query provided as JSON in the POST body.""" | ||
| check_permission_targets(request.specify_collection.id, request.specify_user.id, [ | ||
| QueryBuilderPt.execute, QueryBuilderPt.export_kml]) | ||
| try: | ||
| spquery = json.load(request) | ||
| except ValueError as e: | ||
|
|
@@ -195,13 +195,50 @@ def export_kml(request): | |
| else: | ||
| collection = request.specify_collection | ||
|
|
||
| check_permission_targets(collection.id, request.specify_user.id, [ | ||
| QueryBuilderPt.execute, QueryBuilderPt.export_kml]) | ||
|
|
||
| file_name = format_export_file_name(spquery, "kml") | ||
|
|
||
| thread = Thread(target=do_export, args=(spquery, collection, request.specify_user, file_name, 'kml', the_host)) | ||
| thread.daemon = True | ||
| thread.start() | ||
| return HttpResponse('OK', content_type='text/plain') | ||
|
|
||
|
|
||
| @require_POST | ||
| @login_maybe_required | ||
| @never_cache | ||
| def export_to_web_portal(request): | ||
| """Executes and returns as ZIP the web portal export package for the query provided as JSON in the POST body.""" | ||
| try: | ||
| spquery = json.load(request) | ||
| except ValueError as e: | ||
| return HttpResponseBadRequest(e) | ||
|
|
||
| logger.info('export web portal query: %s', spquery) | ||
|
|
||
| if 'collectionid' in spquery: | ||
| collection = Collection.objects.get(pk=spquery['collectionid']) | ||
| logger.debug('forcing collection to %s', collection.collectionname) | ||
| else: | ||
| collection = request.specify_collection | ||
|
|
||
| check_permission_targets(collection.id, request.specify_user.id, [ | ||
| QueryBuilderPt.execute, | ||
| QueryBuilderPt.export_to_web_portal, | ||
| ]) | ||
|
|
||
| file_name = format_export_file_name(spquery, 'zip') | ||
|
|
||
| thread = Thread( | ||
| target=do_export, | ||
| args=(spquery, collection, request.specify_user, file_name, 'webportal', None), | ||
| ) | ||
| thread.daemon = True | ||
| thread.start() | ||
|
Comment on lines
+234
to
+239
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (optional, performance) I know we've been using threads for Query exports (CSV, KML, and now WebPortal). Ideally, these potentially very memory-intensive operations should be performed in a separate process. That way, as soon as the operation is completed the allocated memory will be returned to the operating system and reused (there is some memory overhead when spawning a new process, which for Specify will largely be ~82 MiB for its setup and global structures as of
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @melton-jason Maybe I can open a new issue for this– what is your suggestion for implementation? |
||
| return HttpResponse('OK', content_type='text/plain') | ||
|
|
||
| @require_POST | ||
| @login_maybe_required | ||
| @never_cache | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.