From 8992ec3875ced79919f51706256e865c54535401 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 3 Mar 2023 18:01:05 +0100 Subject: [PATCH 1/2] Do not pass dataset keyword parameter into datatype.display_data Fixes https://github.com/galaxyproject/galaxy/issues/15678. Krona generates an html dataset that includes a link with dataset as a query param. --- lib/galaxy/webapps/galaxy/api/datasets.py | 4 +++- lib/galaxy/webapps/galaxy/controllers/dataset.py | 2 ++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/lib/galaxy/webapps/galaxy/api/datasets.py b/lib/galaxy/webapps/galaxy/api/datasets.py index 6efb453f51e..bba89747c79 100644 --- a/lib/galaxy/webapps/galaxy/api/datasets.py +++ b/lib/galaxy/webapps/galaxy/api/datasets.py @@ -258,7 +258,9 @@ class FastAPIDatasets: ), ): """Streams the dataset for download or the contents preview to be displayed in a browser.""" - extra_params = get_query_parameters_from_request_excluding(request, {"preview", "filename", "to_ext", "raw"}) + extra_params = get_query_parameters_from_request_excluding( + request, {"preview", "filename", "to_ext", "raw", "dataset"} + ) display_data, headers = self.service.display( trans, history_content_id, preview=preview, filename=filename, to_ext=to_ext, raw=raw, **extra_params ) diff --git a/lib/galaxy/webapps/galaxy/controllers/dataset.py b/lib/galaxy/webapps/galaxy/controllers/dataset.py index fe1578907b9..f7f857951f4 100644 --- a/lib/galaxy/webapps/galaxy/controllers/dataset.py +++ b/lib/galaxy/webapps/galaxy/controllers/dataset.py @@ -210,6 +210,8 @@ class DatasetInterface(BaseUIController, UsesAnnotations, UsesItemRatings, UsesE # Ensure ck_size is an integer before passing through to datatypes. if ck_size: ck_size = int(ck_size) + kwd.pop("dataset", None) + # `dataset` in kwd` would interfere with positional dataset argument of `display_data` method. display_data, headers = data.datatype.display_data( trans, data, preview, filename, to_ext, offset=offset, ck_size=ck_size, **kwd ) From d97adf635924476e77958cccea37da77de507450 Mon Sep 17 00:00:00 2001 From: mvdbeek Date: Fri, 3 Mar 2023 18:35:15 +0100 Subject: [PATCH 2/2] Fix slugified dataset display Fixes https://github.com/galaxyproject/galaxy/issues/15676. --- lib/galaxy/actions/library.py | 2 +- lib/galaxy/managers/hdas.py | 4 ++-- lib/galaxy/model/__init__.py | 9 +++++++-- lib/galaxy/tools/__init__.py | 4 ++-- lib/galaxy/webapps/base/controller.py | 2 +- lib/galaxy/webapps/galaxy/controllers/dataset.py | 12 +++++------- lib/galaxy/webapps/galaxy/services/datasets.py | 2 +- scripts/cleanup_datasets/cleanup_datasets.py | 2 +- templates/display_base.mako | 6 ++---- templates/display_common.mako | 5 +---- templates/webapps/galaxy/dataset/display.mako | 6 +++--- templates/webapps/reports/dataset_info.mako | 2 +- test/unit/app/managers/test_HDAManager.py | 2 +- 13 files changed, 28 insertions(+), 30 deletions(-) diff --git a/lib/galaxy/actions/library.py b/lib/galaxy/actions/library.py index 6a184210d9d..26543f2a463 100644 --- a/lib/galaxy/actions/library.py +++ b/lib/galaxy/actions/library.py @@ -333,7 +333,7 @@ class LibraryActions: raise ObjectNotFound(message) elif ( not trans.app.security_agent.can_access_dataset(current_user_roles, item.dataset) - and item.history.user == trans.user + and item.user == trans.user ): message = f"You do not have permission to access the history dataset with id ({str(item.id)})." raise ItemAccessibilityException(message) diff --git a/lib/galaxy/managers/hdas.py b/lib/galaxy/managers/hdas.py index 7abbee7f3b7..905602aed95 100644 --- a/lib/galaxy/managers/hdas.py +++ b/lib/galaxy/managers/hdas.py @@ -182,7 +182,7 @@ class HDAManager( copy.set_size() original_annotation = self.annotation(hda) - self.annotate(copy, original_annotation, user=hda.history.user, flush=False) + self.annotate(copy, original_annotation, user=hda.user, flush=False) if flush: if history: history.add_pending_items() @@ -292,7 +292,7 @@ class HDAManager( # .... annotatable def annotation(self, hda): # override to scope to history owner - return self._user_annotation(hda, hda.history.user) + return self._user_annotation(hda, hda.user) def _set_permissions(self, trans, hda, role_ids_dict): # The user associated the DATASET_ACCESS permission on the dataset with 1 or more roles. We diff --git a/lib/galaxy/model/__init__.py b/lib/galaxy/model/__init__.py index b9ab021d18c..946c56af1aa 100644 --- a/lib/galaxy/model/__init__.py +++ b/lib/galaxy/model/__init__.py @@ -4492,6 +4492,11 @@ class HistoryDatasetAssociation(DatasetInstance, HasTags, Dictifiable, UsesAnnot self.copied_from_history_dataset_association = copied_from_history_dataset_association self.copied_from_library_dataset_dataset_association = copied_from_library_dataset_dataset_association + @property + def user(self): + if self.history: + return self.history.user + def __create_version__(self, session): state = inspect(self) changes = {} @@ -4539,7 +4544,7 @@ class HistoryDatasetAssociation(DatasetInstance, HasTags, Dictifiable, UsesAnnot self.validated_state = other_hda.validated_state self.validated_state_message = other_hda.validated_state_message if include_tags and self.history: - self.copy_tags_from(self.history.user, other_hda) + self.copy_tags_from(self.user, other_hda) self.dataset = new_dataset or other_hda.dataset if old_dataset: old_dataset.full_delete() @@ -4687,7 +4692,7 @@ class HistoryDatasetAssociation(DatasetInstance, HasTags, Dictifiable, UsesAnnot for hda in self.dataset.history_associations: if hda.id == self.id: continue - if not hda.purged and hda.history and hda.history.user and hda.history.user == user: + if not hda.purged and hda.history and hda.user and hda.user == user: break else: rval += self.get_total_size() diff --git a/lib/galaxy/tools/__init__.py b/lib/galaxy/tools/__init__.py index f39e9a214f0..a27c6ea904b 100644 --- a/lib/galaxy/tools/__init__.py +++ b/lib/galaxy/tools/__init__.py @@ -2152,8 +2152,8 @@ class Tool(Dictifiable): for p_name in rup_dict: redirect_url += f"&{p_name}={rup_dict[p_name]}" # Add the current user email to redirect_url - if data.history.user: - USERNAME = str(data.history.user.email) + if data.user: + USERNAME = str(data.user.email) else: USERNAME = "Anonymous" redirect_url += f"&USERNAME={USERNAME}" diff --git a/lib/galaxy/webapps/base/controller.py b/lib/galaxy/webapps/base/controller.py index cfdaa71ff1c..4dd2d399bbc 100644 --- a/lib/galaxy/webapps/base/controller.py +++ b/lib/galaxy/webapps/base/controller.py @@ -1077,7 +1077,7 @@ class UsesVisualizationMixin(UsesLibraryMixinItems): user = trans.get_user() if not user: error("Must be logged in to manage Galaxy items") - if data.history.user != user: + if data.user != user: error(f"{data.__class__.__name__} is not owned by current user") if check_accessible: diff --git a/lib/galaxy/webapps/galaxy/controllers/dataset.py b/lib/galaxy/webapps/galaxy/controllers/dataset.py index f7f857951f4..cfaaa617e04 100644 --- a/lib/galaxy/webapps/galaxy/controllers/dataset.py +++ b/lib/galaxy/webapps/galaxy/controllers/dataset.py @@ -153,7 +153,7 @@ class DatasetInterface(BaseUIController, UsesAnnotations, UsesItemRatings, UsesE return trans.show_error_message("You are not allowed to access this dataset") if data.purged or data.dataset.purged: return trans.show_error_message("The dataset you are attempting to view has been purged.") - elif data.deleted and not (trans.user_is_admin or (data.history and trans.get_user() == data.history.user)): + elif data.deleted and not (trans.user_is_admin or (data.history and trans.get_user() == data.user)): return trans.show_error_message("The dataset you are attempting to view has been deleted.") elif data.state == Dataset.states.UPLOAD: return trans.show_error_message( @@ -211,7 +211,7 @@ class DatasetInterface(BaseUIController, UsesAnnotations, UsesItemRatings, UsesE if ck_size: ck_size = int(ck_size) kwd.pop("dataset", None) - # `dataset` in kwd` would interfere with positional dataset argument of `display_data` method. + # `dataset` in kwd would interfere with positional dataset argument of `display_data` method. display_data, headers = data.datatype.display_data( trans, data, preview, filename, to_ext, offset=offset, ck_size=ck_size, **kwd ) @@ -490,7 +490,7 @@ class DatasetInterface(BaseUIController, UsesAnnotations, UsesItemRatings, UsesE if data is None: trans.log_event(f"Problem retrieving dataset id ({dataset_id}).") return None, self.message_exception(trans, "The dataset id is invalid.") - if dataset_id is not None and data.history.user is not None and data.history.user != trans.user: + if dataset_id is not None and data.user and data.user != trans.user: trans.log_event(f"User attempted to edit a dataset they do not own (encoded: {dataset_id}, decoded: {id}).") return None, self.message_exception(trans, "The dataset id is invalid.") if data.history.user and not data.dataset.has_manage_permissions_roles(trans.app.security_agent): @@ -500,9 +500,7 @@ class DatasetInterface(BaseUIController, UsesAnnotations, UsesItemRatings, UsesE manage_permissions_action = trans.app.security_agent.get_action( trans.app.security_agent.permitted_actions.DATASET_MANAGE_PERMISSIONS.action ) - permissions = { - manage_permissions_action: [trans.app.security_agent.get_private_user_role(data.history.user)] - } + permissions = {manage_permissions_action: [trans.app.security_agent.get_private_user_role(data.user)]} trans.app.security_agent.set_dataset_permission(data.dataset, permissions) return data, None @@ -573,7 +571,7 @@ class DatasetInterface(BaseUIController, UsesAnnotations, UsesItemRatings, UsesE return self.display(trans, dataset_id=slug, filename=filename) truncated, dataset_data = self.hda_manager.text_data(dataset, preview) - dataset.annotation = self.get_item_annotation_str(trans.sa_session, dataset.history.user, dataset) + dataset.annotation = self.get_item_annotation_str(trans.sa_session, dataset.user, dataset) # If dataset is chunkable, get first chunk. first_chunk = None diff --git a/lib/galaxy/webapps/galaxy/services/datasets.py b/lib/galaxy/webapps/galaxy/services/datasets.py index 20182007816..f3aadcb1d73 100644 --- a/lib/galaxy/webapps/galaxy/services/datasets.py +++ b/lib/galaxy/webapps/galaxy/services/datasets.py @@ -573,7 +573,7 @@ class DatasetsService(ServiceBase, UsesVisualizationMixin): item_url = web.url_for( controller="dataset", action="display_by_username_and_slug", - username=hda.history.user.username, + username=hda.user and hda.user.username, slug=self.encode_id(hda.id), preview=False, ) diff --git a/scripts/cleanup_datasets/cleanup_datasets.py b/scripts/cleanup_datasets/cleanup_datasets.py index 8d5834b3b4f..eee06b243b5 100755 --- a/scripts/cleanup_datasets/cleanup_datasets.py +++ b/scripts/cleanup_datasets/cleanup_datasets.py @@ -624,7 +624,7 @@ def _purge_dataset(app, dataset, remove_from_disk, info_only=False): for hda in dataset.history_associations: if not hda.purged: hda.purged = True - if hda.history.user is not None and hda.history.user not in usage_users: + if hda.user and hda.user not in usage_users: usage_users.append(hda.history.user) for user in usage_users: user.adjust_total_disk_usage(-dataset.get_total_size()) diff --git a/templates/display_base.mako b/templates/display_base.mako index e0a6d5e2403..bd07e619a17 100644 --- a/templates/display_base.mako +++ b/templates/display_base.mako @@ -101,8 +101,7 @@ modern_route = modern_route_for_controller(get_controller_name(item)) item_plural = get_item_plural( item ) href_to_all_items = h.url_for( controller='/' + modern_route, action='list_published') - href_to_user_items = h.url_for( controller='/' + modern_route, action='list_published', xxx=item.user.username) - href_to_user_items = href_to_user_items.replace( 'xxx', 'f-username') + href_to_user_items = h.url_for( controller='/' + modern_route, action='list_published', **{"f-username": item.user and item.user.username}) %>
@@ -118,8 +117,7 @@ modern_route = modern_route_for_controller(get_controller_name(item)) item_plural = get_item_plural( item ) href_to_all_items = h.url_for( controller='/' + modern_route , action='list_published') - href_to_user_items = h.url_for( controller='/' + modern_route, action='list_published', xxx=item.user.username) - href_to_user_items = href_to_user_items.replace( 'xxx', 'f-username') + href_to_user_items = h.url_for( controller='/' + modern_route, action='list_published', **{"f-username": item.user and item.user.username}) %>
diff --git a/templates/display_common.mako b/templates/display_common.mako index ee5aadb1485..f449c0c0858 100644 --- a/templates/display_common.mako +++ b/templates/display_common.mako @@ -125,10 +125,7 @@ <%def name="get_item_user( item )"> <% # Exceptions first, default last. - if isinstance( item, model.HistoryDatasetAssociation ): - return item.history.user - else: - return item.user + return item.user %> diff --git a/templates/webapps/galaxy/dataset/display.mako b/templates/webapps/galaxy/dataset/display.mako index fb174b6fbd1..d389836b40f 100644 --- a/templates/webapps/galaxy/dataset/display.mako +++ b/templates/webapps/galaxy/dataset/display.mako @@ -66,7 +66,7 @@ %if truncated:
This dataset is large and only the first megabyte is shown below. | - Show all + Show all
%endif ## TODO: why is the default font size so small? @@ -126,9 +126,9 @@

Author

-

${item.history.user.username | h}

+

${item.user.username | h}

-
+
## Page meta. diff --git a/templates/webapps/reports/dataset_info.mako b/templates/webapps/reports/dataset_info.mako index 9023f0a9604..42865d3d7b5 100644 --- a/templates/webapps/reports/dataset_info.mako +++ b/templates/webapps/reports/dataset_info.mako @@ -61,7 +61,7 @@ ${time_ago( hda.update_time )} %if hda.history and hda.history.user: - ${hda.history.user.email} + ${hda.user.email} %else: anonymous %endif diff --git a/test/unit/app/managers/test_HDAManager.py b/test/unit/app/managers/test_HDAManager.py index 2bbdf1b2ba4..94ebfbb0b4a 100644 --- a/test/unit/app/managers/test_HDAManager.py +++ b/test/unit/app/managers/test_HDAManager.py @@ -480,7 +480,7 @@ class TestHDASerializer(HDATestCase): def test_file_name_serializers(self): hda = self._create_vanilla_hda() - owner = hda.history.user + owner = hda.user keys = ["file_name"] self.log("file_name should be included if app configured to do so")