Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions internal/service/broadcast_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,9 @@ func (s *BroadcastService) ListBroadcasts(ctx context.Context, params domain.Lis
return nil, err
}

// If WithTemplates is true, fetch template details for each variation
if params.WithTemplates {
// If WithTemplates is true, fetch template details for each variation. Without Templates read
// every lookup would be refused and logged: the broadcasts come back without their templates.
if params.WithTemplates && userWorkspace.HasPermission(domain.PermissionResourceTemplates, domain.PermissionTypeRead) {
for _, broadcast := range response.Broadcasts {
for i, variation := range broadcast.TestSettings.Variations {
if variation.TemplateID != "" {
Expand Down
67 changes: 65 additions & 2 deletions internal/service/broadcast_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,23 @@ func authOK(auth *domainmocks.MockAuthService, ctx context.Context, workspaceID
auth.EXPECT().AuthenticateUserForWorkspace(ctx, workspaceID).Return(ctx, &domain.User{ID: "user1"}, userWorkspace, nil)
}

// authAs authenticates a member of the workspace with the given role and permissions.
func authAs(auth *domainmocks.MockAuthService, ctx context.Context, workspaceID, role string, perms domain.UserPermissions) {
userWorkspace := &domain.UserWorkspace{
UserID: "user1",
WorkspaceID: workspaceID,
Role: role,
Permissions: perms,
}
auth.EXPECT().AuthenticateUserForWorkspace(ctx, workspaceID).Return(ctx, &domain.User{ID: "user1"}, userWorkspace, nil)
}

// broadcastsAndTemplatesRead is what listing broadcasts with their templates takes.
var broadcastsAndTemplatesRead = domain.UserPermissions{
domain.PermissionResourceBroadcasts: {Read: true, Write: true},
domain.PermissionResourceTemplates: {Read: true},
}

func TestBroadcastService_CreateBroadcast_Success(t *testing.T) {
d := setupBroadcastSvc(t)
defer d.ctrl.Finish()
Expand Down Expand Up @@ -805,7 +822,7 @@ func TestBroadcastService_ListBroadcasts_WithTemplates(t *testing.T) {

ctx := context.Background()
params := domain.ListBroadcastsParams{WorkspaceID: "w1", WithTemplates: true}
authOK(d.authService, ctx, params.WorkspaceID)
authAs(d.authService, ctx, params.WorkspaceID, "member", broadcastsAndTemplatesRead)

b := testBroadcast(params.WorkspaceID, "b1")
// Ensure there is a variation to load template for
Expand Down Expand Up @@ -1129,7 +1146,7 @@ func TestBroadcastService_ListBroadcasts_WithTemplates_TemplateError(t *testing.

ctx := context.Background()
params := domain.ListBroadcastsParams{WorkspaceID: "w1", WithTemplates: true}
authOK(d.authService, ctx, params.WorkspaceID)
authAs(d.authService, ctx, params.WorkspaceID, "member", broadcastsAndTemplatesRead)

b := testBroadcast(params.WorkspaceID, "b1")
b.TestSettings.Variations = []domain.BroadcastVariation{{VariationName: "A", TemplateID: "tplA"}}
Expand Down Expand Up @@ -4035,3 +4052,49 @@ func TestBroadcastService_SendToIndividual_ListUnsubscribeHeader(t *testing.T) {
})
}
}

// Without Templates read every template lookup is refused: the broadcasts come back without
// their templates, without the refused calls and their error logs. The strict mock fails the
// test on any GetTemplateByID call.
func TestBroadcastService_ListBroadcasts_WithTemplates_WithoutTemplatesRead(t *testing.T) {
d := setupBroadcastSvc(t)
defer d.ctrl.Finish()

ctx := context.Background()
params := domain.ListBroadcastsParams{WorkspaceID: "w1", WithTemplates: true}
authAs(d.authService, ctx, params.WorkspaceID, "member", domain.UserPermissions{
domain.PermissionResourceBroadcasts: {Read: true, Write: true},
})

b := testBroadcast(params.WorkspaceID, "b1")
b.TestSettings.Variations = []domain.BroadcastVariation{{VariationName: "A", TemplateID: "tplA"}}
d.repo.EXPECT().ListBroadcasts(ctx, gomock.Any()).Return(
&domain.BroadcastListResponse{Broadcasts: []*domain.Broadcast{b}, TotalCount: 1}, nil)

out, err := d.svc.ListBroadcasts(ctx, params)
require.NoError(t, err)
require.Len(t, out.Broadcasts, 1)
v := out.Broadcasts[0].TestSettings.Variations[0]
assert.Equal(t, "tplA", v.TemplateID)
assert.Nil(t, v.Template)
}

// An owner holds every permission without listing them.
func TestBroadcastService_ListBroadcasts_WithTemplates_Owner(t *testing.T) {
d := setupBroadcastSvc(t)
defer d.ctrl.Finish()

ctx := context.Background()
params := domain.ListBroadcastsParams{WorkspaceID: "w1", WithTemplates: true}
authAs(d.authService, ctx, params.WorkspaceID, "owner", nil)

b := testBroadcast(params.WorkspaceID, "b1")
b.TestSettings.Variations = []domain.BroadcastVariation{{VariationName: "A", TemplateID: "tplA"}}
d.repo.EXPECT().ListBroadcasts(ctx, gomock.Any()).Return(
&domain.BroadcastListResponse{Broadcasts: []*domain.Broadcast{b}, TotalCount: 1}, nil)
d.templateSvc.EXPECT().GetTemplateByID(ctx, params.WorkspaceID, "tplA", int64(0)).Return(&domain.Template{ID: "tplA"}, nil)

out, err := d.svc.ListBroadcasts(ctx, params)
require.NoError(t, err)
require.NotNil(t, out.Broadcasts[0].TestSettings.Variations[0].Template)
}
56 changes: 56 additions & 0 deletions plans/fix-broadcasts-list-without-templates-read-plan.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# Broadcasts list without Templates read plan

**Status**: plan reviewed, implemented
**Date**: 2026-11-12
**Target**: 44.0 (`## [44.0] - unreleased` on `dev`). No migration, no environment variable, no console change, no
docs change.
**Base**: `origin/dev` at `6688f19`.

## Goal

`broadcasts.list` with `with_templates` no longer tries, and logs a refusal for, each variation's template when the
caller has no Templates read. Follow-up of #536, which listed this under "Known, not handled".

## Current behaviour

- `BroadcastsPage` lists broadcasts with `with_templates: true` (`console/src/pages/BroadcastsPage.tsx:1378-1393`),
whatever the member's permissions.
- `BroadcastService.ListBroadcasts` (`internal/service/broadcast_service.go:275-327`) authenticates the caller
(`userWorkspace`, `:278`), then, with `WithTemplates`, calls `templateSvc.GetTemplateByID` per variation. Without
Templates read each call answers a permission error, which the loop logs at error level ("Failed to fetch template
for broadcast variation") and skips: the response carries the broadcasts without templates.
- So the answer is already right; the cost is one refused call and one error log per variation, per page load, for
the console and for any API key without Templates read.

## Design

The fix belongs in the service, which already holds the caller's permissions, rather than in the console (reviewed
alternative: making the page wait for `workspaces.members` before listing, which would add a serial request for
everyone, a new loading state and a query key that eight prefix invalidations depend on).

1. `ListBroadcasts`: fetch the variations' templates only when
`params.WithTemplates && userWorkspace.HasPermission(domain.PermissionResourceTemplates, domain.PermissionTypeRead)`.
Without the permission the broadcasts come back without templates, as they already did, without the refused calls
or the logs. A comment says why.
2. `CHANGELOG.md`: no new line. #536's `Fix` line (44.0, unreleased) already says "these screens no longer request
templates such a member cannot read", which this makes true for the broadcasts list too.

## Tests

| Implementation | Test file | Cases |
|---|---|---|
| `internal/service/broadcast_service.go` | `internal/service/broadcast_service_test.go` | a helper authenticates a member with given permissions; without Templates read, `WithTemplates`: `GetTemplateByID` is not called (strict gomock) and the broadcast comes back with its variation, template nil; the two existing `WithTemplates` cases authenticate a member with Templates read (with it, `GetTemplateByID` is called, as now); an owner without explicit permissions gets the templates |

## Known, not handled

- `broadcasts.get` with `with_templates` (`internal/http/broadcast_handler.go:187-205`) does the same in the
handler, which has no `userWorkspace`; the console never asks it for templates.
- `useWorkspacePermissions` keeps the previous workspace's permissions until the next answer when the workspace
changes while a page stays mounted.

## Test commands

```
go test ./internal/service/ -run 'TestBroadcastService_ListBroadcasts' -v
make test-service
```
Loading