Add support for multiple targets for the announcement banner#578
Add support for multiple targets for the announcement banner#578
Conversation
📝 WalkthroughWalkthroughThe PR refactors announcement targeting from a simple boolean flag ( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/contexts/AnnouncementProvider.tsx`:
- Line 11: The provider currently hard-codes the runtime target (e.g.
"selfhosted") and no longer imports isNetBirdHosted(), causing cloud users to
see wrong banners and announcements with no targets to be dropped; restore
runtime detection by re-importing isNetBirdHosted from "@utils/netbird", compute
a runtimeTarget (e.g. runtimeTarget = isNetBirdHosted() ? "selfhosted" :
"cloud") inside AnnouncementProvider (the component that filters announcements),
and update the filter logic in whatever function/method filters announcements
(refer to the announcement filtering code around the existing targets check) so
that an announcement is kept if announcement.targets is undefined/empty OR
includes runtimeTarget (instead of comparing to a hard-coded string). Ensure all
occurrences that previously used the hard-coded "selfhosted" are replaced to use
runtimeTarget so cloud vs self-hosted splits behave the same as
DashboardLayout's isNetBirdHosted() branching.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: eb748f8b-6423-452f-8a84-d48f2ed215a9
📒 Files selected for processing (2)
announcements.jsonsrc/contexts/AnnouncementProvider.tsx
| } from "react"; | ||
| import { usePermissions } from "@/contexts/PermissionsProvider"; | ||
| import { isLocalDev, isNetBirdHosted } from "@utils/netbird"; | ||
| import { isLocalDev } from "@utils/netbird"; |
There was a problem hiding this comment.
Restore runtime target detection before filtering.
Line 80 now matches against a hard-coded "selfhosted" target for every deployment, because the isNetBirdHosted() signal was removed at Line 11. That means cloud users will also see self-hosted banners, cloud-targeted banners can never render, and announcements without targets are silently dropped even though the field is optional on Line 34. src/layouts/DashboardLayout.tsx:27-46 still uses isNetBirdHosted() for the same cloud/self-hosted split, so the provider should derive the current target instead of hard-coding one.
Proposed fix
-import { isLocalDev } from "@utils/netbird";
+import { isLocalDev, isNetBirdHosted } from "@utils/netbird";
...
-const TARGET = "selfhosted";
+type AnnouncementTarget = "cloud" | "selfhosted";
...
export interface Announcement extends AnnouncementVariant {
tag: string;
text: string;
link?: string;
linkText?: string;
isExternal?: boolean;
closeable: boolean;
- targets?: string[];
+ targets?: AnnouncementTarget[];
}
...
- const filtered = raw.filter((a) => a.targets?.includes(TARGET));
+ const currentTarget: AnnouncementTarget = isNetBirdHosted()
+ ? "cloud"
+ : "selfhosted";
+ const filtered = raw.filter(
+ (a) => !a.targets?.length || a.targets.includes(currentTarget),
+ );Also applies to: 18-18, 34-34, 80-80
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/contexts/AnnouncementProvider.tsx` at line 11, The provider currently
hard-codes the runtime target (e.g. "selfhosted") and no longer imports
isNetBirdHosted(), causing cloud users to see wrong banners and announcements
with no targets to be dropped; restore runtime detection by re-importing
isNetBirdHosted from "@utils/netbird", compute a runtimeTarget (e.g.
runtimeTarget = isNetBirdHosted() ? "selfhosted" : "cloud") inside
AnnouncementProvider (the component that filters announcements), and update the
filter logic in whatever function/method filters announcements (refer to the
announcement filtering code around the existing targets check) so that an
announcement is kept if announcement.targets is undefined/empty OR includes
runtimeTarget (instead of comparing to a hard-coded string). Ensure all
occurrences that previously used the hard-coded "selfhosted" are replaced to use
runtimeTarget so cloud vs self-hosted splits behave the same as
DashboardLayout's isNetBirdHosted() branching.
1 participant
Summary by CodeRabbit