fix error when group artifact is submitted without url (#162)

UMN-LATIS · Apr 29, 2024 · c5f1835 · c5f1835
1 parent 5cd4568
commit c5f1835
Show file tree

Hide file tree

Showing 7 changed files with 113 additions and 24 deletions.
diff --git a/resources/js/components/Button.vue b/resources/js/components/Button.vue
@@ -9,6 +9,8 @@
         variant === 'secondary',
       'tw-text-bs-blue hover:tw-bg-blue-100 tw-text-xs tw-uppercase tw-font-semibold tw-p-2 tw-bg-transparent tw-border-none tw-whitespace-nowrap':
         variant === 'tertiary',
+      'tw-border tw-border-red-500 tw-bg-transparent hover:tw-bg-bs-red tw-text-red-500 !tw-px-3 !tw-py-2':
+        variant === 'danger',
     }"
     v-bind="$attrs"
     :to="componentType === RouterLink ? to : undefined"

diff --git a/resources/js/components/EditGroup.vue b/resources/js/components/EditGroup.vue
@@ -150,7 +150,7 @@
         </div>
       </div>
     </div>
-    <div class="row">
+    <div class="row" data-cy="group-artifacts-section">
       <div class="col-md-12">
         <div class="row">
           <div class="col-md-12">
@@ -166,28 +166,35 @@
         <div
           v-for="(artifact, key) in localGroup.artifacts"
           :key="key"
-          class="form-row"
+          class="form-row tw-flex tw-items-start tw-gap-2"
         >
-          <div class="form-group col-md-5">
-            <input
-              v-model="artifact.label"
-              type=""
-              class="form-control"
-              placeholder="Label"
-            />
-          </div>
-          <div class="form-group col-md-6">
-            <input
-              v-model="artifact.target"
-              class="form-control"
-              placeholder="Target URL"
-            />
-          </div>
-          <div class="form-group col-md-1 d-flex flex-column">
-            <button class="btn btn-danger" @click="removeArtifact(key)">
-              <i class="fas fa-trash-alt"></i>
-            </button>
-          </div>
+          <InputGroup
+            v-model="artifact.label"
+            label="Label"
+            required
+            placeholder="Label"
+            :showLabel="false"
+            class="tw-flex-1"
+            :validateWhenUntouched="true"
+            :isValid="!!artifact.label"
+            data-cy="artifact-label"
+          />
+          <InputGroup
+            v-model="artifact.target"
+            label="Target URL"
+            required
+            placeholder="Target URL"
+            :showLabel="false"
+            class="tw-flex-1"
+            :validateWhenUntouched="true"
+            :isValid="isValidUrl(artifact.target)"
+            errorText="Invalid URL. Must be like 'https://umn.edu'"
+            data-cy="artifact-target"
+          />
+          <button class="btn btn-danger tw-py-1" @click="removeArtifact(key)">
+            <i class="fas fa-trash-alt tw-m-0"></i>
+            <span class="sr-only">Remove Artifact</span>
+          </button>
         </div>
       </div>
     </div>
@@ -306,7 +313,8 @@ import Members from "./Members.vue";
 import Modal from "./Modal.vue";
 import FolderWidget from "./FolderWidget.vue";
 import PersonSearch from "./PersonSearch.vue";
-import { dayjs, axios, getTempId, isTempId } from "@/utils";
+import { dayjs, axios, isValidUrl } from "@/utils";
+import InputGroup from "./InputGroup.vue";
 
 export default {
   components: {
@@ -315,6 +323,7 @@ export default {
     FolderWidget,
     PersonSearch,
     ComboBox,
+    InputGroup,
   },
   props: ["group"],
   emits: ["update:editing", "update:reload"],
@@ -420,6 +429,7 @@ export default {
       });
   },
   methods: {
+    isValidUrl,
     handleUpdateParentGroup(group) {
       this.localGroup.parent_group_id = group?.id ?? null;
     },
@@ -464,6 +474,14 @@ export default {
         }
       }
 
