Address review feedback

* use `#element` over `#accumulate` * show different notification when credentials weren't added
jenkinsci · Sep 28, 2023 · d71602e · d71602e
1 parent 53e74eb
commit d71602e
Showing 1 changed file with 15 additions and 8 deletions.
diff --git 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");
+            }
         }
 
         /**