Remove topics from backend-api

[jvorcak]

No description provided.

[weeco]

@claude Review this PR

[claude]

Claude finished @weeco's task in 2m 30s —— View job

---

PR Review: Remove topics from backend-api

• Gather context and read CLAUDE.md
• Read key changed files and diff
• Analyze the refactoring changes
• Post comprehensive review

---

Good overall direction — migrating topic-related API calls from the MobX-based backend-api.ts to proper React Query hooks in react-query/api/topic.tsx is the right approach. The new hooks are well-structured and their JSDoc comments clearly document what they replace. A few issues worth addressing:

---

Bugs

1. hasEditPermissions hardcoded to true — authorization bypass

frontend/src/components/pages/topics/topic-configuration.tsx:237

const hasEditPermissions = true;

This unconditionally allows the edit button for every user, removing the ACL-based permission check. TopicActions includes 'editConfig' and Topic.allowedActions is still present (used in topic-details.tsx:85-89 for tab-level gating). The parent TopicConfiguration component (tab-config.tsx:27) already receives the full Topic object, so allowedActions can be passed down.

The old code presumably checked topic.allowedActions?.includes('editConfig') before rendering the edit button. The fact that isTopicConfigEdittable() (line 396) still accepts hasEditPermissions: boolean shows the plumbing is in place — it just isn't being wired up anymore.

Fix this →

---

2. Missing response.ok checks in query functions

frontend/src/react-query/api/topic.tsx

Several query functions don't check response.ok before calling response.json(). Error responses (4xx/5xx) with a JSON body will be silently treated as valid data instead of thrown as errors. Compare with useUpdateTopicConfigMutation and useDeleteTopicMutation which correctly throw on non-OK responses.

Affected hooks:

• useTopicsQuery (line 107–112)
• useLegacyListTopicsQuery (line 75–80)
• useTopicDocumentationQuery (line 264–270)
• useTopicConsumersQuery (line 294–299)
• useTopicAclsQuery (line 344–350)
• useTopicPartitionsQuery (line 397–405)
• useAllTopicPartitionsQuery (line 437–448)

Example fix:

const response = await config.fetch(`${config.restBasePath}/topics`, { method: 'GET', headers });
if (!response.ok) throw new Error(`Failed to fetch topics: ${response.statusText}`);
return response.json();

---

Code quality

3. Duplicated auth header boilerplate

frontend/src/react-query/api/topic.tsx

The JWT auth header block is copy-pasted ~10 times:

const headers: HeadersInit = {};
if (config.jwt) {
  headers.Authorization = `Bearer ${config.jwt}`;
}

Since this is used everywhere in this file, extracting a createAuthHeaders() helper would reduce repetition and ensure consistency:

function createAuthHeaders(extraHeaders?: HeadersInit): HeadersInit {
  const headers: HeadersInit = { ...extraHeaders };
  if (config.jwt) headers.Authorization = `Bearer ${config.jwt}`;
  return headers;
}

---

4. useLegacyListTopicsQuery reuses the Connect query key for REST data

frontend/src/react-query/api/topic.tsx:61-84

The hook uses createConnectQueryKey({ schema: listTopics, ..., cardinality: 'infinite' }) as its query key, but the queryFn fetches from the REST API and returns a GetTopicsResponse (not a Connect protobuf type). This is misleading and risks cache collisions or type mismatches if anything else uses the same Connect query key expecting protobuf data.

If the intent is to share invalidation with the Connect-based useListTopicsQuery, a simpler approach would be to keep ['topics'] as the key and add the Connect query key invalidation in useCreateTopicMutation.onSuccess explicitly.

---

5. Cache isolation between useTopicsQuery and useLegacyListTopicsQuery

frontend/src/react-query/api/topic.tsx

Both hooks hit the same /topics REST endpoint, but use different query keys (['topics'] vs the Connect key). When useCreateTopicMutation.onSuccess invalidates ['topics'] (line 155), it won't refresh results from useLegacyListTopicsQuery and vice versa — which can lead to stale topic lists after mutations.

---

