Add save check, refactor component

Resolves MS-785

client/cypress/e2e/frontend/rooms/room-page.spec.js

@@ -324,6 +324,7 @@ describe('The Room Page (student)', () => {
         edges: [],
       },
     };
+    const requestTime = 500;
     const operations = {
       MeQuery,
       RoomEntriesQuery: {
@@ -354,7 +355,7 @@ describe('The Room Page (student)', () => {
                 },
               },
             });
-          }, 500);
+          }, requestTime);
         });
       },
     };
@@ -377,7 +378,7 @@ describe('The Room Page (student)', () => {
     cy.getByDataCy('save-button').click();
     cy.getByDataCy('save-button').click();
     cy.getByDataCy('room-title').should('contain', 'A Room');
-    cy.wait(1000); // wait for both requests to finish, if there's more than one
+    cy.wait(2 * requestTime); // wait for both requests to finish, if there's more than one
     cy.getByDataCy('room-entry').should('have.length', 1);
   });
 

client/src/components/content-block-form/ContentBlockForm.vue

@@ -111,7 +111,7 @@
     <footer class="content-block-form__footer">
       <div class="content-block-form__buttons">
         <button
-          :disabled="!isValid"
+          :disabled="!isValid && !isSaving"
           class="button button--primary"
           data-cy="save-button"
           @click="save(localContentBlock)"
@@ -128,14 +128,17 @@
   </div>
 </template>
 
-<script lang="ts">
-import { PropType, defineComponent } from 'vue';
+<script setup lang="ts">
 import Toggle from '@/components/ui/Toggle.vue';
 import ContentFormSection from '@/components/content-block-form/ContentFormSection.vue';
 import InputWithLabel from '@/components/ui/InputWithLabel.vue';
 import AddContentLink from '@/components/content-block-form/AddContentLink.vue';
-
 import ContentElement from '@/components/content-block-form/ContentElement.vue';
