Skip to content

Commit dcf9525

Browse files
alexluongclaude
andauthored
fix: flat portal topics without separator (#651)
* refactor: remove redundant tenant_id from portal redirect URL The tenant_id is already embedded in the JWT token's sub claim, so it's unnecessary in the URL query parameter. Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(portal): handle mixed flat and nested topics in TopicPicker Topics are now parsed individually for separators instead of requiring all topics to share the same separator. This allows flat topics (job, document) to render without nesting while hierarchical topics (user.created, user.updated) are grouped. Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix: portal vite reload * test: update tests for tenant topics and portal URL changes - Update e2e test to expect literal ["*"] for tenant topics with wildcard destination (matches 78f162e behavior change) - Update portal handler tests to check for token= instead of tenant_id= in redirect URL (matches 20463b2 refactor) Co-Authored-By: Claude Opus 4.5 <[email protected]> --------- Co-authored-by: Claude Opus 4.5 <[email protected]>
1 parent 78f162e commit dcf9525

File tree

6 files changed

+58
-33
lines changed

6 files changed

+58
-33
lines changed

cmd/e2e/api_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ func (suite *basicSuite) TestTenantsAPI() {
168168
Body: map[string]interface{}{
169169
"id": tenantID,
170170
"destinations_count": 1,
171-
"topics": suite.config.Topics,
171+
"topics": []string{"*"},
172172
},
173173
},
174174
},

internal/apirouter/tenant_handlers.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ func (h *TenantHandlers) RetrievePortal(c *gin.Context) {
209209
theme = ""
210210
}
211211

212-
portalURL := scheme + "://" + c.Request.Host + "?token=" + jwtToken + "&tenant_id=" + tenant.ID
212+
portalURL := scheme + "://" + c.Request.Host + "?token=" + jwtToken
213213
if theme != "" {
214214
portalURL += "&theme=" + theme
215215
}