[jvorcak]

I need to double check how we fetch permissions still, related to some work I've done on Friday

[malinskibeniamin]

You can rely on a useQueryClient hook to grab the singleton instance too.

[malinskibeniamin]

If we don't use the _force, should we drop it

[malinskibeniamin]

Can we avoid nested ternaries? I know this was there before but maybe an easy opportunity to clean up

[malinskibeniamin]

Why do we need this type of dynamic import? I would just import it, I guess react doctor might complain about it being a barrel file of some sort? I think in this case it would make it worse to have this import everywhere, just hoist it to the top of the file.

[malinskibeniamin]

@claude review again

[claude]

Claude finished @malinskibeniamin's task in 2m 15s —— View job

---

PR Review: Remove topics from backend-api

• Read CLAUDE.md and gather context
• Review changed files
• Post comprehensive review

---

Good progress since the last review — the hasEditPermissions auth bypass is properly fixed in topic-configuration.tsx:239-242, correctly deriving from allowedActions. A few things still need attention:

---

Open inline comments (from @malinskibeniamin, 2026-03-31)

1. Use useQueryClient() hook in admin-debug-bundle.tsx:220

The component imports and uses the queryClient singleton directly:

import queryClient from '../../../query-client';
// ...
useEffect(() => {
  queryClient.invalidateQueries({ queryKey: ['topicPartitionsAll'] });
}, []);

Since this is a functional component, the hook is available. Prefer const queryClient = useQueryClient(); inside the component body instead of the module import.

Fix this →

---

2. Drop unused _force parameter in connector-details.tsx:558

async refreshData(_force: boolean): Promise<void> {

The parameter is never read (underscore is just suppressing the lint warning). The body comment makes it clear topics are now handled by React Query in children. Drop the parameter and update the two call sites at lines 553 and 555.

Fix this →

---

3. Nested ternaries in topic-details.tsx:211-216

const topicConfig: ConfigEntry[] | null | undefined =
  topicConfigData === undefined
    ? undefined
    : topicConfigData === null || topicConfigData.error !== null
      ? null
      : topicConfigData.configEntries;

Could be an if/else block or early-return pattern to avoid the nested ternary, e.g.:

let topicConfig: ConfigEntry[] | null | undefined;
if (topicConfigData === undefined) {
  topicConfig = undefined;
} else if (topicConfigData === null || topicConfigData.error !== null) {
  topicConfig = null;
} else {
  topicConfig = topicConfigData.configEntries;
}

Fix this →

---

4. Inline dynamic imports in reassign-partitions.tsx:807-832

GetTopicsResponse and Partition are referenced with inline import(...) expressions in getter bodies:

queryClient.getQueryData<import('../../../state/rest-interfaces').GetTopicsResponse>(['topics'])
// ...
queryClient.getQueryData<Map<string, import('../../../state/rest-interfaces').Partition[] | null>>([...])

Partition is already imported at the top (line 54). GetTopicsResponse just needs to be added to the existing import. Both inline dynamic imports should be replaced with the top-level type references.

Fix this →

---

Remaining items from previous review

5. Missing response.ok checks — Still unaddressed in 7 query functions. Error responses (4xx/5xx) will be silently parsed as valid data:

• useLegacyListTopicsQuery line 80
• useTopicsQuery line 111
• useTopicDocumentationQuery line 268
• useTopicConsumersQuery line 298
• useTopicAclsQuery line 348
• useTopicPartitionsQuery line 401
• useAllTopicPartitionsQuery line 440

Compare: useUpdateTopicConfigMutation and useDeleteTopicMutation correctly throw on !response.ok.

Fix this →

6. useLegacyListTopicsQuery uses Connect query key for REST data (line 61-68) — The hook uses createConnectQueryKey as its React Query key but fetches from the REST API, not the Connect/protobuf layer. This risks cache collisions with useListTopicsQuery. No change yet.

---

Not blocking

Items 1–4 are all small, clean-up style changes that address @malinskibeniamin's review comments. Items 5–6 are pre-existing (carried over from the last review).