mirror of
https://github.com/galaxyproject/galaxy.git
synced 2026-09-01 15:37:32 +08:00
Merge pull request #21599 from arash77/tool-not-found
[25.1] Fix loading of credentials when associated tools are missing
This commit is contained in:
@@ -18,7 +18,7 @@
|
||||
* <ServiceCredentialsGroupsList :service-groups="groups" />
|
||||
*/
|
||||
|
||||
import { faKey, faPencilAlt, faTrash, faWrench } from "@fortawesome/free-solid-svg-icons";
|
||||
import { faExclamationTriangle, faKey, faPencilAlt, faTrash, faWrench } from "@fortawesome/free-solid-svg-icons";
|
||||
import { BModal } from "bootstrap-vue";
|
||||
import { faCheck } from "font-awesome-6";
|
||||
import { storeToRefs } from "pinia";
|
||||
@@ -71,7 +71,7 @@ const props = defineProps<Props>();
|
||||
|
||||
const { confirm } = useConfirmDialog();
|
||||
|
||||
const { getToolNameById } = useToolStore();
|
||||
const { getToolForId, getToolNameById } = useToolStore();
|
||||
|
||||
const userToolsServiceCredentialsStore = useUserToolsServiceCredentialsStore();
|
||||
const { userToolsServicesCurrentGroupIds } = storeToRefs(userToolsServiceCredentialsStore);
|
||||
@@ -95,12 +95,25 @@ const cardTitle = computed(() => (group: ServiceCredentialsGroupDetails) => {
|
||||
return `${group.serviceDefinition.name} (v${group.serviceDefinition.version}) - ${group.name}`;
|
||||
});
|
||||
|
||||
/**
|
||||
* Checks if the source tool for a credential group is missing/deleted.
|
||||
* @param {ServiceCredentialsGroupDetails} group - The credential group to check.
|
||||
* @returns {boolean} True if the tool is no longer available.
|
||||
*/
|
||||
const isToolMissing = computed(() => (group: ServiceCredentialsGroupDetails) => {
|
||||
return !getToolForId(group.sourceId);
|
||||
});
|
||||
|
||||
/**
|
||||
* Checks if a credential group is currently in use by any tool.
|
||||
* @param {ServiceCredentialsGroupDetails} group - The credential group to check.
|
||||
* @returns {boolean} True if the group is in use.
|
||||
*/
|
||||
const isGroupInUse = computed(() => (group: ServiceCredentialsGroupDetails) => {
|
||||
if (isToolMissing.value(group)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
const userToolKey = userToolsServiceCredentialsStore.getUserToolKey(group.sourceId, group.sourceVersion);
|
||||
const userToolService = userToolsServicesCurrentGroupIds.value[userToolKey];
|
||||
for (const groupId of Object.values(userToolService || {})) {
|
||||
@@ -111,6 +124,18 @@ const isGroupInUse = computed(() => (group: ServiceCredentialsGroupDetails) => {
|
||||
return false;
|
||||
});
|
||||
|
||||
/**
|
||||
* Gets the display name for a tool, with a fallback for missing/deleted tools.
|
||||
* @param {ServiceCredentialsGroupDetails} group - The credential group.
|
||||
* @returns {string} The tool name or a fallback indicator.
|
||||
*/
|
||||
const getToolDisplayName = computed(() => (group: ServiceCredentialsGroupDetails) => {
|
||||
if (isToolMissing.value(group)) {
|
||||
return `${group.sourceId} (deleted)`;
|
||||
}
|
||||
return getToolNameById(group.sourceId);
|
||||
});
|
||||
|
||||
/**
|
||||
* Deletes a credential group after user confirmation.
|
||||
* @param {ServiceCredentialsGroupDetails} groupToDelete - The group to delete.
|
||||
@@ -120,7 +145,9 @@ const isGroupInUse = computed(() => (group: ServiceCredentialsGroupDetails) => {
|
||||
async function deleteGroup(groupToDelete: ServiceCredentialsGroupDetails): Promise<void> {
|
||||
let message = `Are you sure you want to delete the credentials group "${groupToDelete.name}"?`;
|
||||
|
||||
if (isGroupInUse.value(groupToDelete)) {
|
||||
if (isToolMissing.value(groupToDelete)) {
|
||||
message = message.concat(` The associated tool is no longer available.`);
|
||||
} else if (isGroupInUse.value(groupToDelete)) {
|
||||
message = message.concat(` This group is currently in use by '${getToolNameById(groupToDelete.sourceId)}'.`);
|
||||
}
|
||||
|
||||
@@ -221,23 +248,40 @@ async function onSaveChanges(): Promise<void> {
|
||||
* @returns {CardBadge[]} Array of badge configurations.
|
||||
*/
|
||||
function getBadgesFor(group: ServiceCredentialsGroupDetails): CardBadge[] {
|
||||
const badges: CardBadge[] = [
|
||||
{
|
||||
id: `tool-${group.sourceId}`,
|
||||
icon: faWrench,
|
||||
title: "This tool is using this credentials group. Click to view.",
|
||||
label: getToolNameById(group.sourceId),
|
||||
to: `/root?tool_id=${group.sourceId}&tool_version=${group.sourceVersion}`,
|
||||
},
|
||||
{
|
||||
const toolMissing = isToolMissing.value(group);
|
||||
const badges: CardBadge[] = [];
|
||||
|
||||
if (toolMissing) {
|
||||
badges.push({
|
||||
id: `tool-missing-${group.id}`,
|
||||
icon: faExclamationTriangle,
|
||||
title: "The tool associated with these credentials is no longer available. You cannot edit or use this group.",
|
||||
label: "Tool Unavailable",
|
||||
variant: "warning",
|
||||
});
|
||||
}
|
||||
|
||||
badges.push({
|
||||
id: `tool-${group.sourceId}`,
|
||||
icon: faWrench,
|
||||
title: toolMissing
|
||||
? "This tool is no longer available."
|
||||
: "This tool is using this credentials group. Click to view.",
|
||||
label: getToolDisplayName.value(group),
|
||||
to: toolMissing ? undefined : `/root?tool_id=${group.sourceId}&tool_version=${group.sourceVersion}`,
|
||||
});
|
||||
|
||||
if (!toolMissing) {
|
||||
badges.push({
|
||||
id: `in-use-${group.id}`,
|
||||
icon: faCheck,
|
||||
title: "This group is currently in use.",
|
||||
label: "In Use",
|
||||
variant: "success",
|
||||
visible: isGroupInUse.value(group),
|
||||
},
|
||||
];
|
||||
});
|
||||
}
|
||||
|
||||
return badges;
|
||||
}
|
||||
|
||||
@@ -247,6 +291,7 @@ function getBadgesFor(group: ServiceCredentialsGroupDetails): CardBadge[] {
|
||||
* @returns {CardAction[]} Array of action configurations
|
||||
*/
|
||||
function getPrimaryActions(group: ServiceCredentialsGroupDetails): CardAction[] {
|
||||
const toolMissing = isToolMissing.value(group);
|
||||
const primaryActions: CardAction[] = [
|
||||
{
|
||||
id: `delete-${group.id}`,
|
||||
@@ -259,10 +304,11 @@ function getPrimaryActions(group: ServiceCredentialsGroupDetails): CardAction[]
|
||||
{
|
||||
id: `edit-${group.id}`,
|
||||
label: "Edit",
|
||||
title: "Edit this group",
|
||||
title: !toolMissing ? "Cannot edit - tool definition not available" : "Edit this group",
|
||||
icon: faPencilAlt,
|
||||
variant: "outline-info",
|
||||
handler: () => editGroup(group),
|
||||
disabled: toolMissing,
|
||||
},
|
||||
];
|
||||
return primaryActions;
|
||||
|
||||
@@ -272,14 +272,30 @@ class CredentialsService:
|
||||
|
||||
for user_credentials, credentials_group, credential in existing_user_credentials:
|
||||
cred_id = user_credentials.id
|
||||
definition = self._get_credentials_definition(
|
||||
user,
|
||||
cast(SOURCE_TYPE, user_credentials.source_type),
|
||||
user_credentials.source_id,
|
||||
user_credentials.source_version,
|
||||
user_credentials.name,
|
||||
user_credentials.version,
|
||||
)
|
||||
definition = None
|
||||
try:
|
||||
definition = self._get_credentials_definition(
|
||||
user,
|
||||
cast(SOURCE_TYPE, user_credentials.source_type),
|
||||
user_credentials.source_id,
|
||||
user_credentials.source_version,
|
||||
user_credentials.name,
|
||||
user_credentials.version,
|
||||
)
|
||||
except ObjectNotFound:
|
||||
# Tool was removed or is no longer available - create a minimal fallback definition
|
||||
# using the stored credential data so the UI can still display the credentials
|
||||
if include_definition:
|
||||
definition = CredentialsRequirement(
|
||||
name=user_credentials.name,
|
||||
version=user_credentials.version,
|
||||
description="",
|
||||
label="",
|
||||
optional=False,
|
||||
variables=[],
|
||||
secrets=[],
|
||||
)
|
||||
|
||||
user_credentials_dict.setdefault(
|
||||
cred_id,
|
||||
{
|
||||
@@ -295,7 +311,7 @@ class CredentialsService:
|
||||
},
|
||||
)
|
||||
|
||||
if include_definition:
|
||||
if include_definition and definition:
|
||||
user_credentials_dict[cred_id]["definition"] = {
|
||||
"name": definition.name,
|
||||
"version": definition.version,
|
||||
|
||||
@@ -458,6 +458,73 @@ class TestCredentialsApi(integration_util.IntegrationTestCase, integration_util.
|
||||
vault_ref = self._get_vault_ref(payload, group["id"], secret["name"])
|
||||
self._check_vault_entry_exists(test_user_email, vault_ref, should_exist=False)
|
||||
|
||||
@skip_without_tool(CREDENTIALS_TEST_TOOL)
|
||||
def test_list_credentials_with_missing_tool(self):
|
||||
# Create credentials for the test tool
|
||||
payload = self._build_credentials_payload()
|
||||
self._provide_user_credentials(payload)
|
||||
|
||||
# Verify credentials exist normally
|
||||
credentials_list = self._check_credentials_exist()
|
||||
assert len(credentials_list) == 1
|
||||
user_credentials_id = credentials_list[0]["id"]
|
||||
|
||||
# Save the tool reference before removing it
|
||||
tool = self._app.toolbox.get_tool(CREDENTIALS_TEST_TOOL)
|
||||
assert tool is not None, f"Tool {CREDENTIALS_TEST_TOOL} should be available before removal"
|
||||
|
||||
try:
|
||||
# Remove the tool to simulate it being unavailable
|
||||
# Use remove_from_panel=False to keep restoration simple
|
||||
self._app.toolbox.remove_tool_by_id(CREDENTIALS_TEST_TOOL, remove_from_panel=False)
|
||||
|
||||
# Verify tool is actually removed
|
||||
assert self._app.toolbox.get_tool(CREDENTIALS_TEST_TOOL) is None
|
||||
|
||||
# Test 1: List credentials with include_definition=True
|
||||
response = self._get("/api/users/current/credentials?include_definition=true")
|
||||
self._assert_status_code_is(response, 200)
|
||||
credentials_with_definition = response.json()
|
||||
|
||||
assert len(credentials_with_definition) == 1
|
||||
credential = credentials_with_definition[0]
|
||||
|
||||
# Check that the credential still has basic information
|
||||
assert credential["id"] == user_credentials_id
|
||||
assert credential["source_id"] == CREDENTIALS_TEST_TOOL
|
||||
assert credential["source_type"] == "tool"
|
||||
|
||||
# Check that the fallback definition was provided
|
||||
assert "definition" in credential
|
||||
definition = credential["definition"]
|
||||
assert definition["name"] == payload["service_credential"]["name"]
|
||||
assert definition["version"] == payload["service_credential"]["version"]
|
||||
assert definition["description"] == ""
|
||||
assert definition["label"] == ""
|
||||
assert definition["optional"] is False
|
||||
assert definition["variables"] == []
|
||||
assert definition["secrets"] == []
|
||||
|
||||
# Verify that groups are still present and accessible
|
||||
assert len(credential["groups"]) > 0
|
||||
|
||||
# Test 2: List credentials without include_definition
|
||||
response = self._get("/api/users/current/credentials")
|
||||
self._assert_status_code_is(response, 200)
|
||||
credentials_without_definition = response.json()
|
||||
|
||||
assert len(credentials_without_definition) == 1
|
||||
credential_no_def = credentials_without_definition[0]
|
||||
|
||||
# Should not have definition field when not requested
|
||||
assert "definition" not in credential_no_def
|
||||
assert credential_no_def["id"] == user_credentials_id
|
||||
assert len(credential_no_def["groups"]) > 0
|
||||
finally:
|
||||
# Restore the tool to avoid affecting other tests
|
||||
if tool is not None:
|
||||
self._app.toolbox.register_tool(tool)
|
||||
|
||||
def _provide_user_credentials(self, payload=None, status_code=200):
|
||||
payload = payload or self._build_credentials_payload()
|
||||
response = self._post("/api/users/current/credentials", data=payload, json=True)
|
||||
|
||||
Reference in New Issue
Block a user