Conversation
Dao-Ho
left a comment
There was a problem hiding this comment.
This is a good start, and definitely one of the flows we want. But some things to consider.
What you have: whenever a new user gets created under an org, you insert that user into an existing org/hotel in our DB.
Some questions to consider:
- What happens if a user is created before their org is created?
- What happens if the org is created on clerk, and a member is created under that org, but it isn't in our internal database. How do we handle that now?
I think this should give you a good direction on improving this.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #175 +/- ##
==========================================
+ Coverage 0.62% 19.07% +18.45%
==========================================
Files 78 48 -30
Lines 3218 2307 -911
==========================================
+ Hits 20 440 +420
+ Misses 3198 1852 -1346
- Partials 0 15 +15
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
Update on the PR for clerk orgs.
Debt:
Current FLOW:
B)
|
Dao-Ho
left a comment
There was a problem hiding this comment.
Just a couple of comments, otherwise this is awesome, great ship! 🚀 🚀 🚀
backend/internal/handler/clerk.go
Outdated
| hotel, err := h.HotelsRepository.FindByClerkOrgID(c.Context(), payload.Data.Organization.ID) | ||
| if err != nil { | ||
| if errors.Is(err, errs.ErrNotFoundInDB) { | ||
| return c.SendStatus(fiber.StatusServiceUnavailable) |
There was a problem hiding this comment.
I would actually return an internal error here if the hotel hasn't been inserted into our DB yet instead of service unavailable.
Although we don't have to in this pr, it might make sense to build a sync pipeline and trigger a sync and reinsert if this is the case. We can do that separately though
| return nil, err | ||
| } | ||
| return &hotel, nil | ||
| } |
There was a problem hiding this comment.
We can just have clerk org id as our hotel id, just like how clerk user id is our user id. They are the same and do not need a separate field.
You can then reformat your fetch to be just FindHotel(id)
| @@ -0,0 +1 @@ | |||
| ALTER TABLE public.hotels ADD COLUMN clerk_org_id text UNIQUE; | |||
There was a problem hiding this comment.
With comments above - we wont need this
Description
Type of Change
Related Issue(s)
Closes #
Related to #
What Changed?
Testing & Validation
How this was tested
Screenshots/Recordings
Unfinished Work & Known Issues
Notes & Nuances
Pre-Merge Checklist
Code Quality
Testing & CI
Documentation
Reviewer Notes