Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add New Client Page Implementation #63

Open
wants to merge 19 commits into
base: develop
Choose a base branch
from
Open

Add New Client Page Implementation #63

wants to merge 19 commits into from

Conversation

MelikaDab
Copy link
Contributor

@MelikaDab MelikaDab commented May 21, 2024

Developer: Melika Dabiri

Closes #48

Pull Request Summary

Implemented the Add New Client page:

  • Refactored the UI code into reusable components
  • Form is dynamic, the household member fields displayed are dependent on the number specified, and clicking the authorized member radio button will display corresponding fields. Unclicking the radio button will remove the authorized member fields. Similarly, reducing the number of children or adults will reduce the number of displayed fields accordingly.
  • Expanded API for householdMember and authorizedMember
  • Submitting the form will save the household and authorized members as well as the main client with its authMem and householdMem fields filled out accordingly

Modifications

addNewClient page, api folder, components folder

Testing Considerations

I did multiple tests adding clients with different numbers of household members, child and adult, and authorized members. Some points left to consider are:

  • When pulling the changes, I noticed a 'current' field in the householdMember schema. I have not yet had the chance to change the addNewClient code to include this field but it should be a very easy add on!
  • The authMem field in the client schema is an array, but to my knowledge the form does not allow for multiple authorized members (clicking the radio button only provides one set of input fields). I designed the code using the array format of authMem, but small changes will have to be made if we actually end up allowing the client to input multiple authorized members.
  • Deleting a client currently leaves their household members and authorized members in the database. Depending on our purposes, we might need to revise this so that deleting a client will also involve removing their corresponding household members and authorized members.
  • Further API endpoints can be added to get authorized/household members by their own id or by their head client id if necessary.

Lastly, the addNewClient page.tsx file has currently grown pretty lengthy. I would appreciate any feedback on whether the way I went about doing things could be simplified! I can also separate the helper functions into a separate file as some of them can also be repurposed as general utility functions for using the API.

Pull Request Checklist

  • Code is neat, readable, and works
  • Comments are appropriate
  • The commit messages follows our guidelines
  • The developer name is specified
  • The summary is completed
  • Assign reviewers

Screenshots/Screencast

image

Clicking Add Client saves the data to the database and logs the result on the console.

@MelikaDab MelikaDab self-assigned this May 21, 2024
@ajesus7 ajesus7 requested review from ajesus7 June 8, 2024 22:23
@ajesus7
Copy link
Contributor

ajesus7 commented Jun 10, 2024

Hi Melika, great job on all of the work on this page!!

I'm having an issue where when I try to create a new client, I get:

and when I try to add a child to the client, the following message appears:

  • "TypeError: Cannot read properties of undefined (reading 'map') at eval (page.tsx:165:24)".

Possible Fixes?

Did you happen to run into either of these issues? I tried adding the client without logging in and then with logging in but encountered the same problem.

  • I feel like maybe the issue could be related to some form of input validation and my text inputs not matching what the server is expecting? I'm not entirely sure what format the date or phone number should be submitted as.

Are you able to share what inputs you used to make the form work correctly, and I can try to replicate on my end.

Actionable Item

- I think adding some placeholders within the input tags with the desired input format such as "### ### ####" for phone number, might mitigate some of these input issues (if this is what is causing the problem).

Smaller Notes

  • There is a hydration error because of a div being nested within a label tag on the AddNewClientPage, but I wasn't able to find exactly where this was occurring. This can probably be made as its own bug fix; the error popup from React obscures the Add Client submit button, haha.
  • I resolved a merge conflict where there were two versions for the 'caniuse-lite' module/dependency, and I accepted the current version which was more up to date.
  • Child and adult header tags (when created new child/adult) are serif instead of sans-serif.

All in all, really great work on this!! :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add New Client Implementation
2 participants