+      for (let artifact of this.localGroup.artifacts) {
+        if (!artifact.label || !isValidUrl(artifact.target)) {
+          this.saveError = "Every artifact must have a label and valid URL";
+          this.showError = true;
+          return false;
+        }
+      }
+
       if (this.localGroup.group_type == null) {
         this.saveError = "You must select a group type";
         this.showError = true;

diff --git a/resources/js/components/InputGroup.vue b/resources/js/components/InputGroup.vue
@@ -65,6 +65,8 @@ const props = withDefaults(
     isValid: undefined,
     validator: () => true,
     validateWhenUntouched: false,
+    labelClass: "",
+    inputClass: "",
   },
 );
 

diff --git a/resources/js/components/ViewGroup.vue b/resources/js/components/ViewGroup.vue
@@ -117,7 +117,10 @@
           <template v-if="group.artifacts.length">
             <dt>Artifacts</dt>
             <dd>
-              <ul class="tw-list-none tw-pl-0 tw-m-0">
+              <ul
+                class="tw-list-none tw-pl-0 tw-m-0"
+                data-cy="group-artifacts-list"
+              >
                 <li v-for="(artifact, index) in group.artifacts" :key="index">
                   <a :href="artifact.target">{{ artifact.label }}</a>
                 </li>

diff --git a/resources/js/utils/index.ts b/resources/js/utils/index.ts
@@ -5,3 +5,4 @@ export { sortByName } from "./sortByName";
 export { getTempId, isTempId } from "./tempIdHelpers";
 export { parseIntFromRouteParam } from "./parseIntFromRouteParam";
 export { sortByValueAtPath } from "./sortByValueAtPath";
+export { isValidUrl } from "./isValidUrl";
diff --git a/resources/js/utils/isValidUrl.ts b/resources/js/utils/isValidUrl.ts
@@ -0,0 +1,8 @@
+export function isValidUrl(urlString: string) {
+    try {
+        new URL(urlString);
+        return true;
+    } catch (error) {
+        return false;
+    }
+}
diff --git a/tests/cypress/integration/groupCreation.test.ts b/tests/cypress/integration/groupCreation.test.ts
@@ -60,6 +60,61 @@ describe("Groups UI", () => {
       cy.contains("Save").click();
       cy.contains("McAdmin");
     });
+
+    it("requires url and label to add a group artifact", () => {
+      cy.create("App\\Group").then((group) => {
+        cy.visit(`/group/${group.id}`);
+        cy.contains("Edit Group").click();
+        cy.contains("Add Artifact").click();
+
+        // try to save without entering any data
+        cy.contains("Save").click();
+
+        // an error should be displayed
+        cy.contains("Every artifact must have a label and valid URL");
+
+        // dismiss the error
+        cy.contains("Close").click();
+
+        // try to save with a label but no URL
+        cy.get("[data-cy=group-artifacts-section]").within(() => {
+          cy.get("[data-cy=artifact-label] input").type("CLA Website")
+        });
+
+        cy.contains("Save").click();
+
+        // an error should be displayed
+        cy.contains("Every artifact must have a label and valid URL");
+
+        // dismiss the error
+        cy.contains("Close").click();
+
+        // add an invalid URL
+        cy.get("[data-cy=group-artifacts-section]").within(() => {
+          cy.get("[data-cy=artifact-target] input").type("not a url")
+          // an error should show up next to the URL input
+          cy.contains("Invalid URL");
+        });
+
+        // attempting to save again shows the error
+        cy.contains("Save").click();
+        cy.contains("Every artifact must have a label and valid URL");
+
+        // dismiss the error
+        cy.contains("Close").click();
+
+        // finally add a valid url
+        cy.get("[data-cy=group-artifacts-section]").within(() => {
+          cy.get("[data-cy=artifact-target] input").clear().type("https://cla.umn.edu")
+        });
+
+        // saving should be successful
+        cy.contains("Save").click();
+        cy.get("[data-cy=group-artifacts-list]").within(() => {
+          cy.contains("CLA Website").should("have.attr", "href", "https://cla.umn.edu");
+        });
+      });
+    })
   });
 
   context("when authenticated as default user", () => {