+import ContentElementActions from '@/components/content-block-form/ContentElementActions.vue';
+import { DEFAULT_FEATURE_SET } from '@/consts/features.consts';
+import { computed, inject, provide, ref } from 'vue';
+import { ContentBlock, numberOrUndefined } from '@/@types';
+import { CHOOSER, transformInnerContents } from '@/components/content-block-form/helpers.js';
 import {
   insertAtIndex,
   moveToIndex,
@@ -143,193 +146,172 @@ import {
   replaceAtIndex,
   swapElements,
 } from '@/graphql/immutable-operations';
+import { Modal } from '@/plugins/modal.types';
 
-import { CHOOSER, transformInnerContents } from '@/components/content-block-form/helpers.js';
-import ContentElementActions from '@/components/content-block-form/ContentElementActions.vue';
-import { ContentBlock, numberOrUndefined } from '@/@types';
-import { DEFAULT_FEATURE_SET } from '@/consts/features.consts';
-
-// TODO: refactor this file, it's huuuuuge!
-interface ContentBlockFormData {
-  localContentBlock: any;
+export interface Props {
+  title?: String;
+  contentBlock: ContentBlock;
+  features?: string;
+  isSaving?: boolean;
 }
 
-export default defineComponent({
-  props: {
-    title: {
-      type: String,
-      default: '',
-    },
-    contentBlock: {
-      type: Object as PropType<ContentBlock>,
-      required: true,
-    },
-    features: {
-      type: String,
-      default: DEFAULT_FEATURE_SET,
-    },
-  },
-  provide(): object {
-    return {
-      features: this.features,
-    };
-  },
-  components: {
-    ContentElementActions,
-    ContentElement,
-    AddContentLink,
-    InputWithLabel,
-    ContentFormSection,
-    Toggle,
-  },
-  data(): ContentBlockFormData {
-    return {
-      localContentBlock: Object.assign(
-        {},
-        {
-          title: this.contentBlock.title,
-          // contents: [...this.contentBlock.contents],
-          contents: transformInnerContents([...this.contentBlock.contents]),
-          id: this.contentBlock.id || undefined,
-          isAssignment: this.contentBlock.type && this.contentBlock.type.toLowerCase() === 'task',
-        }
-      ),
-    };
-  },
-  computed: {
-    isValid(): boolean {
-      return this.localContentBlock.title > '';
-    },
-    hasDefaultFeatures(): boolean {
-      return this.features === DEFAULT_FEATURE_SET;
-    },
-  },
-  methods: {
-    update(index: number, element: any, parent?: number) {
-      if (parent === undefined) {
-        // element is top level
-        this.localContentBlock.contents = replaceAtIndex(this.localContentBlock.contents, index, element);
-      } else {
-        const parentBlock = this.localContentBlock.contents[parent];
+const props = withDefaults(defineProps<Props>(), {
+  title: '',
+  features: DEFAULT_FEATURE_SET,
+  isSaving: false,
+});
 
-        const newElementContents = replaceAtIndex(parentBlock.contents, index, element);
-        const newBlock = {
-          ...parentBlock,
-          contents: newElementContents,
-        };
-        this.localContentBlock.contents = replaceAtIndex(this.localContentBlock.contents, parent, newBlock);
-      }
-    },
-    addBlock(afterOuterIndex: number, innerIndex?: number) {
-      if (innerIndex !== undefined) {
-        const block = this.localContentBlock.contents[afterOuterIndex];
-        const element = {
-          ...block,
-          contents: insertAtIndex(block.contents, innerIndex + 1, {
-            id: -1,
-            type: CHOOSER,
-          }),
-        };
+const emit = defineEmits(['save']);
+const modal = inject('modal') as Modal;
 
-        this.localContentBlock.contents = replaceAtIndex(this.localContentBlock.contents, afterOuterIndex, element);
-      } else {
-        const element = {
-          id: -1,
-          type: CHOOSER,
-          includeListOption: true,
-        };
+const localContentBlock = ref(
+  Object.assign(
+    {},
+    {
+      title: props.contentBlock.title,
+      contents: transformInnerContents([...props.contentBlock.contents]),
+      id: props.contentBlock.id || undefined,
+      isAssignment: props.contentBlock.type && props.contentBlock.type.toLowerCase() === 'task',
+    }
+  )
+);
 
-        this.localContentBlock.contents = insertAtIndex(this.localContentBlock.contents, afterOuterIndex + 1, element);
-      }
-    },
-    remove(outer: number, inner?: number, askForConfirmation = true) {
-      if (askForConfirmation) {
-        this.$modal
-          .open('confirm')
-          .then(() => {
-            this.executeRemoval(outer, inner);
-          })
-          .catch(() => {});
-      } else {
-        this.executeRemoval(outer, inner);
-      }
-    },
-    shift(outer: number, inner: numberOrUndefined = undefined, distance: number) {
-      if (inner === undefined) {
-        this.localContentBlock.contents = swapElements(this.localContentBlock.contents, outer, outer + distance);
-      } else {
-        const { contents } = this.localContentBlock;
-        const outerElement = contents[outer];
-        const newOuterElement = {
-          ...outerElement,
-          contents: swapElements(outerElement.contents, inner, inner + distance),
-        };
-        this.localContentBlock.contents = replaceAtIndex(contents, outer, newOuterElement);
-      }
-    },
-    top(outer: number, inner: numberOrUndefined = undefined) {
-      if (inner === undefined) {
-        this.localContentBlock.contents = moveToIndex(this.localContentBlock.contents, outer, 0);
-      } else {
-        const { contents } = this.localContentBlock;
-        const outerElement = contents[outer];
-        const newOuterElement = {
-          ...outerElement,
-          contents: moveToIndex(outerElement.contents, inner, 0),
-        };
-        this.localContentBlock.contents = replaceAtIndex(contents, outer, newOuterElement);
-      }
-    },
-    up(outer: number, inner: numberOrUndefined = undefined) {
-      this.shift(outer, inner, -1);
-    },
-    down(outer: number, inner: numberOrUndefined = undefined) {
-      this.shift(outer, inner, 1);
-    },
-    bottom(outer: number, inner: numberOrUndefined = undefined) {
-      if (inner === undefined) {
-        const maxIndex = this.localContentBlock.contents.length - 1;
-        this.localContentBlock.contents = moveToIndex(this.localContentBlock.contents, outer, maxIndex);
-      } else {
-        const { contents } = this.localContentBlock;
-        const outerElement = contents[outer];
-        const maxIndex = outerElement.contents.length - 1;
-        const newOuterElement = {
-          ...outerElement,
-          contents: moveToIndex(outerElement.contents, inner, maxIndex),
-        };
-        this.localContentBlock.contents = replaceAtIndex(contents, outer, newOuterElement);
-      }
-    },
-    executeRemoval(outer: number, inner: numberOrUndefined = undefined) {
-      if (inner === undefined) {
-        // not a list item container, just remove the element from the outer array
-        this.localContentBlock.contents = removeAtIndex(this.localContentBlock.contents, outer);
-      } else {
-        let prevInnerContents = this.localContentBlock.contents[outer].contents;
-        let innerContents = removeAtIndex(prevInnerContents, inner);
+const hasDefaultFeatures = computed(() => {
+  return props.features === DEFAULT_FEATURE_SET;
+});
+const isValid = computed(() => {
+  return localContentBlock.value.title > '';
+});
 
-        if (innerContents.length) {
-          /*
+const save = (contentBlock: ContentBlock) => {
+  if (!props.isSaving) {
+    emit('save', contentBlock);
+  }
+};
+const executeRemoval = (outer: number, inner: numberOrUndefined = undefined) => {
+  if (inner === undefined) {
+    // not a list item container, just remove the element from the outer array
+    localContentBlock.value.contents = removeAtIndex(localContentBlock.value.contents, outer);
+  } else {
+    let prevInnerContents = localContentBlock.value.contents[outer].contents;
+    let innerContents = removeAtIndex(prevInnerContents, inner);
+
+    if (innerContents.length) {
+      /*
               there is still an element inside the outer element after removal,
               so we replace the previous element in the outer array with the new one with fewer contents
              */
-          let element = {
-            ...this.localContentBlock.contents[outer],
-            contents: innerContents,
-          };
-          this.localContentBlock.contents = replaceAtIndex(this.localContentBlock.contents, outer, element);
-        } else {
-          // inner contents is now empty, remove the whole element from the outer array
-          this.localContentBlock.contents = removeAtIndex(this.localContentBlock.contents, outer);
-        }
-      }
-    },
-    save(contentBlock: ContentBlock) {
-      this.$emit('save', contentBlock);
-    },
-  },
-});
+      let element = {
+        ...localContentBlock.value.contents[outer],
+        contents: innerContents,
+      };
+      localContentBlock.value.contents = replaceAtIndex(localContentBlock.value.contents, outer, element);
+    } else {
+      // inner contents is now empty, remove the whole element from the outer array
+      localContentBlock.value.contents = removeAtIndex(localContentBlock.value.contents, outer);
+    }
+  }
+};
+const update = (index: number, element: any, parent?: number) => {
+  if (parent === undefined) {
+    // element is top level
+    localContentBlock.value.contents = replaceAtIndex(localContentBlock.value.contents, index, element);
+  } else {
+    const parentBlock = localContentBlock.value.contents[parent];
+
+    const newElementContents = replaceAtIndex(parentBlock.contents, index, element);
+    const newBlock = {
+      ...parentBlock,
+      contents: newElementContents,
+    };
+    localContentBlock.value.contents = replaceAtIndex(localContentBlock.value.contents, parent, newBlock);
+  }
+};
+const addBlock = (afterOuterIndex: number, innerIndex?: number) => {
+  if (innerIndex !== undefined) {
+    const block = localContentBlock.value.contents[afterOuterIndex];
+    const element = {
+      ...block,
+      contents: insertAtIndex(block.contents, innerIndex + 1, {
+        id: -1,
+        type: CHOOSER,
+      }),
+    };
+
+    localContentBlock.value.contents = replaceAtIndex(localContentBlock.value.contents, afterOuterIndex, element);
+  } else {
+    const element = {
+      id: -1,
+      type: CHOOSER,
+      includeListOption: true,
+    };
+
+    localContentBlock.value.contents = insertAtIndex(localContentBlock.value.contents, afterOuterIndex + 1, element);
+  }
+};
+
+const remove = (outer: number, inner?: number, askForConfirmation = true) => {
+  if (askForConfirmation) {
+    modal
+      .open('confirm')
+      .then(() => {
+        executeRemoval(outer, inner);
+      })
+      .catch(() => {});
+  } else {
+    executeRemoval(outer, inner);
+  }
+};
+const shift = (outer: number, inner: numberOrUndefined = undefined, distance: number) => {
+  if (inner === undefined) {
+    localContentBlock.value.contents = swapElements(localContentBlock.value.contents, outer, outer + distance);
+  } else {
+    const { contents } = localContentBlock.value;
+    const outerElement = contents[outer];
+    const newOuterElement = {
+      ...outerElement,
+      contents: swapElements(outerElement.contents, inner, inner + distance),
+    };
+    localContentBlock.value.contents = replaceAtIndex(contents, outer, newOuterElement);
+  }
+};
+const top = (outer: number, inner: numberOrUndefined = undefined) => {
+  if (inner === undefined) {
+    localContentBlock.value.contents = moveToIndex(localContentBlock.value.contents, outer, 0);
+  } else {
+    const { contents } = localContentBlock.value;
+    const outerElement = contents[outer];
+    const newOuterElement = {
+      ...outerElement,
+      contents: moveToIndex(outerElement.contents, inner, 0),
+    };
+    localContentBlock.value.contents = replaceAtIndex(contents, outer, newOuterElement);
+  }
+};
+const up = (outer: number, inner: numberOrUndefined = undefined) => {
+  shift(outer, inner, -1);
+};
+
+const down = (outer: number, inner: numberOrUndefined = undefined) => {
+  shift(outer, inner, 1);
+};
+const bottom = (outer: number, inner: numberOrUndefined = undefined) => {
+  if (inner === undefined) {
+    const maxIndex = localContentBlock.value.contents.length - 1;
+    localContentBlock.value.contents = moveToIndex(localContentBlock.value.contents, outer, maxIndex);
+  } else {
+    const { contents } = localContentBlock.value;
+    const outerElement = contents[outer];
+    const maxIndex = outerElement.contents.length - 1;
+    const newOuterElement = {
+      ...outerElement,
+      contents: moveToIndex(outerElement.contents, inner, maxIndex),
+    };
+    localContentBlock.value.contents = replaceAtIndex(contents, outer, newOuterElement);
+  }
+};
+
+provide('features', props.features);
 </script>
 
 <style lang="scss">

client/src/pages/rooms/newRoomEntry.vue

@@ -2,12 +2,13 @@
   <content-block-form
     :content-block="roomEntry"
     :features="features"
+    :is-saving="isSaving"
     @save="save"
     @back="goBack"
   />
 </template>
 
-<script>
+<script lang="ts">
 import { defineComponent } from 'vue';
 
 import NEW_ROOM_ENTRY_MUTATION from 'gql/mutations/rooms/addRoomEntry.gql';
@@ -44,6 +45,7 @@ export default defineComponent({
         title: '',
         contents: [],
       },
+      isSaving: false,
     };
   },
 
@@ -58,6 +60,7 @@ export default defineComponent({
       });
     },
     save({ title, contents }) {
+      this.isSaving = true;
       const entry = {
         ...{
           title,
@@ -105,6 +108,9 @@ export default defineComponent({
         })
         .then(() => {
           this.goBack();
+        })
+        .finally(() => {
+          this.isSaving = false;
         });
     },
   },