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

backend rewrite #453

Open
wants to merge 22 commits into
base: main
Choose a base branch
from
Open

backend rewrite #453

wants to merge 22 commits into from
+14,675 −11,433

Conversation

dlustre
Copy link
Collaborator

@dlustre dlustre commented May 31, 2024 •
edited
Loading

Changes

  • universalDatePicker()
  • better ux for loading and error states for menu fetches. retry=false, so the loads are fast (untested for mobile though)
  • light/dark/system colorscheme setting

Note

Above was already merged.

  • extend test context to include db, api, and testdata for any apiTest()
  • create tables, services, procedures, and tests for ratings and users
  • upgrade to expo 51 and tamagui 1.100.0
  • migrate to pnpm catalogs
  • pull-to-refresh functionality
  • db diagram gen script
  • schema changes (see diagram below)

?

  • not sure if we should be caching jwt in asyncstorage for web but thats what i did for right now
    stopped caching jwt when on web.

Closes #457, Closes #447, Closes #449, Closes #437

demo

2024-05-30.22-22-15.mp4

ui for missing nutrition facts

image

generated diagram with db:diagram script

erd

@dlustre dlustre requested a review from bjsilva1 May 31, 2024 05:36
@bjsilva1
Copy link
Contributor

bjsilva1 commented Jun 3, 2024

Looks good 👍
I was able to run it well on android, but I got this error when running on web:

image

I think it has something to do with the metro config, I will keep looking into to it on my end.

@dlustre
Copy link
Collaborator Author

dlustre commented Jun 3, 2024

Looks good 👍 I was able to run it well on android, but I got this error when running on web:

image

I think it has something to do with the metro config, I will keep looking into to it on my end.

Web should work in one of the previous commits (37bad80) and we can narrow down and debug

@bjsilva1
Copy link
Contributor

bjsilva1 commented Jun 3, 2024 •
edited
Loading

I still get the same error on that commit, this is most likely a client side issue.

Also, I noticed this issue when trying to run pnpm install on the latest commit:
image

I think it's a simple fix of changing the version in the tooling/eslint/package.json

@dlustre
Copy link
Collaborator Author

dlustre commented Jun 3, 2024

I still get the same error on that commit, this is most likely a client side issue.

Also, I noticed this issue when trying to run pnpm install on the latest commit:

image

I think it's a simple fix of changing the version in the tooling/eslint/package.json

  • Yeah it's fixed in my upcoming commit

dlustre and others added 2 commits June 3, 2024 04:59
@dlustre
add ratings & users
16a430f 
- flatten api file structure
- refactor db to use latest drizzle ver
- add web fallback for useWarmupBrowser
@KatyH820
chore: change the port number
fa9add5
@bjsilva1 bjsilva1 mentioned this pull request Jun 4, 2024
Merged
3 tasks
@dlustre dlustre changed the title better ux for loading and error states for menu fetches create test fixtures to simplify api tests Jun 6, 2024
@dlustre dlustre added the backend label Jun 7, 2024
@dlustre dlustre changed the title create test fixtures to simplify api tests backend rewrite Jun 17, 2024
bjsilva1
bjsilva1 reviewed Sep 29, 2024
View reviewed changes
Copy link
Contributor

@bjsilva1 bjsilva1 left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good, going to make note of some of the TODOs left in the pr

TODO

Backend

  • Add separate tests in services.test.ts that test upserts independently of database queries.
  • Add back the getMenu procedure with router tests
  • Flush out the push notification system. We will start off by adding simple push notification tests and error handling.
  • Design the system for push notifications to work with pinned items
  • [Minor] Add tests for getRestaurantByDate
  • Add test for daily and weekly server functions
  • Event scraping tests using test HTML file
  • Refactor weekly cron job into its own function? (up for debate)

Still need to test front end, will do once my docker decides to work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bjsilva1 bjsilva1 bjsilva1 left review comments

Assignees
No one assigned
Labels
backend frontend
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Error in Serverless Deployment Store User Data Food Ratings (backend) Upgrade to Expo 51
3 participants
@dlustre @bjsilva1 @KatyH820