From 1ff9da55a55e29b89fbf7335af3c7f9913dd9398 Mon Sep 17 00:00:00 2001 From: 555aaditya <107977627+555aaditya@users.noreply.github.com> Date: Mon, 19 Oct 2026 01:59:29 +0530 Subject: [PATCH] feat: keep the shared unibox overview across read-only POSTs (contact, deal and task search and summary, segment and template previews, campaign estimate) and drop it when opening a message marks it read --- internal/api/handler/unibox.go | 26 +++++++ .../handler/unibox_overview_forget_test.go | 49 ++++++++++++ internal/api/resource_scope_routes_test.go | 14 ++++ internal/app/unibox/email.go | 3 + internal/app/unibox/overview_forget_test.go | 78 +++++++++++++++++++ 5 files changed, 170 insertions(+) create mode 100644 internal/api/handler/unibox_overview_forget_test.go create mode 100644 internal/app/unibox/overview_forget_test.go diff --git a/internal/api/handler/unibox.go b/internal/api/handler/unibox.go index c0e1934e1..fea06a441 100644 --- a/internal/api/handler/unibox.go +++ b/internal/api/handler/unibox.go @@ -653,6 +653,29 @@ func (h *Handler) UniboxReply(c *gin.Context) { c.JSON(http.StatusOK, resp) } +// readOnlyPosts take a POST only to carry a query body and are gated on a read permission, so they write nothing the overview counts. +var readOnlyPosts = map[string]bool{ + "/v1/campaign-template-preview": true, + "/v1/campaigns-estimate": true, + "/v1/contacts/search": true, + "/v1/segments/preview": true, + "/v1/segments/:id/members/lookup": true, + "/v1/templates/:id/render": true, + "/v1/crm/deals/search": true, + "/v1/crm/deals/summary": true, + "/v1/crm/tasks/search": true, + "/v1/crm/tasks/summary": true, +} + +// ReadOnlyPostRoutes lists readOnlyPosts as "POST path", for the test that keeps each one registered. +func ReadOnlyPostRoutes() []string { + out := make([]string, 0, len(readOnlyPosts)) + for path := range readOnlyPosts { + out = append(out, http.MethodPost+" "+path) + } + return out +} + // ForgetUniboxOverviewOnWrite drops the organization's shared unibox overview after any successful write. func (h *Handler) ForgetUniboxOverviewOnWrite(c *gin.Context) { c.Next() @@ -660,6 +683,9 @@ func (h *Handler) ForgetUniboxOverviewOnWrite(c *gin.Context) { case http.MethodGet, http.MethodHead, http.MethodOptions: return } + if readOnlyPosts[c.FullPath()] { + return + } if h.UniboxService == nil || c.Writer.Status() >= http.StatusBadRequest { return } diff --git a/internal/api/handler/unibox_overview_forget_test.go b/internal/api/handler/unibox_overview_forget_test.go new file mode 100644 index 000000000..4add6cfd9 --- /dev/null +++ b/internal/api/handler/unibox_overview_forget_test.go @@ -0,0 +1,49 @@ +package handler + +import ( + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + "github.com/google/uuid" + "github.com/warmbly/warmbly/internal/api/middleware" + "github.com/warmbly/warmbly/internal/app/unibox" +) + +type forgetCountingUnibox struct { + unibox.UniboxService + forgets int +} + +func (f *forgetCountingUnibox) ForgetOverview(uuid.UUID) { f.forgets++ } + +func TestForgetUniboxOverviewOnWrite(t *testing.T) { + gin.SetMode(gin.TestMode) + tests := []struct { + name, method, route, path string + status, want int + }{ + {"a unibox write", http.MethodPatch, "/v1/unibox/seen", "/v1/unibox/seen", http.StatusOK, 1}, + {"a mailbox write", http.MethodDelete, "/v1/emails/:id", "/v1/emails/x", http.StatusOK, 1}, + {"a read", http.MethodGet, "/v1/unibox/overview", "/v1/unibox/overview", http.StatusOK, 0}, + {"a refused write", http.MethodPatch, "/v1/unibox/seen", "/v1/unibox/seen", http.StatusBadRequest, 0}, + {"a contact search", http.MethodPost, "/v1/contacts/search", "/v1/contacts/search", http.StatusOK, 0}, + {"a CRM summary", http.MethodPost, "/v1/crm/deals/summary", "/v1/crm/deals/summary", http.StatusOK, 0}, + {"a template render", http.MethodPost, "/v1/templates/:id/render", "/v1/templates/x/render", http.StatusOK, 0}, + {"a contact create", http.MethodPost, "/v1/contacts", "/v1/contacts", http.StatusCreated, 1}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + svc := &forgetCountingUnibox{} + h := &Handler{UniboxService: svc} + r := gin.New() + r.Use(func(c *gin.Context) { c.Set(middleware.OrganizationIDKey, uuid.New()) }, h.ForgetUniboxOverviewOnWrite) + r.Handle(tt.method, tt.route, func(c *gin.Context) { c.Status(tt.status) }) + r.ServeHTTP(httptest.NewRecorder(), httptest.NewRequest(tt.method, tt.path, nil)) + if svc.forgets != tt.want { + t.Errorf("forgets = %d, want %d", svc.forgets, tt.want) + } + }) + } +} diff --git a/internal/api/resource_scope_routes_test.go b/internal/api/resource_scope_routes_test.go index 30411bd34..93534b06a 100644 --- a/internal/api/resource_scope_routes_test.go +++ b/internal/api/resource_scope_routes_test.go @@ -25,3 +25,17 @@ func TestScopeAwareRoutesExist(t *testing.T) { } } } + +// A read-only POST that is renamed falls out of the list and forgets the overview again; this keeps the list honest. +func TestReadOnlyPostRoutesExist(t *testing.T) { + r := Run(&handler.Handler{}, &middleware.Handler{}, &middleware.OidcHandler{}, "", gin.TestMode, nil) + registered := map[string]bool{} + for _, rt := range r.Routes() { + registered[rt.Method+" "+rt.Path] = true + } + for _, route := range handler.ReadOnlyPostRoutes() { + if !registered[route] { + t.Errorf("read-only POST %q is not registered", route) + } + } +} diff --git a/internal/app/unibox/email.go b/internal/app/unibox/email.go index 324962ba9..31828f509 100644 --- a/internal/app/unibox/email.go +++ b/internal/app/unibox/email.go @@ -55,6 +55,9 @@ func (s *uniboxService) getByID(ctx context.Context, orgID, id uuid.UUID, markSe if changed, err := s.uniboxRepository.MarkSeenBulk(ctx, orgID, []uuid.UUID{id}, true); err == nil { msg.Seen = true s.relaySeen(ctx, orgID, changed) + if len(changed) > 0 { + s.ForgetOverview(orgID) + } } } resp.ID = msg.ID diff --git a/internal/app/unibox/overview_forget_test.go b/internal/app/unibox/overview_forget_test.go new file mode 100644 index 000000000..92a473222 --- /dev/null +++ b/internal/app/unibox/overview_forget_test.go @@ -0,0 +1,78 @@ +package unibox + +import ( + "context" + "testing" + + "github.com/google/uuid" + "github.com/warmbly/warmbly/internal/errx" + "github.com/warmbly/warmbly/internal/models" +) + +// fakeOpenRepo is the forward fake plus the one write opening a message makes. +type fakeOpenRepo struct { + *fakeForwardRepo + marked int +} + +func (f *fakeOpenRepo) MarkSeenBulk(_ context.Context, _ uuid.UUID, ids []uuid.UUID, _ bool) ([]uuid.UUID, error) { + f.marked++ + return ids, nil +} + +// cachedOverviewService returns a service whose overview for orgID is already computed once. +func cachedOverviewService(t *testing.T, seen bool) (*uniboxService, uuid.UUID, uuid.UUID, *int) { + t.Helper() + s, msg := forwardService(t, nil) + msg.Seen = seen + s.uniboxRepository = &fakeOpenRepo{fakeForwardRepo: s.uniboxRepository.(*fakeForwardRepo)} + computes := 0 + s.overview = newOverviewCache(func(context.Context, overviewScope) (*models.UniboxOverview, error) { + computes++ + return &models.UniboxOverview{}, nil + }) + orgID := uuid.New() + if _, err := s.overview.get(context.Background(), overviewScope{OrgID: orgID}); err != nil { + t.Fatal(err) + } + return s, orgID, msg.ID, &computes +} + +func TestOpeningAnUnreadMessageForgetsTheOverview(t *testing.T) { + s, orgID, id, computes := cachedOverviewService(t, false) + if _, xerr := s.GetByID(context.Background(), orgID, id); xerr != nil { + t.Fatal(xerr) + } + if _, err := s.overview.get(context.Background(), overviewScope{OrgID: orgID}); err != nil { + t.Fatal(err) + } + if *computes != 2 { + t.Errorf("overview computed %d times, want a fresh one after the open marked a message read", *computes) + } +} + +func TestOpeningWithoutChangingReadStateKeepsTheOverview(t *testing.T) { + for name, open := range map[string]func(*uniboxService, uuid.UUID, uuid.UUID) *errx.Error{ + "an already read message": func(s *uniboxService, org, id uuid.UUID) *errx.Error { + _, xerr := s.GetByID(context.Background(), org, id) + return xerr + }, + "a peek": func(s *uniboxService, org, id uuid.UUID) *errx.Error { + _, xerr := s.Peek(context.Background(), org, id) + return xerr + }, + } { + t.Run(name, func(t *testing.T) { + s, orgID, id, computes := cachedOverviewService(t, name == "an already read message") + if xerr := open(s, orgID, id); xerr != nil { + t.Fatal(xerr) + } + if _, err := s.overview.get(context.Background(), overviewScope{OrgID: orgID}); err != nil { + t.Fatal(err) + } + if *computes != 1 { + t.Errorf("overview computed %d times, want the cached one", *computes) + } + }) + } +}