From cda3bc5a59869bf89965eae6e5289421e56420a3 Mon Sep 17 00:00:00 2001 From: David Paul Graham <43794491+dpgraham4401@users.noreply.github.com> Date: Fri, 23 Aug 2024 13:46:50 -0500 Subject: [PATCH] Icon cosmetic fixes and avatar component (#778) * remove exception exposure in profile views * use composition (with mixins) instead of inheritence to implement to_representation method and remove black fields * fix manifest router url patterns * fix styles for icons * add avatar component * use avatar component in profile feature --- .../Manifest/Actions/ManifestCancelBtn.tsx | 2 +- .../Manifest/Actions/ManifestEditBtn.tsx | 2 +- .../Manifest/Actions/ManifestSaveBtn.tsx | 2 +- .../components/RcraSite/RcraSiteDetails.tsx | 10 ++-- .../SyncManifestBtn/SyncManifestBtn.tsx | 4 +- client/app/components/User/UserInfoForm.tsx | 19 ++++---- .../legacyUi/HtPaginate/HtPageBtns.tsx | 8 ++-- .../legacyUi/HtTooltip/HtTooltip.tsx | 8 ++-- .../app/components/ui/Avatar/Avatar.spec.tsx | 17 +++++++ client/app/components/ui/Avatar/Avatar.tsx | 48 +++++++++++++++++++ client/app/components/ui/Button/Button.tsx | 8 ++-- client/app/components/ui/index.ts | 2 + client/package-lock.json | 31 +++++++++++- client/package.json | 1 + .../serializers/handler_serializer.py | 9 ---- server/manifest/serializers/manifest.py | 33 +++---------- server/manifest/serializers/mixins.py | 17 +++++++ server/manifest/serializers/signatures.py | 30 ++---------- server/manifest/urls.py | 21 +++----- server/manifest/views.py | 2 +- server/profile/views.py | 24 ++++------ 21 files changed, 175 insertions(+), 123 deletions(-) create mode 100644 client/app/components/ui/Avatar/Avatar.spec.tsx create mode 100644 client/app/components/ui/Avatar/Avatar.tsx create mode 100644 server/manifest/serializers/mixins.py diff --git a/client/app/components/Manifest/Actions/ManifestCancelBtn.tsx b/client/app/components/Manifest/Actions/ManifestCancelBtn.tsx index 28c80a57..6f00da38 100644 --- a/client/app/components/Manifest/Actions/ManifestCancelBtn.tsx +++ b/client/app/components/Manifest/Actions/ManifestCancelBtn.tsx @@ -28,7 +28,7 @@ export function ManifestCancelBtn() { }} > Cancel - + ); } diff --git a/client/app/components/Manifest/Actions/ManifestEditBtn.tsx b/client/app/components/Manifest/Actions/ManifestEditBtn.tsx index e5abf1e4..c8527f6c 100644 --- a/client/app/components/Manifest/Actions/ManifestEditBtn.tsx +++ b/client/app/components/Manifest/Actions/ManifestEditBtn.tsx @@ -22,7 +22,7 @@ export function ManifestEditBtn({ children: _unused, ...props }: ManifestEditBtn return ( Edit - + ); } diff --git a/client/app/components/Manifest/Actions/ManifestSaveBtn.tsx b/client/app/components/Manifest/Actions/ManifestSaveBtn.tsx index ee79b8d7..7bb63c1b 100644 --- a/client/app/components/Manifest/Actions/ManifestSaveBtn.tsx +++ b/client/app/components/Manifest/Actions/ManifestSaveBtn.tsx @@ -11,7 +11,7 @@ export function ManifestSaveBtn({ children: _unused, ...props }: ManifestSaveBtn return ( Save - + ); } diff --git a/client/app/components/RcraSite/RcraSiteDetails.tsx b/client/app/components/RcraSite/RcraSiteDetails.tsx index f2c3dadc..57ba83f6 100644 --- a/client/app/components/RcraSite/RcraSiteDetails.tsx +++ b/client/app/components/RcraSite/RcraSiteDetails.tsx @@ -39,20 +39,20 @@ export function RcraSiteDetails({ handler }: RcraSiteDetailsProps) {

- Can e-Sign:{' '} + Can e-Sign: {handler.canEsign ? ( - + ) : ( - + )}

Has registered e-Manifest user:{' '} {handler.hasRegisteredEmanifestUser ? ( - + ) : ( - + )}

diff --git a/client/app/components/Rcrainfo/buttons/SyncManifestBtn/SyncManifestBtn.tsx b/client/app/components/Rcrainfo/buttons/SyncManifestBtn/SyncManifestBtn.tsx index d353fea1..5be6dd75 100644 --- a/client/app/components/Rcrainfo/buttons/SyncManifestBtn/SyncManifestBtn.tsx +++ b/client/app/components/Rcrainfo/buttons/SyncManifestBtn/SyncManifestBtn.tsx @@ -63,8 +63,8 @@ export function SyncManifestBtn({ if (siteId) syncSiteManifest(siteId); }} > - {`Sync Manifest `} - + Sync Manifest + ); } diff --git a/client/app/components/User/UserInfoForm.tsx b/client/app/components/User/UserInfoForm.tsx index cba1c6aa..2e492770 100644 --- a/client/app/components/User/UserInfoForm.tsx +++ b/client/app/components/User/UserInfoForm.tsx @@ -2,11 +2,11 @@ import { zodResolver } from '@hookform/resolvers/zod'; import React, { createRef, useState } from 'react'; import { Button, Col, Form, Row } from 'react-bootstrap'; import { useForm } from 'react-hook-form'; -import { FaUser } from 'react-icons/fa'; import { z } from 'zod'; import { HtForm } from '~/components/legacyUi'; -import { Spinner } from '~/components/ui'; +import { Avatar, AvatarFallback, AvatarImage, Spinner } from '~/components/ui'; import { HaztrakUser, ProfileSlice, useUpdateUserMutation } from '~/store'; +import { FaUser } from 'react-icons/fa'; interface UserProfileProps { user: HaztrakUser; @@ -53,12 +53,15 @@ export function UserInfoForm({ user }: UserProfileProps) {
- +

{user.username}

diff --git a/client/app/components/legacyUi/HtPaginate/HtPageBtns.tsx b/client/app/components/legacyUi/HtPaginate/HtPageBtns.tsx index 15fe858e..abcc0c72 100644 --- a/client/app/components/legacyUi/HtPaginate/HtPageBtns.tsx +++ b/client/app/components/legacyUi/HtPaginate/HtPageBtns.tsx @@ -36,10 +36,10 @@ export function HtPageBtns({ table }: HtPageBtnsProps) { onClick={() => table.setPageIndex(0)} disabled={!table.getCanPreviousPage()} > - + - + {paginationRange.map((pageNumber, index) => { @@ -58,13 +58,13 @@ export function HtPageBtns({ table }: HtPageBtnsProps) { ); })} - + table.setPageIndex(table.getPageCount() - 1)} disabled={!table.getCanNextPage()} > - +
diff --git a/client/app/components/legacyUi/HtTooltip/HtTooltip.tsx b/client/app/components/legacyUi/HtTooltip/HtTooltip.tsx index ee2dd020..780d5ec4 100644 --- a/client/app/components/legacyUi/HtTooltip/HtTooltip.tsx +++ b/client/app/components/legacyUi/HtTooltip/HtTooltip.tsx @@ -1,4 +1,4 @@ -import { ReactElement } from 'react'; +import { forwardRef, ReactElement, Ref } from 'react'; import { OverlayTrigger, Tooltip, TooltipProps } from 'react-bootstrap'; import { FaInfoCircle } from 'react-icons/fa'; @@ -9,14 +9,14 @@ interface HtToolTipProps extends TooltipProps { children: ReactElement; } -export function HtTooltip(props: HtToolTipProps): ReactElement { +export const HtTooltip = forwardRef, HtToolTipProps>((props, _ref) => { const { children, text, ...rest } = props; return ( {text}}> {children} ); -} +}); interface InfoTooltipProps { message: string; @@ -30,7 +30,7 @@ interface InfoTooltipProps { export function InfoIconTooltip({ message }: InfoTooltipProps) { return ( - + ); } diff --git a/client/app/components/ui/Avatar/Avatar.spec.tsx b/client/app/components/ui/Avatar/Avatar.spec.tsx new file mode 100644 index 00000000..80b9eb75 --- /dev/null +++ b/client/app/components/ui/Avatar/Avatar.spec.tsx @@ -0,0 +1,17 @@ +import { render } from '@testing-library/react'; +import { describe, expect, it } from 'vitest'; +import { Avatar } from './Avatar'; + +describe('Avatar component', () => { + it('renders with default props', () => { + const { container } = render(); + expect(container.firstChild).toHaveClass( + 'tw-relative tw-flex tw-h-10 tw-w-10 tw-shrink-0 tw-overflow-hidden tw-rounded-full' + ); + }); + + it('applies additional class names', () => { + const { container } = render(); + expect(container.firstChild).toHaveClass('custom-class'); + }); +}); diff --git a/client/app/components/ui/Avatar/Avatar.tsx b/client/app/components/ui/Avatar/Avatar.tsx new file mode 100644 index 00000000..c5ac180a --- /dev/null +++ b/client/app/components/ui/Avatar/Avatar.tsx @@ -0,0 +1,48 @@ +import * as React from 'react'; +import * as AvatarPrimitive from '@radix-ui/react-avatar'; + +import { cn } from '~/lib/utils'; + +const Avatar = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef +>(({ className, ...props }, ref) => ( + +)); +Avatar.displayName = AvatarPrimitive.Root.displayName; + +const AvatarImage = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef +>(({ className, ...props }, ref) => ( + +)); +AvatarImage.displayName = AvatarPrimitive.Image.displayName; + +const AvatarFallback = React.forwardRef< + React.ElementRef, + React.ComponentPropsWithoutRef +>(({ className, ...props }, ref) => ( + +)); +AvatarFallback.displayName = AvatarPrimitive.Fallback.displayName; + +export { Avatar, AvatarImage, AvatarFallback }; diff --git a/client/app/components/ui/Button/Button.tsx b/client/app/components/ui/Button/Button.tsx index baf9e66f..fd5524e3 100644 --- a/client/app/components/ui/Button/Button.tsx +++ b/client/app/components/ui/Button/Button.tsx @@ -5,7 +5,7 @@ import * as React from 'react'; import { cn } from '~/lib/utils'; const buttonVariants = cva( - 'tw-text-md tw-inline-flex tw-items-center tw-justify-center tw-whitespace-nowrap tw-font-medium tw-ring-offset-background tw-transition-colors focus-visible:tw-outline-none focus-visible:tw-ring-2 focus-visible:tw-ring-ring focus-visible:tw-ring-offset-2 disabled:tw-pointer-events-none disabled:tw-opacity-50', + 'tw-text-md tw-inline-flex tw-items-center tw-justify-center tw-whitespace-nowrap tw-p-2 tw-font-medium tw-ring-offset-background tw-transition-colors focus-visible:tw-outline-none focus-visible:tw-ring-2 focus-visible:tw-ring-ring focus-visible:tw-ring-offset-2 disabled:tw-pointer-events-none disabled:tw-opacity-50', { variants: { variant: { @@ -18,9 +18,9 @@ const buttonVariants = cva( link: 'tw-text-primary tw-underline-offset-4 hover:tw-underline', }, size: { - default: 'tw-h-10 tw-px-4 tw-py-2', - sm: 'tw-h-9 tw-rounded-md tw-px-3', - lg: 'tw-h-11 tw-rounded-md tw-px-8', + default: 'tw-h-10', + sm: 'tw-h-9 tw-px-3', + lg: 'tw-h-11 tw-px-8', icon: 'tw-h-10 tw-w-10', }, rounded: { diff --git a/client/app/components/ui/index.ts b/client/app/components/ui/index.ts index 48efccf0..466d9c3a 100644 --- a/client/app/components/ui/index.ts +++ b/client/app/components/ui/index.ts @@ -25,3 +25,5 @@ export { export { Card, CardContent, CardDescription, CardFooter, CardHeader, CardTitle } from './Card/Card'; export { Spinner } from './Spinner/Spinner'; + +export { Avatar, AvatarImage, AvatarFallback } from './Avatar/Avatar'; diff --git a/client/package-lock.json b/client/package-lock.json index 225bb004..457927db 100644 --- a/client/package-lock.json +++ b/client/package-lock.json @@ -1,16 +1,17 @@ { "name": "haztrak", - "version": "0.7.2", + "version": "0.8.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "haztrak", - "version": "0.7.2", + "version": "0.8.0", "dependencies": { "@formkit/auto-animate": "^0.8.2", "@hookform/error-message": "^2.0.1", "@hookform/resolvers": "^3.7.0", + "@radix-ui/react-avatar": "^1.1.0", "@radix-ui/react-dialog": "^1.1.1", "@radix-ui/react-dropdown-menu": "^2.1.1", "@radix-ui/react-separator": "^1.1.0", @@ -1575,6 +1576,32 @@ } } }, + "node_modules/@radix-ui/react-avatar": { + "version": "1.1.0", + "resolved": "https://registry.npmjs.org/@radix-ui/react-avatar/-/react-avatar-1.1.0.tgz", + "integrity": "sha512-Q/PbuSMk/vyAd/UoIShVGZ7StHHeRFYU7wXmi5GV+8cLXflZAEpHL/F697H1klrzxKXNtZ97vWiC0q3RKUH8UA==", + "license": "MIT", + "dependencies": { + "@radix-ui/react-context": "1.1.0", + "@radix-ui/react-primitive": "2.0.0", + "@radix-ui/react-use-callback-ref": "1.1.0", + "@radix-ui/react-use-layout-effect": "1.1.0" + }, + "peerDependencies": { + "@types/react": "*", + "@types/react-dom": "*", + "react": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc", + "react-dom": "^16.8 || ^17.0 || ^18.0 || ^19.0 || ^19.0.0-rc" + }, + "peerDependenciesMeta": { + "@types/react": { + "optional": true + }, + "@types/react-dom": { + "optional": true + } + } + }, "node_modules/@radix-ui/react-collection": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/@radix-ui/react-collection/-/react-collection-1.1.0.tgz", diff --git a/client/package.json b/client/package.json index 75136e58..5a57ce66 100644 --- a/client/package.json +++ b/client/package.json @@ -20,6 +20,7 @@ "@formkit/auto-animate": "^0.8.2", "@hookform/error-message": "^2.0.1", "@hookform/resolvers": "^3.7.0", + "@radix-ui/react-avatar": "^1.1.0", "@radix-ui/react-dialog": "^1.1.1", "@radix-ui/react-dropdown-menu": "^2.1.1", "@radix-ui/react-separator": "^1.1.0", diff --git a/server/manifest/serializers/handler_serializer.py b/server/manifest/serializers/handler_serializer.py index 03efbf48..89589b82 100644 --- a/server/manifest/serializers/handler_serializer.py +++ b/server/manifest/serializers/handler_serializer.py @@ -2,7 +2,6 @@ from rcrasite.serializers import RcraSiteSerializer from rest_framework import serializers -from rest_framework.exceptions import ValidationError from manifest.models import Handler, ManifestPhone, Transporter @@ -95,11 +94,3 @@ class Meta: "electronicSignaturesInfo", "signed", ] - - def to_internal_value(self, data): - """Move fields related to rcra_site to an internal rcra_site dictionary.""" - try: - internal = super().to_internal_value(data) - return internal - except ValidationError as exc: - raise exc diff --git a/server/manifest/serializers/manifest.py b/server/manifest/serializers/manifest.py index f73ad771..b793584f 100644 --- a/server/manifest/serializers/manifest.py +++ b/server/manifest/serializers/manifest.py @@ -1,5 +1,5 @@ import logging -from typing import Dict, override +from typing import Dict from rcrasite.models import RcraStates from rest_framework import serializers @@ -18,33 +18,12 @@ TransporterSerializer, ) -logger = logging.getLogger(__name__) - - -class ManifestBaseSerializer(serializers.ModelSerializer): - def remove_empty_fields(self, data): - """Remove empty fields when serializing""" - for field in self.fields: - try: - if data[field] is None: - data.pop(field) - except KeyError: - pass - return data +from .mixins import RemoveEmptyFieldsMixin - @override - def to_representation(self, instance): - data = super().to_representation(instance) - return self.remove_empty_fields(data) - - def __str__(self): - return f"{self.__class__.__name__}" - - def __repr__(self): - return f"<{self.__class__.__name__}({self.data})>" +logger = logging.getLogger(__name__) -class AdditionalInfoSerializer(ManifestBaseSerializer): +class AdditionalInfoSerializer(RemoveEmptyFieldsMixin, serializers.ModelSerializer): originalManifestTrackingNumbers = serializers.JSONField( allow_null=True, required=False, @@ -84,7 +63,7 @@ class Meta: ) -class ManifestSerializer(ManifestBaseSerializer): +class ManifestSerializer(RemoveEmptyFieldsMixin, serializers.ModelSerializer): """Manifest serializer""" createdDate = serializers.DateTimeField( @@ -276,7 +255,7 @@ class Meta: ] -class PortOfEntrySerializer(ManifestBaseSerializer): +class PortOfEntrySerializer(RemoveEmptyFieldsMixin, serializers.ModelSerializer): """ Serializer for Port Of Entry """ diff --git a/server/manifest/serializers/mixins.py b/server/manifest/serializers/mixins.py new file mode 100644 index 00000000..5fe2448e --- /dev/null +++ b/server/manifest/serializers/mixins.py @@ -0,0 +1,17 @@ +from rest_framework import serializers + + +class RemoveEmptyFieldsMixin(serializers.Serializer): + def remove_empty_fields(self, data): + """Remove empty fields when serializing""" + for field in self.fields: + try: + if data[field] is None: + data.pop(field) + except KeyError: + pass + return data + + def to_representation(self, instance): + data = super().to_representation(instance) + return self.remove_empty_fields(data) diff --git a/server/manifest/serializers/signatures.py b/server/manifest/serializers/signatures.py index 2cb3cc24..2c420b05 100644 --- a/server/manifest/serializers/signatures.py +++ b/server/manifest/serializers/signatures.py @@ -6,29 +6,7 @@ from rest_framework import serializers from manifest.models import ESignature, PaperSignature, QuickerSign, Signer - - -class HandlerBaseSerializer(serializers.ModelSerializer): - # ToDo: remove - - def __str__(self): - return f"{self.__class__.__name__}" - - def __repr__(self): - return f"<{self.__class__.__name__}({self.data})>" - - def to_representation(self, instance): - """ - Remove empty fields when serializing - """ - data = super().to_representation(instance) - for field in self.fields: - try: - if data[field] is None: - data.pop(field) - except KeyError: - pass - return data +from manifest.serializers.mixins import RemoveEmptyFieldsMixin class QuickerSignSerializer(serializers.Serializer): @@ -94,7 +72,7 @@ class Meta: ] -class SignerSerializer(HandlerBaseSerializer): +class SignerSerializer(RemoveEmptyFieldsMixin, serializers.ModelSerializer): """Serializer for EPA Signer Object""" userId = serializers.CharField( @@ -147,7 +125,7 @@ class Meta: ] -class ESignatureSerializer(HandlerBaseSerializer): +class ESignatureSerializer(RemoveEmptyFieldsMixin, serializers.ModelSerializer): """Serializer for Electronic Signature on manifest""" signer = SignerSerializer( @@ -192,7 +170,7 @@ class Meta: ] -class PaperSignatureSerializer(HandlerBaseSerializer): +class PaperSignatureSerializer(RemoveEmptyFieldsMixin, serializers.ModelSerializer): """ Serializer for Paper Signature on manifest which indicates the change of custody with paper manifests diff --git a/server/manifest/urls.py b/server/manifest/urls.py index 5d88396a..e4bcfb19 100644 --- a/server/manifest/urls.py +++ b/server/manifest/urls.py @@ -10,7 +10,7 @@ ) manifest_router = SimpleRouter(trailing_slash=False) -manifest_router.register("", ManifestViewSet, basename="manifest") +manifest_router.register("manifest", ManifestViewSet) emanifest_patterns = ( [ @@ -32,17 +32,10 @@ app_name = "manifest" urlpatterns = [ - path( - "manifest", - include( - [ - # e-Manifest - path("/emanifest", include(emanifest_patterns)), - # Manifest Tracking Numbers - path("/mtn", include(mtn_patterns)), - # Manifests - path("/", include(manifest_router.urls)), - ] - ), - ), + # e-Manifest + path("manifest/emanifest", include(emanifest_patterns)), + # Manifest Tracking Numbers + path("manifest/mtn", include(mtn_patterns)), + # Manifests + path("", include(manifest_router.urls)), ] diff --git a/server/manifest/views.py b/server/manifest/views.py index c3df8351..56507b40 100644 --- a/server/manifest/views.py +++ b/server/manifest/views.py @@ -29,7 +29,7 @@ class ManifestViewSet(viewsets.GenericViewSet, mixins.RetrieveModelMixin, mixins allowed_methods = ["GET", "POST", "PUT"] queryset = Manifest.objects.all() serializer_class = ManifestSerializer - lookup_regex = "[0-9]{9}[a-zA-Z]{3}" + lookup_value_regex = "[0-9]{9}[a-zA-Z]{3}" @extend_schema(request=ManifestSerializer) def create(self, request: Request) -> Response: diff --git a/server/profile/views.py b/server/profile/views.py index 128c0e16..f80bb523 100644 --- a/server/profile/views.py +++ b/server/profile/views.py @@ -4,13 +4,16 @@ from celery.exceptions import CeleryError from celery.result import AsyncResult as CeleryTask +from rcrasite.tasks import sync_user_rcrainfo_sites from rest_framework import status -from rest_framework.generics import GenericAPIView, RetrieveAPIView, RetrieveUpdateAPIView +from rest_framework.generics import ( + CreateAPIView, + RetrieveAPIView, + RetrieveUpdateAPIView, +) from rest_framework.request import Request from rest_framework.response import Response -from rcrasite.tasks import sync_user_rcrainfo_sites - class ProfileDetailsView(RetrieveAPIView): """Displays a user's HaztrakProfile""" @@ -18,13 +21,8 @@ class ProfileDetailsView(RetrieveAPIView): queryset = Profile.objects.all() serializer_class = ProfileSerializer - def get(self, request, *args, **kwargs): - try: - profile = get_user_profile(user=self.request.user) - serializer = self.serializer_class(profile) - return Response(data=serializer.data, status=status.HTTP_200_OK) - except Profile.DoesNotExist as exc: - return Response(data={"error": str(exc)}, status=status.HTTP_404_NOT_FOUND) + def get_object(self): + return get_user_profile(user=self.request.user) class RcrainfoProfileRetrieveUpdateView(RetrieveUpdateAPIView): @@ -43,7 +41,7 @@ def get_object(self): return RcrainfoProfile.objects.get_by_trak_username(self.kwargs.get(self.lookup_url_kwarg)) -class RcrainfoProfileSyncView(GenericAPIView): +class RcrainfoProfileSyncView(CreateAPIView): """ This endpoint launches a task to sync the logged-in user's RCRAInfo profile with their haztrak (Rcra)profile. @@ -52,12 +50,10 @@ class RcrainfoProfileSyncView(GenericAPIView): queryset = None response = Response - def post(self, request: Request) -> Response: + def create(self, request: Request, **kwargs) -> Response: try: task: CeleryTask = sync_user_rcrainfo_sites.delay(str(self.request.user)) return self.response({"taskId": task.id}) - except RcrainfoProfile.DoesNotExist as exc: - return self.response(data={"error": str(exc)}, status=status.HTTP_404_NOT_FOUND) except CeleryError as exc: return self.response( data={"error": str(exc)}, status=status.HTTP_500_INTERNAL_SERVER_ERROR