From 33d68037a7ffa3b82990b4384998c3561b9cea7e Mon Sep 17 00:00:00 2001 From: Emily <44536222+emi-hi@users.noreply.github.com> Date: Wed, 13 Mar 2024 14:45:07 -0700 Subject: [PATCH] feat: CTHUB 198 - save/update users (#218) * chore: -backend: updates users moved into new service -frontend: adds immer for setting state, uses original users array rather than creating new one * chore: moves transaction.atomic to service --- django/api/decorators/permission.py | 17 +++++- django/api/serializers/user.py | 10 +++- django/api/services/permissions.py | 13 +++++ django/api/services/user.py | 23 ++++++++ django/api/viewsets/user.py | 47 +++++++++------ react/package.json | 1 + react/src/uploads/UploadContainer.js | 7 ++- react/src/users/UsersContainer.js | 77 +++++++++++++++++++++---- react/src/users/components/UsersPage.js | 46 ++++++++------- react/src/users/routes.js | 3 +- 10 files changed, 185 insertions(+), 59 deletions(-) create mode 100644 django/api/services/permissions.py create mode 100644 django/api/services/user.py diff --git a/django/api/decorators/permission.py b/django/api/decorators/permission.py index f94256a0..a681f4e9 100644 --- a/django/api/decorators/permission.py +++ b/django/api/decorators/permission.py @@ -1,8 +1,7 @@ from rest_framework.response import Response from rest_framework import status -from api.models.user import User -from api.models.user_permission import UserPermission -from api.models.permission import Permission +from api.services.permissions import create_permission_list + def check_upload_permission(): def wrapper(func): def wrapped(request, *args, **kwargs): @@ -20,3 +19,15 @@ def wrapped(request, *args, **kwargs): return func(request, *args, **kwargs) return wrapped return wrapper + +def check_admin_permission(): + def wrapper(func): + def wrapped(request, *args, **kwargs): + permissions = create_permission_list(request.user) + if 'admin' not in permissions: + return Response( + "You do not have permission to make changes to other users' permissions.", status=status.HTTP_403_FORBIDDEN + ) + return func(request, *args, **kwargs) + return wrapped + return wrapper diff --git a/django/api/serializers/user.py b/django/api/serializers/user.py index fc2b490f..f4ffc6a6 100644 --- a/django/api/serializers/user.py +++ b/django/api/serializers/user.py @@ -17,7 +17,15 @@ class UserSerializer(ModelSerializer): def get_user_permissions(self, obj): user_permission = UserPermission.objects.filter(user_id=obj.id) permissions = PermissionSerializer(user_permission, read_only=True, many=True) - return permissions.data + admin = False + uploader = False + for i in permissions.data: + if i['description'] == 'admin': + admin = True + if i['description'] == 'uploader': + uploader = True + + return {'admin': admin, 'uploader': uploader} class Meta: model = User diff --git a/django/api/services/permissions.py b/django/api/services/permissions.py new file mode 100644 index 00000000..1c0d583a --- /dev/null +++ b/django/api/services/permissions.py @@ -0,0 +1,13 @@ +from api.models.user import User +from api.models.user_permission import UserPermission +from api.models.permission import Permission + +def create_permission_list(user): + user = User.objects.filter(idir=user).first() + user_permission = UserPermission.objects.filter(user_id=user.id) + permissions = [] + if user_permission: + for each in user_permission: + permission = Permission.objects.get(id=each.permission_id) + permissions.append(permission.description) + return permissions \ No newline at end of file diff --git a/django/api/services/user.py b/django/api/services/user.py new file mode 100644 index 00000000..520934be --- /dev/null +++ b/django/api/services/user.py @@ -0,0 +1,23 @@ +from django.db import transaction +from api.models.user_permission import UserPermission +from api.models.permission import Permission +from api.models.user import User + +@transaction.atomic +def update_permissions(self, request): + msg = [] + permissions = Permission.objects.all() + UserPermission.objects.all().delete() + for each in request.data: + for k, v in each.items(): + if k == 'idir': + user = User.objects.get(idir=v) + if k == 'user_permissions': + for permission_description, value in v.items(): + if value == True or (user.idir == request.user and permission_description == 'admin'): + ## if they are updating permissions then they are already admin user, they cannot remove their own admin + permission = permissions.get(description=permission_description) + try: + UserPermission.objects.create(user_id=user.id, permission_id=permission.id) + except Exception as error: + msg.append("{} permission could not be added to {}".format(permission_description, user.idir)) \ No newline at end of file diff --git a/django/api/viewsets/user.py b/django/api/viewsets/user.py index 13d0629c..7ee41019 100644 --- a/django/api/viewsets/user.py +++ b/django/api/viewsets/user.py @@ -1,9 +1,13 @@ +from django.utils.decorators import method_decorator +from rest_framework import status from rest_framework.decorators import action from rest_framework.permissions import AllowAny from rest_framework.response import Response from rest_framework.viewsets import GenericViewSet from api.models.user import User -from api.serializers.user import UserSerializer, UserSaveSerializer +from api.serializers.user import UserSerializer +from api.decorators.permission import check_admin_permission +from api.services.user import update_permissions class UserViewSet(GenericViewSet): """ @@ -16,8 +20,6 @@ class UserViewSet(GenericViewSet): serializer_classes = { 'default': UserSerializer, - 'update': UserSaveSerializer, - 'create': UserSaveSerializer, } @@ -28,6 +30,25 @@ def get_serializer_class(self): return self.serializer_classes['default'] + @action(detail=False, methods=['post']) + @method_decorator(check_admin_permission()) + def new(self, request): + user_to_insert = request.data['idir'].upper() + try: + User.objects.get_or_create(idir=user_to_insert) + return Response(user_to_insert, status=status.HTTP_200_OK) + except Exception as e: + return Response({"response": str(e)}, status=status.HTTP_400_BAD_REQUEST) + + @action(detail=False, methods=['put']) + @method_decorator(check_admin_permission()) + def update_permissions(self, request): + error_msg = update_permissions(self, request) + if error_msg: + return Response(error_msg, status=status.HTTP_400_BAD_REQUEST) + else: + return Response('User permissions were updated!', status=status.HTTP_201_CREATED) + @action(detail=False) def current(self, request): """ @@ -37,20 +58,8 @@ def current(self, request): serializer = self.get_serializer(user) return Response(serializer.data) + @method_decorator(check_admin_permission()) def list(self, request): - request = self.request - ##check if user is admin before producing list of all users - users = User.objects.all() - current_user = users.filter(idir=request.user).first() - if current_user: - current_user_serializer = UserSerializer(current_user) - current_user_permissions = current_user_serializer.data['user_permissions'] - is_admin = False - if current_user_permissions: - for i in current_user_permissions: - for v in i.values(): - if v == 'admin': - is_admin = True - if is_admin == True: - serializer = UserSerializer(users, many=True) - return Response(serializer.data) \ No newline at end of file + users = User.objects.all().order_by('idir') + serializer = UserSerializer(users, many=True) + return Response(serializer.data) \ No newline at end of file diff --git a/react/package.json b/react/package.json index 9ec00bf3..32b09dc4 100644 --- a/react/package.json +++ b/react/package.json @@ -11,6 +11,7 @@ "axios": "^0.24.0", "buffer": "^6.0.3", "crypto-browserify": "^3.12.0", + "immer": "^10.0.4", "jsonwebtoken": "^8.5.1", "keycloak-js": "^15.0.2", "process": "^0.11.10", diff --git a/react/src/uploads/UploadContainer.js b/react/src/uploads/UploadContainer.js index 0f1a292a..d07f0681 100644 --- a/react/src/uploads/UploadContainer.js +++ b/react/src/uploads/UploadContainer.js @@ -19,6 +19,7 @@ const UploadContainer = () => { const [replaceData, setReplaceData] = useState('false'); // if true, we will replace all const [alertContent, setAlertContent] = useState(); const [alert, setAlert] = useState(false); + const [currentUser, setCurrentUser] = useState(''); const [alertSeverity, setAlertSeverity] = useState(''); // existing data with what is being uploaded const [open, setOpen] = useState(false); @@ -42,9 +43,9 @@ const UploadContainer = () => { setDatasetList(response.data); setLoading(false); axios.get(ROUTES_USERS.CURRENT).then((currentUserResp) => { - const permissions = currentUserResp.data.user_permissions.map((each) => each.description); - if (permissions.includes('admin')) { + if (currentUserResp.data.user_permissions.admin === true) { setAdminUser(true); + setCurrentUser(currentUserResp.data.idir); } }); }); @@ -148,7 +149,7 @@ const UploadContainer = () => { {adminUser && ( - + )} diff --git a/react/src/users/UsersContainer.js b/react/src/users/UsersContainer.js index d12f334e..ceada6ed 100644 --- a/react/src/users/UsersContainer.js +++ b/react/src/users/UsersContainer.js @@ -1,25 +1,69 @@ import { withRouter } from 'react-router-dom'; -import CircularProgress from '@mui/material/CircularProgress'; -import React, { useState, useEffect } from 'react'; +import PropTypes from 'prop-types'; +import { CircularProgress, Alert } from '@mui/material'; +import React, { useState, useEffect, useCallback } from 'react'; +import { produce } from 'immer'; import ROUTES_USERS from './routes'; import UsersPage from './components/UsersPage'; import useAxios from '../app/utilities/useAxios'; -const UsersContainer = () => { +const UsersContainer = (props) => { + const { currentUser } = props; const [loading, setLoading] = useState(false); const [users, setUsers] = useState([]); - const [userUpdates, setUserUpdates] = useState([]); - const axios = useAxios() + const [newUser, setNewUser] = useState(''); + const [permissionMessage, setPermissionMessage] = useState(''); + const [messageSeverity, setMessageSeverity] = useState(''); + const axios = useAxios(); - const refreshDetails = () => { + const handleCheckboxChange = useCallback((event) => { + const idir = event.target.name; + const permissionType = event.target.id; + const { checked } = event.target; + setUsers( + produce((draft) => { + const user = draft.find((user) => user.idir === idir); + user.user_permissions[permissionType] = checked; + }), + ); + }, []); + + const handleAddNewUser = () => { + axios.post(ROUTES_USERS.CREATE, { idir: newUser }) + .then((response) => { + const userAdded = response.data; + setMessageSeverity('success'); + setPermissionMessage(`${userAdded} was added to the user list`); + const userObject = { idir: userAdded, user_permissions: { admin: false, uploader: false } }; + setUsers( + produce((draft) => { + draft.push(userObject); + }), + ); + }) + .catch((error) => { + setMessageSeverity('error'); + setPermissionMessage('new user could not be added, sorry!'); + }); + }; + + const handleSubmitPermissionUpdates = () => { + axios.put(ROUTES_USERS.UPDATE, users) + .then((response) => { + setMessageSeverity('success'); + setPermissionMessage(response.data); + }) + .catch((error) => { + setMessageSeverity('error'); + setPermissionMessage(error.data); + }); + }; + + useEffect(() => { setLoading(true); axios.get(ROUTES_USERS.LIST).then((listResponse) => { setUsers(listResponse.data); }); - }; - - useEffect(() => { - refreshDetails(); setLoading(false); }, []); @@ -32,8 +76,19 @@ const UsersContainer = () => { } return (
- + {permissionMessage && {permissionMessage}} +
); }; +UsersContainer.propTypes = { + currentUser: PropTypes.string.isRequired, +}; export default withRouter(UsersContainer); diff --git a/react/src/users/components/UsersPage.js b/react/src/users/components/UsersPage.js index 68a00ace..e6076840 100644 --- a/react/src/users/components/UsersPage.js +++ b/react/src/users/components/UsersPage.js @@ -1,33 +1,32 @@ import React from 'react'; import PropTypes from 'prop-types'; import { - Box, Button, Grid, TextField, Checkbox, + Box, Button, Grid, TextField, Checkbox, Tooltip, } from '@mui/material'; import ClearIcon from '@mui/icons-material/Clear'; import SaveIcon from '@mui/icons-material/Save'; const UsersPage = (props) => { - const { users, userUpdates, setUserUpdates } = props; - + const { + currentUser, + users, + handleAddNewUser, + setNewUser, + handleCheckboxChange, + handleSubmitPermissionUpdates, + } = props; const userRow = (user) => { - const userPerms = { admin: false, uploader: false }; - user.user_permissions.forEach((permission) => { - userPerms[permission.description] = true; - }); - - const handleRadioChange = (event) => { - const { checked } = event.target; - const permissionType = event.target.id; - console.log(permissionType); - console.log(userPerms); - userPerms[permissionType] = checked; - console.log(userPerms[permissionType]); - }; + const disableAdmin = currentUser === user.idir; return ( + - { handleRadioChange(event); }} /> - { handleRadioChange(event); }} /> + { handleCheckboxChange(event); }} /> + + + { handleCheckboxChange(event); }} /> + + {user.idir} @@ -53,10 +52,10 @@ const UsersPage = (props) => { - + { setNewUser(event.target.value); }} /> - @@ -76,7 +75,7 @@ const UsersPage = (props) => { userRow(user) ))} - @@ -87,5 +86,10 @@ const UsersPage = (props) => { }; UsersPage.propTypes = { users: PropTypes.arrayOf(PropTypes.shape()).isRequired, + handleAddNewUser: PropTypes.func.isRequired, + setNewUser: PropTypes.func.isRequired, + handleCheckboxChange: PropTypes.func.isRequired, + handleSubmitPermissionUpdates: PropTypes.func.isRequired, + currentUser: PropTypes.string.isRequired, }; export default UsersPage; diff --git a/react/src/users/routes.js b/react/src/users/routes.js index 945af831..ef19fa16 100644 --- a/react/src/users/routes.js +++ b/react/src/users/routes.js @@ -3,7 +3,8 @@ const API_BASE_PATH = '/api/users'; const USERS = { LIST: API_BASE_PATH, CURRENT: `${API_BASE_PATH}/current`, - + CREATE: `${API_BASE_PATH}/new`, + UPDATE: `${API_BASE_PATH}/update_permissions`, }; export default USERS;