From 16b68cd9cc1e6edd7b250cc2240bac6997ae9a0a Mon Sep 17 00:00:00 2001
From: Nicolas Pennec <nicolas.pennec@cg-wire.com>
Date: Wed, 11 Oct 2023 12:19:52 +0200
Subject: [PATCH] [settings] improve studio settings form

- refactor with <form>
- validate fields on submit
- add Remove Logo action
---
 src/components/pages/Settings.vue | 264 +++++++++++++++---------------
 src/locales/en.js                 |   1 +
 src/store/api/people.js           |   3 +-
 src/store/modules/people.js       |  17 +-
 4 files changed, 137 insertions(+), 148 deletions(-)

diff --git a/src/components/pages/Settings.vue b/src/components/pages/Settings.vue
index 04e881c760..84901c9a21 100644
--- a/src/components/pages/Settings.vue
+++ b/src/components/pages/Settings.vue
@@ -1,103 +1,97 @@
 <template>
   <div class="settings page">
-    <div class="settings-content">
-      <div class="settings-body">
-        <h2>
-          {{ $t('settings.title') }}
-        </h2>
-        <p>
-          <label class="label">
-            {{ $t('settings.logo') }}
-          </label>
-        </p>
-        <div class="logo-wrapper" v-if="organisation.has_avatar">
+    <form ref="form" class="settings-form" @submit.prevent="saveSettings">
+      <h2>
+        {{ $t('settings.title') }}
+      </h2>
+      <div class="field">
+        <label class="label">
+          {{ $t('settings.logo') }}
+        </label>
+        <div class="logo-wrapper" v-if="form.has_avatar">
           <img :src="organisationLogoPath" />
         </div>
         <p class="no-logo" v-else>
-          {{ $t('settings.no_logo') }}
+          <em>{{ $t('settings.no_logo') }}</em>
         </p>
         <p>
-          <button class="button set-logo-button" @click="showAvatarModal">
+          <button
+            type="button"
+            class="button set-logo-button"
+            @click="showAvatarModal"
+          >
             {{ $t('settings.set_logo') }}
           </button>
         </p>
+        <p v-if="form.has_avatar">
+          <button
+            type="button"
+            class="button is-link remove-logo-button"
+            @click="removeAvatar"
+          >
+            {{ $t('settings.remove_logo') }}
+          </button>
+        </p>
+      </div>
+      <text-field
+        class="mt2"
+        :label="$t('settings.fields.name')"
+        v-model.trim="form.name"
+      />
+      <text-field
+        :label="$t('settings.fields.hours_by_day')"
+        :max="24"
+        type="number"
+        v-model="form.hours_by_day"
+      />
+      <combobox-boolean
+        :label="$t('settings.fields.use_original_name')"
+        v-model="form.use_original_file_name"
+      />
+      <combobox-boolean
+        :label="$t('settings.fields.show_hd_default')"
+        v-model="form.hd_by_default"
+      />
+      <combobox-boolean
+        :label="$t('settings.fields.timesheets_locked')"
+        v-model="form.timesheets_locked"
+      />
+      <h2>
+        {{ $t('settings.integrations') }}
+      </h2>
+      <text-field
+        :label="$t('settings.fields.slack_token')"
+        v-model.trim="form.chat_token_slack"
+      />
+      <text-field
+        :label="$t('settings.fields.discord_token')"
+        v-model.trim="form.chat_token_discord"
+      />
+      <div class="mattermost_integrations">
         <text-field
-          class="mt2"
-          :label="$t('settings.fields.name')"
-          @enter="saveSettings()"
-          v-model="form.name"
-        />
-        <text-field
-          :label="$t('settings.fields.hours_by_day')"
-          type="number"
-          @enter="saveSettings()"
-          v-model="form.hours_by_day"
-        />
-        <combobox-boolean
-          :label="$t('settings.fields.use_original_name')"
-          @enter="saveSettings()"
-          v-model="form.use_original_file_name"
-        />
-        <combobox-boolean
-          :label="$t('settings.fields.show_hd_default')"
-          @enter="saveSettings()"
-          v-model="form.hd_by_default"
-        />
-        <combobox-boolean
-          :label="$t('settings.fields.timesheets_locked')"
-          @enter="saveSettings()"
-          v-model="form.timesheets_locked"
-        />
-        <h2>
-          {{ $t('settings.integrations') }}
-        </h2>
-        <text-field
-          :label="$t('settings.fields.slack_token')"
-          @enter="saveSettings()"
-          v-model="form.chat_token_slack"
-        />
-
-        <text-field
-          :label="$t('settings.fields.discord_token')"
-          @enter="saveSettings()"
-          v-model="form.chat_token_discord"
+          :label="$t('settings.fields.mattermost_webhook')"
+          v-model.trim="form.chat_webhook_mattermost"
         />
