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

Refactor use of getUserDto schema between functions #453

Closed
annarhughes opened this issue Jun 6, 2024 · 1 comment
Closed

Refactor use of getUserDto schema between functions #453

annarhughes opened this issue Jun 6, 2024 · 1 comment
Labels
complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days help wanted Extra attention is needed maintenance Maintenance / chore work priority: high Should be prioritized next week or longer. state: approved Ready to go. Not blocked or pending.

Comments

@annarhughes
Copy link
Member

annarhughes commented Jun 6, 2024

Is your feature request related to a problem?

Most of our backend api requests are related to users, so before the request function is run, we get the current user from the database and make the user data available in the request and following functions. The problem is that we are passing around the user data in a serialized schema GetUserDto and instead we should be using the pure UserEntity schema/types, for all internal functions. GetUserDto schema should only be used to serialize user data before returning the response, i.e. in the controller

Describe a solution:

  • Remove user from the request context - we are aiming to use userEntity instead - see FirebaseAuthGuard.tsx and getUserByFirebaseId function in user.service.ts
  • Replace all uses of req['user'] in controllers, instead using req['userEntity']
  • Remove all uses of GetUserDto from our functions/service files, instead using UserEntity type
  • Fix any related issues - running yarn test and yarn lint will help discover issues
  • You should also run Cypess tests to ensure this doesn't break anything in the frontend.
  • This is a large issue so you may want to break down into smaller issues

This is the first step (see #454) in improving the maintainability and performance of the API and how it handles users. The cleaner typing will help future contributors understand the code!

Additional context and resources:

Tests may need to be fixed following this update - run to check failing tests

Ticket breakdown

#547
#546
#545

@annarhughes annarhughes added complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days priority: high Should be prioritized next week or longer. maintenance Maintenance / chore work labels Jun 6, 2024
@kyleecodes kyleecodes added this to the 04. Maintenance and Testing milestone Jun 6, 2024
@eleanorreem eleanorreem added the state: approved Ready to go. Not blocked or pending. label Jul 18, 2024
@eleanorreem
Copy link
Contributor

eleanorreem commented Jul 22, 2024

I am going to break this down into bitesized chunks as I think this refactor will be too mammoth on its own. I have linked above

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days help wanted Extra attention is needed maintenance Maintenance / chore work priority: high Should be prioritized next week or longer. state: approved Ready to go. Not blocked or pending.
Projects
None yet
Development

No branches or pull requests

3 participants