From 809d32c4d845cb44037ad59209c54738ff70021e Mon Sep 17 00:00:00 2001 From: Dominik Schmidt Date: Wed, 29 Jul 2026 08:29:58 +0200 Subject: [PATCH] fix: keep a static announcement as fallback when the store is empty Make the store authoritative only once it manages an announcement: an enabled record is exposed, a disabled record clears any static config, and an empty store falls back to a statically configured web.config.options.announcement. This matches the review request and stops a static announcement from being silently dropped. --- services/web/pkg/service/v0/service.go | 34 ++++++++++++++------- services/web/pkg/service/v0/service_test.go | 32 ++++++++++++------- 2 files changed, 44 insertions(+), 22 deletions(-) diff --git a/services/web/pkg/service/v0/service.go b/services/web/pkg/service/v0/service.go index 190b601c73..4587ce0822 100644 --- a/services/web/pkg/service/v0/service.go +++ b/services/web/pkg/service/v0/service.go @@ -156,31 +156,43 @@ func (p Web) getPayload() (payload []byte, err error) { // ensure that the server url has a trailing slash webConfig.Server = strings.TrimRight(webConfig.Server, "/") + "/" - // the runtime store is authoritative for the announcement banner: expose it when live, - // clear it otherwise - webConfig.Options.Announcement = p.currentAnnouncement() + // the runtime store is authoritative once it manages an announcement (enabled or explicitly + // disabled); only when it holds nothing do we keep a statically configured one + // (web.config.options.announcement) + if a, managed := p.managedAnnouncement(); managed { + webConfig.Options.Announcement = a + } return json.Marshal(webConfig) } -// currentAnnouncement returns the stored announcement for config.json, or nil if unset or unavailable. -func (p Web) currentAnnouncement() *config.Announcement { +// managedAnnouncement returns the announcement the runtime store manages for config.json. The +// bool reports whether the store manages one at all: when true, the returned value (nil for an +// explicitly disabled announcement) overrides any static config; when false, the store holds +// nothing and a statically configured announcement is kept. +func (p Web) managedAnnouncement() (*config.Announcement, bool) { if p.announcementStore == nil { - return nil + return nil, false } a, err := p.announcementStore.Get() if err != nil { p.logger.Error().Err(err).Msg("could not read announcement from store") - return nil + return nil, false } - // only live (enabled) announcements with a banner line are exposed in the public config.json - if !a.Enabled || a.BannerText == "" { - return nil + // an announcement without a banner line is nothing to manage (empty/removed store) + if a.BannerText == "" { + return nil, false } - return &config.Announcement{BannerText: a.BannerText, InfoText: a.InfoText} + // managed but disabled: hide it, overriding any static config + if !a.Enabled { + return nil, true + } + + // only live (enabled) announcements are exposed in the public config.json + return &config.Announcement{BannerText: a.BannerText, InfoText: a.InfoText}, true } // Config implements the Service interface. diff --git a/services/web/pkg/service/v0/service_test.go b/services/web/pkg/service/v0/service_test.go index 612bf59425..9a983f0bca 100644 --- a/services/web/pkg/service/v0/service_test.go +++ b/services/web/pkg/service/v0/service_test.go @@ -11,7 +11,7 @@ import ( "github.com/opencloud-eu/opencloud/services/web/pkg/config" ) -func TestCurrentAnnouncement(t *testing.T) { +func TestManagedAnnouncement(t *testing.T) { newWeb := func(a *announcement.Store) Web { return Web{logger: log.NopLogger(), announcementStore: a} } @@ -19,29 +19,39 @@ func TestCurrentAnnouncement(t *testing.T) { return announcement.NewStore(store.Create(store.Store("memory"))) } - t.Run("nil when there is no store", func(t *testing.T) { - require.Nil(t, newWeb(nil).currentAnnouncement()) + t.Run("not managed when there is no store, so a static config is kept", func(t *testing.T) { + a, managed := newWeb(nil).managedAnnouncement() + require.Nil(t, a) + require.False(t, managed) }) - t.Run("nil when the store is empty", func(t *testing.T) { - require.Nil(t, newWeb(newStore()).currentAnnouncement()) + t.Run("not managed when the store is empty, so a static config is kept", func(t *testing.T) { + a, managed := newWeb(newStore()).managedAnnouncement() + require.Nil(t, a) + require.False(t, managed) }) - t.Run("nil when disabled", func(t *testing.T) { + t.Run("managed but nil when disabled, so a static config is cleared", func(t *testing.T) { s := newStore() require.NoError(t, s.Set(announcement.Announcement{Enabled: false, BannerText: "hi", InfoText: "info"})) - require.Nil(t, newWeb(s).currentAnnouncement()) + a, managed := newWeb(s).managedAnnouncement() + require.Nil(t, a) + require.True(t, managed) }) - t.Run("nil when enabled but the banner text is empty", func(t *testing.T) { + t.Run("not managed when enabled but the banner text is empty", func(t *testing.T) { s := newStore() require.NoError(t, s.Set(announcement.Announcement{Enabled: true, InfoText: "info"})) - require.Nil(t, newWeb(s).currentAnnouncement()) + a, managed := newWeb(s).managedAnnouncement() + require.Nil(t, a) + require.False(t, managed) }) - t.Run("returns banner and info text when enabled with a banner text", func(t *testing.T) { + t.Run("managed with banner and info text when enabled with a banner text", func(t *testing.T) { s := newStore() require.NoError(t, s.Set(announcement.Announcement{Enabled: true, BannerText: "hi", InfoText: "info"})) - require.Equal(t, &config.Announcement{BannerText: "hi", InfoText: "info"}, newWeb(s).currentAnnouncement()) + a, managed := newWeb(s).managedAnnouncement() + require.Equal(t, &config.Announcement{BannerText: "hi", InfoText: "info"}, a) + require.True(t, managed) }) }