-
-        <div id="mattermost_integrations">
-          <text-field
-            :label="$t('settings.fields.mattermost_webhook')"
-            @enter="saveSettings()"
-            v-model="form.chat_webhook_mattermost"
-          />
-          <div
-            class="error pull-right"
-            v-if="this.errors.webhook_error === true"
-          >
-            <em>{{ $t('settings.webhook_error') }}</em>
-          </div>
-        </div>
-
-        <button
-          :class="{
-            button: true,
-            'save-button': true,
-            'is-medium': true,
-            'is-loading': loading.save
-          }"
-          @click="saveSettings()"
+        <div
+          class="error has-text-centered"
+          v-if="this.errors.webhook_error === true"
         >
-          {{ $t('settings.save.button') }}
-        </button>
-        <p
-          :class="{
-            error: true,
-            'is-hidden': !errors.save
-          }"
-        >
-          {{ $t('settings.save.error') }}
-        </p>
+          <em>{{ $t('settings.webhook_error') }}</em>
+        </div>
       </div>
-    </div>
+      <button
+        class="button save-button is-medium"
+        :class="{
+          'is-loading': loading.save
+        }"
+        :disabled="loading.save || !this.$refs.form?.checkValidity()"
+      >
+        {{ $t('settings.save.button') }}
+      </button>
+      <p class="error has-text-centered mt2" v-if="errors.save">
+        <em>{{ $t('settings.save.error') }}</em>
+      </p>
+    </form>
 
     <change-avatar-modal
       :active="modals.avatar"
