Conversation
| import { apiPathPerson } from "@/config/constants"; | ||
| import { useParams } from "next/navigation"; | ||
|
|
||
| export default function RacsPage() { |
There was a problem hiding this comment.
What do you think of using /agents/:id instead? Is this the same as this page: src/app/[lang]/dashboard/agents/[id]/page.tsx?
|
On another note: Since there is a contact details section for the volunteer as well as the opportunity, you might want to consider matching it. |
5baadcd to
65105b2
Compare
|
hey @HansKre , thank you for your feedback & both very good points. I've now rebased and refactored the agent contact details so that I'm using the |
HansKre
left a comment
There was a problem hiding this comment.
Looks pretty good. A few comments, only because codebase has changed quite a bit since you started working on the ticket. Sorry for the inconvenience.
| gap: ${(props) => (props.$isEditing ? "var(--spacing-16)" : "0")}; | ||
| `; | ||
|
|
||
| const Details = styled.div` |
There was a problem hiding this comment.
May I ask you to move styled-components to styles.ts, to match established patterns and keep the actual component lean?
There was a problem hiding this comment.
sure thing, have done it now
| width: 100%; | ||
| `; | ||
|
|
||
| const FieldWrapper = styled.div<{ $hasError?: boolean }>` |
There was a problem hiding this comment.
| const FieldWrapper = styled.div<{ $hasError?: boolean }>` | |
| const FieldWrapper = styled.div<HasError>` |
| import { AgentRoles } from "@/config/constants"; | ||
| import { ApiAgentProfileGet } from "../../../types"; | ||
|
|
||
| const Container = styled.div<{ $isEditing: boolean }>` |
There was a problem hiding this comment.
| const Container = styled.div<{ $isEditing: boolean }>` | |
| const Container = styled.div<IsEditing>` |
|
|
||
| const schema = createAgentContactDetailsSchema(t); | ||
|
|
||
| const initialFormValues = useMemo((): AgentContactDetailsFormData => { |
There was a problem hiding this comment.
Can we do something like:
| const initialFormValues = useMemo((): AgentContactDetailsFormData => { | |
| const initialFormValues = agent?.contactDetails; |
| contactDetails: { | ||
| firstName: values.firstName, | ||
| lastName: values.lastName, | ||
| middleName: values.middleName, | ||
| role: values?.role?.length ? (values.role as AgentRoles[]) : undefined, | ||
| email: values.email, | ||
| phone: values.phone, | ||
| landline: values.landline, | ||
| }, |
There was a problem hiding this comment.
| contactDetails: { | |
| firstName: values.firstName, | |
| lastName: values.lastName, | |
| middleName: values.middleName, | |
| role: values?.role?.length ? (values.role as AgentRoles[]) : undefined, | |
| email: values.email, | |
| phone: values.phone, | |
| landline: values.landline, | |
| }, | |
| contactDetails: { | |
| ...values | |
| }, |
| return ( | ||
| <Container $isEditing={isEditing}> | ||
| <Details> | ||
| {isEditing ? ( |
There was a problem hiding this comment.
Can I kindly ask to extract and move the editing- and displaying-components into separate files, as done in this PR #230?
There was a problem hiding this comment.
thanks for providing the PR, sure thing, I have refactored it now
| phone: z | ||
| .string() | ||
| .min(1, t("dashboard.agentProfile.contactDetails.validation.mobileRequired")) | ||
| .regex(/^\+[0-9\s]+$/, t("dashboard.agentProfile.contactDetails.validation.mobileInvalid")), |
There was a problem hiding this comment.
Just an idea: could move this regex to somewhere central, so that it can be re-used here and in the other schemas
There was a problem hiding this comment.
sure, I've moved it to src/config/constants.ts and also modified the phone schemas for agent, opportunity and volunteer to include it. Let me know what you think
|
thank you @HansKre for checking, I've pushed my latest changes |
HansKre
left a comment
There was a problem hiding this comment.
Super clean and elegant 💪
5413a33 to
81c1c03
Compare
Description
Adds Agent/RAC Contact Details
Related Issues
Closes issue 219
Changes
Screenshots / Demos
As we don't have "role" and "landline" in the database yet, I had to mock a handler in the backend. Also I had some conflicts with the existing PATCH /person/:id, due to the lack of address on the form.
With this handler, you can successfully edit the details of: firstName, middleName, lastName, email and mobile.
However, if you want to successfully edit role or landline, you'll get a mock success response, but you won't be able to see the updated role/landline in the form.
src/server/routes/person.routes.tsI added this to person entity for now:
src/data/entity/person.entity.tsI didn't add this, I just commented out any reference to the address in the updatePerson for now
src/server/utils/data/for-routes.tsI tweaked the Person Schema to include Role and Landline for now
src/server/schema/person.schema.tsChecklist