From d71602e7cc03a96bf876171da18a19d8dcb7efb9 Mon Sep 17 00:00:00 2001 From: Yaroslav Afenkin Date: Thu, 28 Sep 2023 14:53:02 +0300 Subject: [PATCH] Address review feedback * use `#element` over `#accumulate` * show different notification when credentials weren't added --- .../credentials/CredentialsSelectHelper.java | 23 ++++++++++++------- 1 file changed, 15 insertions(+), 8 deletions(-) diff --git a/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java b/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java index 1a2f703f7..ccf6c41fb 100644 --- a/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java +++ b/src/main/java/com/cloudbees/plugins/credentials/CredentialsSelectHelper.java @@ -600,8 +600,8 @@ public JSONObject doAddCredentials(StaplerRequest req, StaplerResponse rsp) thro if (!store.isDomainsModifiable()) { hudson.util.HttpResponses.status(400).generateResponse(req, rsp, null); return new JSONObject() - .accumulate("message", "Domain is read-only") - .accumulate("notificationType", "ERROR"); + .element("message", "Domain is read-only") + .element("notificationType", "ERROR"); } store.checkPermission(CredentialsStoreAction.CREATE); JSONObject data = req.getSubmittedForm(); @@ -610,15 +610,22 @@ public JSONObject doAddCredentials(StaplerRequest req, StaplerResponse rsp) thro if (!store.getDomains().contains(wrapper.getDomain())) { hudson.util.HttpResponses.status(400).generateResponse(req, rsp, null); return new JSONObject() - .accumulate("message", "Store does not have selected domain") - .accumulate("notificationType", "ERROR"); + .element("message", "Store does not have selected domain") + .element("notificationType", "ERROR"); } store.checkPermission(CredentialsStoreAction.CREATE); Credentials credentials = req.bindJSON(Credentials.class, data.getJSONObject("credentials")); - store.addCredentials(wrapper.getDomain(), credentials); - return new JSONObject() - .accumulate("message", "Credentials created") - .accumulate("notificationType", "SUCCESS"); + boolean credentialsWereAdded = store.addCredentials(wrapper.getDomain(), credentials); + if (credentialsWereAdded) { + return new JSONObject() + .element("message", "Credentials created") + .element("notificationType", "SUCCESS"); + } else { + return new JSONObject() + .element("message", "Credentials with specified ID already exist in " + domainName + " domain") + // TODO: or domain does not exist at all? + .element("notificationType", "ERROR"); + } } /**