@@ -128,16 +122,17 @@ export default {
 
   data() {
     return {
-      organisationLogoKey: new Date().getUTCDate(),
       organisationLogoPath: '',
       form: {
-        name: '',
-        hours_by_day: 0,
-        original_file_name: 'false',
-        hd_by_default: 'false',
+        chat_token_discord: '',
         chat_token_slack: '',
         chat_webhook_mattermost: '',
-        chat_token_discord: ''
+        has_avatar: false,
+        hd_by_default: 'false',
+        hours_by_day: 0,
+        name: '',
+        timesheets_locked: 'false',
+        use_original_file_name: 'false'
       },
       errors: {
         save: false,
@@ -155,10 +150,7 @@ export default {
   },
 
   mounted() {
-    this.form = Object.assign(this.form, this.organisation)
-    this.organisationLogoKey = 'key' + new Date().toISOString()
-    this.organisationLogoPath =
-      '/api/pictures/thumbnails/organisations/' + `${this.organisation.id}.png`
+    this.organisationLogoPath = `/api/pictures/thumbnails/organisations/${this.organisation.id}.png`
   },
 
   computed: {
@@ -194,15 +186,13 @@ export default {
         this.loading.save = true
         this.errors.save = false
         this.saveOrganisation(this.form)
-          .then(() => {
-            this.loading.save = false
-            this.errors.save = false
-          })
           .catch(err => {
             console.error(err)
-            this.loading.save = false
             this.errors.save = true
           })
+          .finally(() => {
+            this.loading.save = false
+          })
       }
     },
 
@@ -214,10 +204,8 @@ export default {
           setTimeout(() => {
             this.loading.saveAvatar = false
             this.modals.avatar = false
-            this.organisationLogoPath =
-              '/api/pictures/thumbnails/organisations/' +
-              `${this.organisation.id}.png?t=` +
-              new Date().toISOString()
+            const timestamp = Date.now()
+            this.organisationLogoPath = `/api/pictures/thumbnails/organisations/${this.organisation.id}.png?t=${timestamp}`
           }, 500)
         })
         .catch(err => {
@@ -229,24 +217,33 @@ export default {
 
     showAvatarModal() {
       this.modals.avatar = true
+    },
+
+    removeAvatar() {
+      this.form.has_avatar = false
     }
   },
 
   watch: {
-    organisation() {
-      this.form = {
-        name: this.organisation.name,
-        hours_by_day: this.organisation.hours_by_day,
-        use_original_file_name: this.organisation.use_original_file_name
-          ? 'true'
-          : 'false',
-        timesheets_locked: this.organisation.timesheets_locked
-          ? 'true'
-          : 'false',
-        hd_by_default: this.organisation.hd_by_default ? 'true' : 'false',
-        chat_token_slack: this.organisation.chat_token_slack,
-        chat_token_discord: this.organisation.chat_token_discord,
-        chat_webhook_mattermost: this.organisation.chat_webhook_mattermost
+    organisation: {
+      immediate: true,
+      handler() {
+        this.form = {
+          chat_token_discord: this.organisation.chat_token_discord,
+          chat_token_slack: this.organisation.chat_token_slack,
+          chat_webhook_mattermost: this.organisation.chat_webhook_mattermost,
+          has_avatar: this.organisation.has_avatar,
+          hd_by_default: this.organisation.hd_by_default ? 'true' : 'false',
+          hours_by_day: this.organisation.hours_by_day,
+          name: this.organisation.name,
+          timesheets_locked: this.organisation.timesheets_locked
+            ? 'true'
+            : 'false',
+          use_original_file_name: this.organisation.use_original_file_name
+            ? 'true'
+            : 'false'
+        }
+        console.log(this.form.has_avatar)
       }
     }
   },
@@ -262,11 +259,11 @@ export default {
 <style lang="scss" scoped>
 .dark {
   .settings {
-    background: #36393f;
+    background: $dark-grey-2;
     color: $white-grey;
   }
 
-  .settings-content {
+  .settings-form {
     background: $dark-grey-lighter;
     color: $white-grey;
   }
@@ -277,16 +274,11 @@ export default {
   }
 }
 
-#mattermost_integrations {
+.mattermost_integrations {
   margin-bottom: 4em;
-}
-
-#mattermost_integrations .field {
-  margin-bottom: 0em;
-}
-
-strong {
-  text-transform: uppercase;
+  .field {
+    margin-bottom: 0em;
+  }
 }
 
 .settings {
@@ -294,20 +286,17 @@ strong {
   height: 100%;
 }
 
-.settings-content {
+.settings-form {
   background: white;
   max-width: 500px;
   margin: auto;
   margin-top: 2em;
   margin-bottom: 2em;
+  padding: 2em;
   box-shadow: rgba(0, 0, 0, 0.15) 0px 1px 4px 2px;
   border-radius: 1em;
 }
 
-.settings-body {
-  padding: 2em;
-}
-
 input,
 select,
 span.select {
@@ -342,4 +331,9 @@ h2:first-child {
 .no-logo {
   margin-bottom: 1em;
 }
+
+.remove-logo-button {
+  margin-top: 0.5rem;
+  font-size: 0.7em;
+}
 </style>
diff --git a/src/locales/en.js b/src/locales/en.js
index a083e981b5..95b012d113 100644
--- a/src/locales/en.js
+++ b/src/locales/en.js
@@ -982,6 +982,7 @@ export default {
     integrations: 'Integrations',
     logo: 'Studio logo',
     no_logo: 'There is no logo set.',
+    remove_logo: 'Remove studio logo',
     set_logo: 'Set studio logo',
     title: 'Settings',
     webhook_error: 'An error occurred while posting webhook',
diff --git a/src/store/api/people.js b/src/store/api/people.js
index c5119a8ee9..ba71a2536c 100644
--- a/src/store/api/people.js
+++ b/src/store/api/people.js
@@ -33,7 +33,8 @@ export default {
       hd_by_default: organisation.hd_by_default === 'true',
       chat_token_slack: organisation.chat_token_slack,
       chat_webhook_mattermost: organisation.chat_webhook_mattermost,
-      chat_token_discord: organisation.chat_token_discord
+      chat_token_discord: organisation.chat_token_discord,
+      has_avatar: organisation.has_avatar
     }
     return client.pput(`/api/data/organisations/${organisation.id}`, data)
   },
diff --git a/src/store/modules/people.js b/src/store/modules/people.js
index 55cb341b2a..f023892cf2 100644
--- a/src/store/modules/people.js
+++ b/src/store/modules/people.js
@@ -230,20 +230,14 @@ const actions = {
     form.id = state.organisation.id
     return peopleApi.updateOrganisation(form).then(organisation => {
       commit(SET_ORGANISATION, organisation)
-      Promise.resolve(organisation)
+      return organisation
     })
   },
 
   uploadOrganisationLogo({ commit, state }, formData) {
-    return new Promise((resolve, reject) => {
-      const organisationId = state.organisation.id
-      peopleApi
-        .postOrganisationLogo(organisationId, formData)
-        .then(organisation => {
-          commit(SET_ORGANISATION, { has_avatar: true })
-          resolve()
-        })
-        .catch(reject)
+    const organisationId = state.organisation.id
+    return peopleApi.postOrganisationLogo(organisationId, formData).then(() => {
+      commit(SET_ORGANISATION, { has_avatar: true })
     })
   },
 
@@ -848,8 +842,7 @@ const mutations = {
   },
 
   [SET_ORGANISATION](state, organisation) {
-    Object.assign(state.organisation, organisation)
-    state.organisation = { ...state.organisation }
+    state.organisation = { ...state.organisation, ...organisation }
   },
 
   [RESET_ALL](state, people) {