ai cooking
0
fork

Configure Feed

Select the types of activity you want to include in your feed.

Pmiller/signupbroke (#460)

* try and fix our conversions

* smaller

* move user exist to the user package

* tests passing

* okay simpler template

* get rid of cruft

* works

* test the right things

* fumpt

---------

Co-authored-by: paul miller <paul.miller>

authored by

Paul Miller
paul miller
and committed by
GitHub
5460dfb5 15152e89

+150 -50
+4 -7
internal/auth/clerk.go
··· 172 172 PublishableKey string 173 173 GoogleTagScript template.HTML 174 174 GoogleConversionTag string 175 - Signup bool 175 + UserExistsURL string 176 176 ReturnTo string // read from a data- attribute in the template to avoid JS-string escaping concerns 177 177 }{ 178 178 PublishableKey: c.cfg.Clerk.PublishableKey, 179 179 GoogleTagScript: templates.GoogleTagScript(), 180 180 GoogleConversionTag: templates.GoogleConversionTag(), 181 - Signup: strings.EqualFold(strings.TrimSpace(r.URL.Query().Get("signup")), "true"), // used for ad conversions 181 + UserExistsURL: "/user/exists", 182 182 ReturnTo: returnToFromRequest(r), 183 183 } 184 184 if err := templates.AuthEstablish.Execute(w, data); err != nil { ··· 193 193 if signup { 194 194 base = c.cfg.Clerk.Signup() 195 195 } 196 - redirectURL := c.authEstablishURL(signup, returnToFromRequest(r)) 196 + redirectURL := c.authEstablishURL(returnToFromRequest(r)) 197 197 u := lo.Must(url.Parse(base)) 198 198 q := u.Query() 199 199 q.Set("redirect_url", redirectURL) ··· 201 201 return u.String() 202 202 } 203 203 204 - func (c *clerkClient) authEstablishURL(signup bool, returnTo string) string { 204 + func (c *clerkClient) authEstablishURL(returnTo string) string { 205 205 origin := c.cfg.ResolvedPublicOrigin() // can never be emptpy 206 206 u := lo.Must(url.Parse(origin + "/auth/establish")) 207 207 q := u.Query() 208 - if signup { 209 - q.Set("signup", "true") 210 - } 211 208 if returnTo != "" { 212 209 // Keep the entire relative return target in one opaque query value so 213 210 // nested ?a=1&b=2 segments are not broken apart during Clerk redirects.
+4 -4
internal/recipes/server_test.go
··· 139 139 ) 140 140 t.Cleanup(s.Wait) 141 141 142 - currentUser, err := storage.FindOrCreateFromClerk(t.Context(), "mock-clerk-user-id", auth.DefaultMock()) 142 + req := httptest.NewRequest(http.MethodGet, "/recipes?location=70001001&date=2026-03-06&instructions=make+it+vegetarian", nil) 143 + currentUser, err := storage.FromRequest(t.Context(), req, auth.DefaultMock()) 143 144 if err != nil { 144 145 t.Fatalf("failed to seed user: %v", err) 145 146 } ··· 148 149 t.Fatalf("failed to save user directive: %v", err) 149 150 } 150 151 151 - req := httptest.NewRequest(http.MethodGet, "/recipes?location=70001001&date=2026-03-06&instructions=make+it+vegetarian", nil) 152 152 expectedParams, err := ParseQueryArgs(t.Context(), req, staticLocationLookup{location: location}) 153 153 if err != nil { 154 154 t.Fatalf("failed to build expected params: %v", err) ··· 276 276 ) 277 277 t.Cleanup(s.Wait) 278 278 279 - currentUser, err := storage.FindOrCreateFromClerk(t.Context(), "mock-clerk-user-id", auth.DefaultMock()) 279 + req := httptest.NewRequest(http.MethodGet, "/recipes?location=70001001&date=2026-03-06&instructions=make+it+vegetarian", nil) 280 + currentUser, err := storage.FromRequest(t.Context(), req, auth.DefaultMock()) 280 281 if err != nil { 281 282 t.Fatalf("failed to seed user: %v", err) 282 283 } 283 284 284 - req := httptest.NewRequest(http.MethodGet, "/recipes?location=70001001&date=2026-03-06&instructions=make+it+vegetarian", nil) 285 285 runRequest := func(t *testing.T, directive string) string { 286 286 t.Helper() 287 287
+29 -12
internal/templates/auth_establish.html
··· 6 6 <title>Signing In</title> 7 7 {{.GoogleTagScript}} 8 8 </head> 9 - <body data-return-to="{{.ReturnTo}}"> 9 + <body> 10 10 <script 11 11 crossorigin="anonymous" 12 12 data-clerk-publishable-key="{{.PublishableKey}}" ··· 18 18 url.searchParams.delete("__clerk_db_jwt"); 19 19 history.replaceState({}, "", url.toString()); 20 20 21 - const returnTo = document.body.dataset.returnTo || "/"; 21 + const returnTo = "{{.ReturnTo}}" || "/"; 22 + const userExistsURL = "{{.UserExistsURL}}"; 22 23 const finishRedirect = () => location.replace(returnTo); 23 24 24 - if ({{.Signup}} && 25 - "{{.GoogleConversionTag}}" !== "" && 26 - typeof gtag === "function") { 27 - gtag("event", "conversion", { 28 - send_to: "{{.GoogleConversionTag}}", 29 - event_callback: finishRedirect, 25 + const checkUserExists = async () => { 26 + const response = await fetch(userExistsURL, { 27 + method: "GET", 28 + credentials: "same-origin", 29 + headers: { 30 + "Accept": "application/json", 31 + }, 30 32 }); 31 - setTimeout(finishRedirect, 1500); 32 - return; 33 - } 33 + if (!response.ok) { 34 + console.warn(`auth user exists failed: ${response.status}`); 35 + return { exists: true }; 36 + } 37 + return response.json(); 38 + }; 34 39 35 - finishRedirect(); 40 + checkUserExists().then((payload) => { 41 + if (!payload.exists && 42 + "{{.GoogleConversionTag}}" !== "" && 43 + typeof gtag === "function") { 44 + gtag("event", "conversion", { 45 + send_to: "{{.GoogleConversionTag}}", 46 + event_callback: finishRedirect, 47 + }); 48 + setTimeout(finishRedirect, 1500); 49 + return; 50 + } 51 + finishRedirect(); 52 + }); 36 53 }); 37 54 </script> 38 55 </body>
+60
internal/templates/templates_test.go
··· 122 122 t.Fatalf("spinner page should pass server sign-in state to Clerk refresh logic, body: %s", rendered) 123 123 } 124 124 } 125 + 126 + func TestAuthEstablishTemplateChecksUserExistenceBeforeRedirect(t *testing.T) { 127 + cfg := &config.Config{} 128 + cfg.Clerk.PublishableKey = "pk_test_123" 129 + if err := Init(cfg, "dummyhash.css"); err != nil { 130 + t.Fatalf("Init() error = %v", err) 131 + } 132 + t.Cleanup(func() { 133 + if err := Init(&config.Config{}, "dummyhash.css"); err != nil { 134 + t.Fatalf("cleanup Init() error = %v", err) 135 + } 136 + }) 137 + 138 + data := struct { 139 + PublishableKey string 140 + GoogleTagScript template.HTML 141 + GoogleConversionTag string 142 + UserExistsURL string 143 + ReturnTo string 144 + }{ 145 + PublishableKey: "pk_test_123", 146 + GoogleConversionTag: "AW-123/abc", 147 + UserExistsURL: "/auth/user-exists", 148 + ReturnTo: "/recipe/hash", 149 + } 150 + 151 + var buf bytes.Buffer 152 + if err := AuthEstablish.Execute(&buf, data); err != nil { 153 + t.Fatalf("AuthEstablish.Execute() error = %v", err) 154 + } 155 + 156 + rendered := buf.String() 157 + if !strings.Contains(rendered, `const returnTo = "\/recipe\/hash" || "/";`) { 158 + t.Fatalf("auth establish page should inline return target, body: %s", rendered) 159 + } 160 + if !strings.Contains(rendered, `const userExistsURL = "\/auth\/user-exists";`) { 161 + t.Fatalf("auth establish page should inline user exists endpoint, body: %s", rendered) 162 + } 163 + if !strings.Contains(rendered, `fetch(userExistsURL, {`) { 164 + t.Fatalf("auth establish page should call user exists endpoint, body: %s", rendered) 165 + } 166 + if strings.Contains(rendered, `for (let attempt = 0; attempt < 5; attempt++)`) { 167 + t.Fatalf("auth establish page should not retry user exists check, body: %s", rendered) 168 + } 169 + if !strings.Contains(rendered, `if (!payload.exists &&`) { 170 + t.Fatalf("auth establish page should gate conversion on missing user, body: %s", rendered) 171 + } 172 + if !strings.Contains(rendered, `send_to: "AW-123\/abc"`) { 173 + t.Fatalf("auth establish page should emit configured conversion tag, body: %s", rendered) 174 + } 175 + if !strings.Contains(rendered, `event_callback: finishRedirect`) { 176 + t.Fatalf("auth establish page should redirect after gtag callback, body: %s", rendered) 177 + } 178 + if !strings.Contains(rendered, "console.warn(`auth user exists failed: ${response.status}`)") { 179 + t.Fatalf("auth establish page should log when user exists endpoint returns a failure, body: %s", rendered) 180 + } 181 + if strings.Contains(rendered, `.catch((error) => {`) { 182 + t.Fatalf("auth establish page should not use a catch handler for user exists failures, body: %s", rendered) 183 + } 184 + }
+42 -16
internal/users/server.go
··· 2 2 3 3 import ( 4 4 "context" 5 + "encoding/json" 5 6 "errors" 6 7 "html/template" 7 8 "log/slog" ··· 55 56 mux.HandleFunc("/user", s.handleUser) 56 57 mux.HandleFunc("POST /user/recipes", s.handleUserRecipes) 57 58 mux.HandleFunc("POST /user/favorite", s.handleFavorite) 59 + mux.HandleFunc("GET /user/exists", s.handleExists) 60 + } 61 + 62 + func (s *server) handleExists(w http.ResponseWriter, r *http.Request) { 63 + clerkUserID, err := s.clerk.GetUserIDFromRequest(r) 64 + if err != nil { 65 + if errors.Is(err, auth.ErrNoSession) { 66 + http.Error(w, "no valid session found", http.StatusUnauthorized) 67 + return 68 + } 69 + http.Error(w, "unable to load account", http.StatusInternalServerError) 70 + return 71 + } 72 + exists, err := s.exists(clerkUserID) 73 + if err != nil { 74 + slog.ErrorContext(r.Context(), "auth user exists lookup failed", "clerk_user_id", clerkUserID, "error", err) 75 + http.Error(w, "unable to check account", http.StatusInternalServerError) 76 + return 77 + } 78 + 79 + w.Header().Set("Content-Type", "application/json; charset=utf-8") 80 + if err := json.NewEncoder(w).Encode(struct { 81 + Exists bool `json:"exists"` 82 + }{ 83 + Exists: exists, 84 + }); err != nil { 85 + slog.ErrorContext(r.Context(), "auth user exists encode failed", "clerk_user_id", clerkUserID, "error", err) 86 + } 87 + } 88 + 89 + func (s *server) exists(uid string) (bool, error) { 90 + _, err := s.storage.GetByID(uid) 91 + if errors.Is(err, ErrNotFound) { 92 + return false, nil 93 + } 94 + if err != nil { 95 + return false, err 96 + } 97 + return true, nil 58 98 } 59 99 60 100 // used on user page to manaully save recipes ··· 114 154 if r.URL.Query().Get("tab") == "past" { 115 155 activeTab = "past" 116 156 } 117 - clerkUserID, err := s.clerk.GetUserIDFromRequest(r) 157 + currentUser, err := s.storage.FromRequest(ctx, r, s.clerk) 118 158 if err != nil { 119 159 if !errors.Is(err, auth.ErrNoSession) { 120 160 slog.ErrorContext(ctx, "failed to get clerk user ID", "error", err) ··· 125 165 // clerk_refresh and seee if they are then logged in. But we only want to do that once? 126 166 // TODO stick just show a sign in button on user page if no session 127 167 http.Redirect(w, r, "/", http.StatusSeeOther) 128 - return 129 - } 130 - 131 - currentUser, err := s.storage.FindOrCreateFromClerk(ctx, clerkUserID, s.clerk) 132 - if err != nil { 133 - slog.ErrorContext(ctx, "failed to get user by clerk ID", "clerk_user_id", clerkUserID, "error", err) 134 - http.Error(w, "unable to load account", http.StatusInternalServerError) 135 168 return 136 169 } 137 170 ··· 265 298 http.Error(w, "htmx request required", http.StatusBadRequest) 266 299 return 267 300 } 268 - clerkUserID, err := s.clerk.GetUserIDFromRequest(r) 301 + currentUser, err := s.storage.FromRequest(ctx, r, s.clerk) 269 302 if err != nil { 270 303 if !errors.Is(err, auth.ErrNoSession) { 271 304 slog.ErrorContext(ctx, "failed to get clerk user ID", "error", err) ··· 274 307 } 275 308 w.Header().Set("HX-Redirect", "/") 276 309 w.WriteHeader(http.StatusUnauthorized) 277 - return 278 - } 279 - 280 - currentUser, err := s.storage.FindOrCreateFromClerk(ctx, clerkUserID, s.clerk) 281 - if err != nil { 282 - slog.ErrorContext(ctx, "failed to get user by clerk ID", "clerk_user_id", clerkUserID, "error", err) 283 - http.Error(w, "unable to load account", http.StatusInternalServerError) 284 310 return 285 311 } 286 312
+2 -2
internal/users/storage.go
··· 102 102 if err != nil { 103 103 return nil, err 104 104 } 105 - return s.FindOrCreateFromClerk(ctx, clerkUserID, authClient) 105 + return s.findOrCreateFromClerk(ctx, clerkUserID, authClient) 106 106 } 107 107 108 108 // interface for clerk client 109 - func (s *Storage) FindOrCreateFromClerk(ctx context.Context, clerkUserID string, emailFetcher emailFetcher) (*utypes.User, error) { 109 + func (s *Storage) findOrCreateFromClerk(ctx context.Context, clerkUserID string, emailFetcher emailFetcher) (*utypes.User, error) { 110 110 user, err := s.GetByID(clerkUserID) 111 111 if err == nil { 112 112 return user, nil
+9 -9
internal/users/storage_test.go
··· 113 113 } 114 114 115 115 fetcher := &stubEmailFetcher{email: "should-not-call@example.com"} 116 - got, err := storage.FindOrCreateFromClerk(context.Background(), "user-3", fetcher) 116 + got, err := storage.findOrCreateFromClerk(context.Background(), "user-3", fetcher) 117 117 if err != nil { 118 - t.Fatalf("FindOrCreateFromClerk() error: %v", err) 118 + t.Fatalf("findOrCreateFromClerk() error: %v", err) 119 119 } 120 120 if fetcher.calls != 0 { 121 121 t.Fatalf("expected email fetcher to not be called, calls=%d", fetcher.calls) 122 122 } 123 123 if got.ID != user.ID { 124 - t.Fatalf("FindOrCreateFromClerk() ID = %q, want %q", got.ID, user.ID) 124 + t.Fatalf("findOrCreateFromClerk() ID = %q, want %q", got.ID, user.ID) 125 125 } 126 126 } 127 127 ··· 131 131 132 132 fetcher := &stubEmailFetcher{email: "NewUser@Example.com"} 133 133 start := time.Now() 134 - got, err := storage.FindOrCreateFromClerk(context.Background(), "user-4", fetcher) 134 + got, err := storage.findOrCreateFromClerk(context.Background(), "user-4", fetcher) 135 135 end := time.Now() 136 136 if err != nil { 137 - t.Fatalf("FindOrCreateFromClerk() error: %v", err) 137 + t.Fatalf("findOrCreateFromClerk() error: %v", err) 138 138 } 139 139 if fetcher.calls != 1 { 140 140 t.Fatalf("expected email fetcher to be called once, calls=%d", fetcher.calls) 141 141 } 142 142 if got.ID != "user-4" { 143 - t.Fatalf("FindOrCreateFromClerk() ID = %q, want %q", got.ID, "user-4") 143 + t.Fatalf("findOrCreateFromClerk() ID = %q, want %q", got.ID, "user-4") 144 144 } 145 145 if len(got.Email) != 1 || got.Email[0] != "newuser@example.com" { 146 - t.Fatalf("FindOrCreateFromClerk() Email = %v, want [newuser@example.com]", got.Email) 146 + t.Fatalf("findOrCreateFromClerk() Email = %v, want [newuser@example.com]", got.Email) 147 147 } 148 148 if got.ShoppingDay != time.Saturday.String() { 149 - t.Fatalf("FindOrCreateFromClerk() ShoppingDay = %q, want %q", got.ShoppingDay, time.Saturday.String()) 149 + t.Fatalf("findOrCreateFromClerk() ShoppingDay = %q, want %q", got.ShoppingDay, time.Saturday.String()) 150 150 } 151 151 if got.CreatedAt.Before(start) || got.CreatedAt.After(end) { 152 - t.Fatalf("FindOrCreateFromClerk() CreatedAt = %v, expected between %v and %v", got.CreatedAt, start, end) 152 + t.Fatalf("findOrCreateFromClerk() CreatedAt = %v, expected between %v and %v", got.CreatedAt, start, end) 153 153 } 154 154 155 155 stored, err := storage.GetByID("user-4")
tools/install.sh