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.

Sign up for GitHub

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

Jump to bottom

Refactor getUserProfile subscriptions #430

Open
5 tasks
eleanorreem opened this issue May 23, 2024 · 8 comments · May be fixed by #710
Open
5 tasks

Refactor getUserProfile subscriptions #430

eleanorreem opened this issue May 23, 2024 · 8 comments · May be fixed by #710
Assignees
@leoseg
Labels
complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days help wanted Extra attention is needed javascript Pull requests that update Javascript code maintenance Maintenance / chore work priority: high Should be prioritized next week or longer. state: blocked Task is blocked.
Milestone
02. Roadmaps

Comments

@eleanorreem
Copy link
Contributor

eleanorreem commented May 23, 2024 •
edited
Loading

Overview

We want to start breaking up the GET /user/me request and only fetch information on the pages it is needed. This is starting with subscriptions. We created a new endpoint in this ticket to have a bespoke endpoint for getting subscriptions . The frontend migration is in this ticket and blocks this refactor. After the frontend has been migrated over, we can start breaking up the GetUserDto and remove subscriptions from the object.

Action Items

  • Remove leftJoins of subscription and subscription_user table from the database query in userService.getUserByFirebaseId.
  • This will impact the firebase module and therefore the rest of the site. There may be significant refactors ahead.
  • You will need to remove subscription_user and subscription imports from most modules as they will no longer be required.
  • Remove all instances of ISubscriptionUser. It should either be an entity or a dto.
  • Remove subscriptions property from the GetUserDto.
@eleanorreem eleanorreem added help wanted Extra attention is needed complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days state: blocked Task is blocked. maintenance Maintenance / chore work labels May 23, 2024
@kyleecodes kyleecodes added this to the 02. Roadmaps milestone May 27, 2024
@kyleecodes kyleecodes moved this to To-Do Bloom Backend in Chayn Tech Volunteer Advancement May 30, 2024
@eleanorreem eleanorreem mentioned this issue Jul 14, 2024
Merged
@eleanorreem eleanorreem changed the title Refactor getUserByFirebaseId subscriptions Refactor getUserProfile subscriptions Sep 10, 2024
@eleanorreem eleanorreem added priority: high Should be prioritized next week or longer. javascript Pull requests that update Javascript code state: approved Ready to go. Not blocked or pending. and removed state: blocked Task is blocked. labels Sep 10, 2024
@eleanorreem eleanorreem reopened this Sep 21, 2024
@kyleecodes kyleecodes added the hacktoberfest Hacktoberfest issues label Oct 4, 2024
@leoseg
Copy link
Contributor

leoseg commented Nov 16, 2024

Can I try this one? :)

@leoseg
Copy link
Contributor

leoseg commented Nov 17, 2024

And I have on question regarding the ticket description, I don't see any left join in the service method of userService.getUserByFirebaseId, as the typeorm query there doesnt specify the relations parameter, are you referring to the controller method with the same name which calls the service method getUserProfile?

@github-actions GitHub Actions
Copy link

Thank you @leoseg you have been assigned this issue!
Please follow the directions in our Contributing Guide. We look forward to reviewing your pull request. ✨

Support Chayn's mission? ⭐ Please star this repo to help us find more contributors like you!
Learn more about our impact and sign-up for our volunteer programsto join our mission!. 🌸

@eleanorreem
Copy link
Contributor Author

Ah yes sorry! There was a recent refactor which means its the getUserProfile function you want to address. However, I just noticed the frontend ticket hasn't been done so this ticket should be blocked 🤔 Would you be interested in doing that work at all? Otherwise you might need to hold out until that piece of work is done by someone else.

@eleanorreem eleanorreem added state: blocked Task is blocked. and removed state: approved Ready to go. Not blocked or pending. labels Nov 18, 2024
@leoseg
Copy link
Contributor

leoseg commented Nov 19, 2024

I already started, I could do the refactoring write some unittests and test manually trough swagger, and open a pull request for a first review after that :) The end2end testing with cypress I could do later when its ready. One quick question: which API token do I use for swagger it is the one from firebase? because this one is not working for me

leoseg added a commit to leoseg/bloom-backend that referenced this issue Nov 19, 2024
@leoseg
chaynHQ#430
a61cec5 
Removed joins from user.service.ts getUserProfile with subscriptionUser and removed the getUserDto fields, the corresponding methods in serialize.ts,  as well as the ISubscription interface
leoseg added a commit to leoseg/bloom-backend that referenced this issue Nov 19, 2024
@leoseg
chaynHQ#430
93cac89 
Added unittests for getUserProfile
leoseg added a commit to leoseg/bloom-backend that referenced this issue Nov 19, 2024
@leoseg
chaynHQ#430
11c259a 
Fixxed docker-compose for linux
@leoseg leoseg linked a pull request Nov 19, 2024 that will close this issue
Open
@leoseg
Copy link
Contributor

leoseg commented Nov 19, 2024

#710 I opened a first pull request for a first review, I also edited the docker-compose as proposed in #588 as i needed to do it to get docker-compose running on my ubuntu :) I can do further e2e testing when the frontend is ready :) @eleanorreem

@eleanorreem
Copy link
Contributor Author

@leoseg thanks for opening the PR.

I haven't used Swagger to test API endpoints but I'm imagining you need to have the Firebase Env variables for the backend. Once you have that, I usually use postman and hit the /auth/signin endpoint with a JSON body of {email: string, password: string}. It should return you a token which you can copy into your request headers.

@kyleecodes I don't know if we have any documentation about this but this would be a good thing for us to do!

@leoseg
Copy link
Contributor

leoseg commented Nov 21, 2024

Thanks :) Manual testing trough swagger API seems to work, when the frontend is up to date I will do further tests with cypress!

@kyleecodes kyleecodes removed the hacktoberfest Hacktoberfest issues label Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@leoseg leoseg

Labels
complexity: advanced Time needed to do this ticket will be large e.g. 2-3+ days help wanted Extra attention is needed javascript Pull requests that update Javascript code maintenance Maintenance / chore work priority: high Should be prioritized next week or longer. state: blocked Task is blocked.
Projects
None yet
Milestone
02. Roadmaps
Development

Successfully merging a pull request may close this issue.

Chore(maintenance)/refactor get user profile subscriptions #430 leoseg/bloom-backend
3 participants
@eleanorreem @kyleecodes @leoseg