From 1d9a61132c27e43b39cc97eae46d93489b20ea76 Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 4 Sep 2024 09:48:11 +0200 Subject: [PATCH 1/4] add test for updating quota description --- test/integration/test_quota.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/integration/test_quota.py b/test/integration/test_quota.py index 07137786a353..07c4a0fea936 100644 --- a/test/integration/test_quota.py +++ b/test/integration/test_quota.py @@ -85,6 +85,27 @@ def test_update(self): json_response = show_response.json() assert json_response["name"] == new_quota_name + def test_update_description(self): + quota_name = "test-update-quota-description" + quota = self._create_quota_with_name(quota_name) + quota_id = quota["id"] + + # update description (one needs to specify a name even if should not be changed) + quota_description = "description of test-updated-quota-name" + update_payload = { + "name": quota_name, + "description": quota_description, + } + put_response = self._put(f"quotas/{quota_id}", data=update_payload, json=True) + put_response.raise_for_status() + assert "has been renamed to" in put_response.text + + show_response = self._get(f"quotas/{quota_id}") + show_response.raise_for_status() + json_response = show_response.json() + assert json_response["name"] == quota_name + assert json_response["description"] == quota_description + def test_delete(self): quota_name = "test-delete-quota" quota = self._create_quota_with_name(quota_name) From 0ed4b484bc9193c61b907ec7816eaf9cf3505bfe Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 4 Sep 2024 09:29:58 +0200 Subject: [PATCH 2/4] allow to change description of a quota A [quota update via API](https://github.com/galaxyproject/galaxy/blob/67b4a69c1b36eec4dc5871c5eeb97b6bdea8a288/lib/galaxy/webapps/galaxy/services/quotas.py#L85) requires name or quota to be set, but the [manager](https://github.com/galaxyproject/galaxy/blob/67b4a69c1b36eec4dc5871c5eeb97b6bdea8a288/lib/galaxy/managers/quotas.py#L119) requires a name to be set. If one wants to change just the description of a quota the used select statement gets the quota itself and the elif branch hits resulting in an exception. With the change one can change just the description, but one still needs to specify the name of the quota. We could change this such that also in the manager the name of the quota needs to be specified. --- lib/galaxy/managers/quotas.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/galaxy/managers/quotas.py b/lib/galaxy/managers/quotas.py index 31e7fcc48e6d..6cd74232c8ad 100644 --- a/lib/galaxy/managers/quotas.py +++ b/lib/galaxy/managers/quotas.py @@ -12,7 +12,10 @@ Union, ) -from sqlalchemy import select +from sqlalchemy import ( + and_, + select, +) from galaxy import ( model, @@ -115,7 +118,7 @@ def _parse_amount(self, amount: str) -> Optional[Union[int, bool]]: return False def rename_quota(self, quota, params) -> str: - stmt = select(Quota).where(Quota.name == params.name).limit(1) + stmt = select(Quota).where(and_(Quota.name == params.name, Quota.id != quota.id)).limit(1) if not params.name: raise ActionInputError("Enter a valid name.") elif params.name != quota.name and self.sa_session.scalars(stmt).first(): From 701f6ab6d3cb5070da7189746ba2d62ee15a484c Mon Sep 17 00:00:00 2001 From: Matthias Bernt Date: Wed, 18 Sep 2024 16:05:12 +0200 Subject: [PATCH 3/4] remove redundant check --- lib/galaxy/managers/quotas.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/managers/quotas.py b/lib/galaxy/managers/quotas.py index 6cd74232c8ad..8b91b94a94a8 100644 --- a/lib/galaxy/managers/quotas.py +++ b/lib/galaxy/managers/quotas.py @@ -121,7 +121,7 @@ def rename_quota(self, quota, params) -> str: stmt = select(Quota).where(and_(Quota.name == params.name, Quota.id != quota.id)).limit(1) if not params.name: raise ActionInputError("Enter a valid name.") - elif params.name != quota.name and self.sa_session.scalars(stmt).first(): + elif self.sa_session.scalars(stmt).first(): raise ActionInputError("A quota with that name already exists.") else: old_name = quota.name From fc9d2f1dc7d8272565ac02a4680452ade998f86e Mon Sep 17 00:00:00 2001 From: M Bernt Date: Mon, 16 Sep 2024 15:12:23 +0200 Subject: [PATCH 4/4] remove assert Co-authored-by: Marius van den Beek --- test/integration/test_quota.py | 1 - 1 file changed, 1 deletion(-) diff --git a/test/integration/test_quota.py b/test/integration/test_quota.py index 07c4a0fea936..0debde6ac627 100644 --- a/test/integration/test_quota.py +++ b/test/integration/test_quota.py @@ -98,7 +98,6 @@ def test_update_description(self): } put_response = self._put(f"quotas/{quota_id}", data=update_payload, json=True) put_response.raise_for_status() - assert "has been renamed to" in put_response.text show_response = self._get(f"quotas/{quota_id}") show_response.raise_for_status()