Feature/improve user feedback (#1076)

* add proper type annotations

* fix state management and dead code

* add response messages

frontend/pages/user/profile/edit.vue

@@ -50,7 +50,7 @@
               </v-card>
             </div>
             <div v-if="state" key="change-password">
-              <BaseCardSectionTitle class="mt-10" :title="$t('settings.change-password')"> </BaseCardSectionTitle>
+              <BaseCardSectionTitle class="mt-10" :title="$tc('settings.change-password')"> </BaseCardSectionTitle>
               <v-card outlined>
                 <v-card-text class="pb-0">
                   <v-form ref="passChange">
@@ -60,14 +60,16 @@
                       :label="$t('user.current-password')"
                       validate-on-blur
                       :type="showPassword ? 'text' : 'password'"
-                      @click:append="showPassword.current = !showPassword.current"
+                      :append-icon="showPassword ? $globals.icons.eye : $globals.icons.eyeOff"
+                      @click:append="showPassword = !showPassword"
                     ></v-text-field>
                     <v-text-field
                       v-model="password.newOne"
                       :prepend-icon="$globals.icons.lock"
                       :label="$t('user.new-password')"
                       :type="showPassword ? 'text' : 'password'"
-                      @click:append="showPassword.newOne = !showPassword.newOne"
+                      :append-icon="showPassword ? $globals.icons.eye : $globals.icons.eyeOff"
+                      @click:append="showPassword = !showPassword"
                     ></v-text-field>
                     <v-text-field
                       v-model="password.newTwo"
@@ -76,13 +78,18 @@
                       :rules="[password.newOne === password.newTwo || $t('user.password-must-match')]"
                       validate-on-blur
                       :type="showPassword ? 'text' : 'password'"
-                      @click:append="showPassword.newTwo = !showPassword.newTwo"
+                      :append-icon="showPassword ? $globals.icons.eye : $globals.icons.eyeOff"
+                      @click:append="showPassword = !showPassword"
                     ></v-text-field>
                   </v-form>
                 </v-card-text>
                 <v-card-actions>
                   <v-spacer></v-spacer>
-                  <BaseButton update @click="updatePassword" />
+                  <BaseButton
+                    update
+                    :disabled="!passwordsMatch || password.current.length < 0"
+                    @click="updatePassword"
+                  />
                 </v-card-actions>
               </v-card>
             </div>
@@ -112,7 +119,7 @@
 </template>
 
 <script lang="ts">
-import { ref, reactive, defineComponent, computed, useContext, watch } from "@nuxtjs/composition-api";
+import { ref, reactive, defineComponent, computed, useContext, watch, toRefs } from "@nuxtjs/composition-api";
 import { useUserApi } from "~/composables/api";
 import UserAvatar from "~/components/Domain/User/UserAvatar.vue";
 import { VForm } from "~/types/vuetify";
@@ -141,6 +148,8 @@ export default defineComponent({
       newTwo: "",
     });
 
+    const passwordsMatch = computed(() => password.newOne === password.newTwo && password.newOne.length > 0);
+
     async function updateUser() {
       const { response } = await api.users.updateOne(userCopy.value.id, userCopy.value);
       if (response?.status === 200) {
@@ -162,15 +171,14 @@ export default defineComponent({
       }
     }
 
-    return { updateUser, updatePassword, userCopy, password, domUpdatePassword };
-  },
-  data() {
-    return {
+    const state = reactive({
       hideImage: false,
       passwordLoading: false,
       showPassword: false,
       loading: false,
-    };
+    });
+
+    return { ...toRefs(state), updateUser, updatePassword, userCopy, password, domUpdatePassword, passwordsMatch };
   },
   head() {
     return {
@@ -179,4 +187,3 @@ export default defineComponent({
   },
 });
 </script>
-

mealie/routes/_base/abc_controller.py

@@ -2,11 +2,13 @@
 from functools import cached_property
 
 from fastapi import Depends
+from pydantic import UUID4
 
 from mealie.core.exceptions import mealie_registered_exceptions
 from mealie.repos.all_repositories import AllRepositories
 from mealie.routes._base.checks import OperationChecks
 from mealie.routes._base.dependencies import SharedDependencies
+from mealie.schema.user.user import GroupInDB, PrivateUser
 
 
 class BasePublicController(ABC):
@@ -39,15 +41,15 @@ def repos(self):
         return AllRepositories(self.deps.session)
 
     @property
-    def group_id(self):
+    def group_id(self) -> UUID4:
         return self.deps.acting_user.group_id
 
     @property
-    def user(self):
+    def user(self) -> PrivateUser:
         return self.deps.acting_user
 
     @property
-    def group(self):
+    def group(self) -> GroupInDB:
         return self.deps.repos.groups.get_one(self.group_id)
 
     @cached_property

mealie/routes/users/_helpers.py

@@ -1,9 +1,10 @@
 from fastapi import HTTPException, status
+from pydantic import UUID4
 
 from mealie.schema.user.user import PrivateUser
 
 
-def assert_user_change_allowed(id: int, current_user: PrivateUser):
+def assert_user_change_allowed(id: UUID4, current_user: PrivateUser):
     if current_user.id != id and not current_user.admin:
         # only admins can edit other users
         raise HTTPException(status.HTTP_403_FORBIDDEN, detail="NOT_AN_ADMIN")

mealie/routes/users/crud.py

@@ -1,13 +1,13 @@
 from fastapi import HTTPException, status
 from pydantic import UUID4
 
-from mealie.core import security
 from mealie.core.security import hash_password, verify_password
 from mealie.routes._base import BaseAdminController, controller
 from mealie.routes._base.abc_controller import BaseUserController
 from mealie.routes._base.mixins import CrudMixins
 from mealie.routes._base.routers import AdminAPIRouter, UserAPIRouter
 from mealie.routes.users._helpers import assert_user_change_allowed
+from mealie.schema.response import ErrorResponse, SuccessResponse
 from mealie.schema.user import ChangePassword, UserBase, UserIn, UserOut
 
 user_router = UserAPIRouter(prefix="/users", tags=["Users: CRUD"])
@@ -57,23 +57,39 @@ def update_user(self, item_id: UUID4, new_data: UserBase):
 
         if not self.user.admin and (new_data.admin or self.user.group != new_data.group):
             # prevent a regular user from doing admin tasks on themself
-            raise HTTPException(status.HTTP_403_FORBIDDEN)
+            raise HTTPException(
+                status.HTTP_403_FORBIDDEN, ErrorResponse.respond("User doesn't have permission to change group")
+            )
 
         if self.user.id == item_id and self.user.admin and not new_data.admin:
             # prevent an admin from demoting themself
-            raise HTTPException(status.HTTP_403_FORBIDDEN)
+            raise HTTPException(
+                status.HTTP_403_FORBIDDEN, ErrorResponse.respond("User doesn't have permission to change group")
+            )
 
-        self.repos.users.update(item_id, new_data.dict())
+        try:
+            self.repos.users.update(item_id, new_data.dict())
+        except Exception as e:
+            raise HTTPException(
+                status.HTTP_400_BAD_REQUEST,
+                ErrorResponse.respond("Failed to update user"),
+            ) from e
 
-        if self.user.id == item_id:
-            access_token = security.create_access_token(data=dict(sub=str(self.user.id)))
-            return {"access_token": access_token, "token_type": "bearer"}
+        return SuccessResponse.respond("User updated")
 
     @user_router.put("/{item_id}/password")
     def update_password(self, password_change: ChangePassword):
         """Resets the User Password"""
         if not verify_password(password_change.current_password, self.user.password):
-            raise HTTPException(status.HTTP_400_BAD_REQUEST)
+            raise HTTPException(status.HTTP_400_BAD_REQUEST, ErrorResponse.respond("Invalid current password"))
 
         self.user.password = hash_password(password_change.new_password)
-        return self.repos.users.update_password(self.user.id, self.user.password)
+        try:
+            self.repos.users.update_password(self.user.id, self.user.password)
+        except Exception as e:
+            raise HTTPException(
+                status.HTTP_400_BAD_REQUEST,
+                ErrorResponse.respond("Failed to update password"),
+            ) from e
+
+        return SuccessResponse.respond("Password updated")