internal/apirouter/tenant_handlers_test.go

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -240,7 +240,7 @@ func TestTenantRetrievePortalHandler(t *testing.T) {
240240
router, _, redisClient := setupTestRouter(t, apiKey, jwtSecret)
241241
entityStore := setupTestEntityStore(t, redisClient, nil)
242242

243-
t.Run("should return redirect_url with tenant_id and tenant_id in body", func(t *testing.T) {
243+
t.Run("should return redirect_url with token and tenant_id in body", func(t *testing.T) {
244244
t.Parallel()
245245

246246
// Setup
@@ -261,7 +261,7 @@ func TestTenantRetrievePortalHandler(t *testing.T) {
261261
// Test
262262
assert.Equal(t, http.StatusOK, w.Code)
263263
assert.NotEmpty(t, response["redirect_url"])
264-
assert.Contains(t, response["redirect_url"], "tenant_id="+existingResource.ID)
264+
assert.Contains(t, response["redirect_url"], "token=")
265265
assert.Equal(t, existingResource.ID, response["tenant_id"])
266266

267267
// Cleanup
@@ -288,7 +288,7 @@ func TestTenantRetrievePortalHandler(t *testing.T) {
288288

289289
// Test
290290
assert.Equal(t, http.StatusOK, w.Code)
291-
assert.Contains(t, response["redirect_url"], "tenant_id="+existingResource.ID)
291+
assert.Contains(t, response["redirect_url"], "token=")
292292
assert.Contains(t, response["redirect_url"], "theme=dark")
293293
assert.Equal(t, existingResource.ID, response["tenant_id"])
294294

internal/portal/src/common/TopicPicker/TopicPicker.scss

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,4 +83,10 @@
8383
font-size: var(--font-size-m);
8484
line-height: var(--line-height-m);
8585
}
86+
87+
&__flat-topic {
88+
padding-left: var(--spacing-6);
89+
margin-top: var(--spacing-3);
90+
margin-bottom: var(--spacing-3);
91+
}
8692
}

internal/portal/src/common/TopicPicker/TopicPicker.tsx

Lines changed: 39 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -17,39 +17,32 @@ interface TopicPickerProps {
1717
onTopicsChange: (topics: string[]) => void;
1818
}
1919

20-
const detectSeparator = (topics: string[]): string => {
21-
// Common separators to check
22-
const possibleSeparators = ["/", ".", "-"];
20+
const possibleSeparators = ["/", ".", "-"];
2321

24-
// Find the first separator that appears in all topics
25-
// and is the first occurring separator in each topic
26-
return (
27-
possibleSeparators.find((sep) =>
28-
topics.every((topic) => {
29-
const sepIndex = topic.indexOf(sep);
30-
if (sepIndex === -1) return false;
31-
32-
// Check if any other separator appears before this one
33-
const otherSepsIndex = possibleSeparators
34-
.filter((s) => s !== sep)
35-
.map((s) => topic.indexOf(s))
36-
.filter((idx) => idx !== -1);
37-
38-
return otherSepsIndex.every((idx) => idx === -1 || idx > sepIndex);
39-
}),
40-
) || "-"
41-
); // Fallback to '-' if no consistent separator is found
22+
const findFirstSeparator = (topic: string): string | null => {
23+
let firstIndex = -1;
24+
let firstSep: string | null = null;
25+
26+
for (const sep of possibleSeparators) {
27+
const idx = topic.indexOf(sep);
28+
if (idx !== -1 && (firstIndex === -1 || idx < firstIndex)) {
29+
firstIndex = idx;
30+
firstSep = sep;
31+
}
32+
}
33+
34+
return firstSep;
4235
};
4336

4437
const topics: Topic[] = (() => {
4538
const topicsList = CONFIGS.TOPICS.split(",");
46-
const separator = detectSeparator(topicsList);
4739

4840
return topicsList.map((topic) => {
49-
const parts = topic.split(separator);
41+
const separator = findFirstSeparator(topic);
42+
5043
return {
5144
id: topic,
52-
category: parts[0],
45+
category: separator ? topic.split(separator)[0] : topic,
5346
};
5447
});
5548
})();
@@ -162,6 +155,28 @@ const TopicPicker = ({
162155
const areAllSelected = selectedCount === categoryTopics.length;
163156
const isIndeterminate = selectedCount > 0 && !areAllSelected;
164157

158+
// Check if this is a flat topic (no actual nesting)
159+
const isFlatTopic =
160+
categoryTopics.length === 1 && categoryTopics[0].id === category;
161+
162+
if (isFlatTopic) {
163+
const topic = categoryTopics[0];
164+
return (
165+
<div key={category} className="topic-picker__flat-topic">
166+
<Checkbox
167+
checked={
168+
isEverythingSelected ||
169+
selectedTopics.indexOf(topic.id) !== -1
170+
}
171+
onChange={() => toggleTopic(topic.id)}
172+
label={topic.id}
173+
monospace
174+
disabled={isEverythingSelected}
175+
/>
176+
</div>
177+
);
178+
}
179+
165180
return (
166181
<div key={category} className="topic-picker__category">
167182
<div className="topic-picker__category-header">

internal/portal/vite.config.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import react from "@vitejs/plugin-react-swc";
33
import { sentryVitePlugin } from "@sentry/vite-plugin";
44

55
export default defineConfig(({ mode }) => {
6-
let plugins: UserConfig["plugins"] = [react()];
6+
const plugins: UserConfig["plugins"] = [react()];
77

88
if (process.env.SENTRY_AUTH_TOKEN && mode === "production") {
99
plugins.push(
@@ -28,9 +28,13 @@ export default defineConfig(({ mode }) => {
2828
plugins,
2929
server: {
3030
port: 3334,
31-
// hmr: {
32-
// port: 3334,
33-
// },
31+
host: true,
32+
watch: {
33+
usePolling: true, // Required for Docker volume mounts
34+
},
35+
hmr: {
36+
clientPort: 3334, // Ensure HMR WebSocket connects to the right port
37+
},
3438
},
3539
build: {
3640
sourcemap: true,

0 commit comments

Comments
 (0)