feat: unhide topic feed page + redesign input source editor - #783
Merged
Conversation
- Uncomment topic_feed routes in worktable.ts (TopicFeed and TopicFeedDetail) - Remove v-if="false" guard on topic detail button in observability page - Remove stale development-status comments Co-authored-by: Colin <Colin_XKL@outlook.com>
- Replace plain text inputs with type-aware source rows - Each row has a segmented selector: External RSS / Recipe / Topic - External RSS: free-form URL input (http/https) - Recipe: searchable dropdown loading from /api/admin/recipes - Topic: searchable dropdown loading from /api/admin/topics (excludes self when editing) - Auto-parse existing feedcraft:// URIs into the correct type+id on edit - Load picker data when modal opens (non-fatal fallback on error) - Add section dividers for 'Input Sources' and 'Aggregation Rules' - Update locale strings (zh-CN and en-US) for new UI elements Co-authored-by: Colin <Colin_XKL@outlook.com>
- Use CSS grid (auto 1fr auto) for input-source-row layout
- Add width: 100% to ensure grid container fills parent flex item
- Remove wrapper div approach; input/select are now direct grid children
- Remove redundant size='small' from radio group (prevent text wrapping)
- Add :deep(.arco-radio-group) { white-space: nowrap } for button labels
Co-authored-by: Colin <Colin_XKL@outlook.com>
Contributor
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
Contributor
Reviewer's GuideRe-enables the Topic Feed feature in the admin UI and redesigns the Topic Feed input source editor to support typed sources (external RSS, Recipe, Topic) with improved layout, while wiring these new UI models to existing topic feed APIs and observability navigation. Sequence diagram for TopicFeed input source editor lifecyclesequenceDiagram
actor AdminUser
participant TopicFeedPage
participant loadPickerData
participant getCustomRecipes
participant listTopicFeeds
participant parseUriToSource
participant sourceToUri
participant normalizeTopicPayload
AdminUser->>TopicFeedPage: openTopicFeedPage
TopicFeedPage->>loadPickerData: loadPickerData
loadPickerData->>getCustomRecipes: getCustomRecipes
loadPickerData->>listTopicFeeds: listTopicFeeds
getCustomRecipes-->>loadPickerData: recipesRes
listTopicFeeds-->>loadPickerData: topicsRes
loadPickerData-->>TopicFeedPage: pickerDataLoaded
alt editing_existing_topic
TopicFeedPage->>parseUriToSource: record.input_uris.map(parseUriToSource)
parseUriToSource-->>TopicFeedPage: InputSourceItem[]
end
AdminUser->>TopicFeedPage: submitTopicChanges
TopicFeedPage->>sourceToUri: formData.inputSources.map(sourceToUri)
sourceToUri-->>TopicFeedPage: string[]
TopicFeedPage->>normalizeTopicPayload: normalizeTopicPayload
normalizeTopicPayload-->>TopicFeedPage: TopicFeed
Note over TopicFeedPage: TopicFeed payload is then submitted via existing API call
Flow diagram for Observability navigation to TopicFeedDetail routeflowchart LR
AdminUser([Admin User]) --> ObservabilityPage
ObservabilityPage -->|resource_type === topic| goToTopicDetail
goToTopicDetail -->|router.push| TopicFeedDetailRoute[TopicFeedDetail route]
TopicFeedDetailRoute --> TopicFeedDetailPage[TopicFeed detail view]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In the template, the
Reciperadio label is hard-coded while other labels use i18n keys; consider switching it tot('topic.sourceType.recipe')for consistency and localization support. - The logic for creating a default
InputSourceItem(currently repeated indefaultFormData,addSource, andremoveSource) could be extracted into a small helper to avoid duplication and keep future changes to the default shape in one place.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In the template, the `Recipe` radio label is hard-coded while other labels use i18n keys; consider switching it to `t('topic.sourceType.recipe')` for consistency and localization support.
- The logic for creating a default `InputSourceItem` (currently repeated in `defaultFormData`, `addSource`, and `removeSource`) could be extracted into a small helper to avoid duplication and keep future changes to the default shape in one place.
## Individual Comments
### Comment 1
<location path="web/admin/src/views/dashboard/topic_feed/topic_feed.vue" line_range="450-452" />
<code_context>
- input_uris: formData.value.input_uris
- .map((item) => item.trim())
- .filter((item) => item !== ''),
+ input_uris: formData.value.inputSources
+ .map(sourceToUri)
+ .filter((uri) => uri !== ''),
aggregator_config: formData.value.aggregator_config.map((step) => {
const option: Record<string, string> = {};
</code_context>
<issue_to_address>
**issue (bug_risk):** Avoid emitting recipe/topic URIs when no resource has been selected yet.
When `sourceType` is `recipe` or `topic` and `resourceId` is empty, `sourceToUri` returns `feedcraft://recipe/` or `feedcraft://topic/`, which passes the `uri !== ''` filter and is sent in the payload. This is likely an invalid URI and could confuse backend handling/validation. Please either have `sourceToUri` return `''` when `resourceId` is falsy for non-external types, or filter `inputSources` so only entries with a non-empty `externalUrl`/`resourceId` (per `sourceType`) are mapped, so incomplete UI rows don’t emit malformed URIs.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+450
to
+452
| input_uris: formData.value.inputSources | ||
| .map(sourceToUri) | ||
| .filter((uri) => uri !== ''), |
Contributor
There was a problem hiding this comment.
issue (bug_risk): Avoid emitting recipe/topic URIs when no resource has been selected yet.
When sourceType is recipe or topic and resourceId is empty, sourceToUri returns feedcraft://recipe/ or feedcraft://topic/, which passes the uri !== '' filter and is sent in the payload. This is likely an invalid URI and could confuse backend handling/validation. Please either have sourceToUri return '' when resourceId is falsy for non-external types, or filter inputSources so only entries with a non-empty externalUrl/resourceId (per sourceType) are mapped, so incomplete UI rows don’t emit malformed URIs.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request enables the TopicFeed feature by activating its routes and UI entry points. It significantly improves the user experience in the Topic Feed editor by replacing manual URI inputs with a structured 'Input Sources' section, allowing users to select between External RSS, Recipes, or other Topics using dedicated controls. Review feedback identifies opportunities to enhance internationalization for new UI labels, ensure robust URI generation when selections are cleared, and improve the resilience of data fetching for the resource pickers by using Promise.allSettled.
Comment on lines
+143
to
+144
| <a-radio value="recipe">Recipe</a-radio> | ||
| <a-radio value="topic">Topic</a-radio> |
Contributor
There was a problem hiding this comment.
These radio button labels are hardcoded. They should use the internationalization keys defined in the locale files to maintain consistency and support multi-language support.
<a-radio value="recipe">{{
t('topic.sourceType.recipe')
}}</a-radio>
<a-radio value="topic">{{
t('topic.sourceType.topic')
}}</a-radio>
Comment on lines
+413
to
+421
| const sourceToUri = (source: InputSourceItem): string => { | ||
| if (source.sourceType === 'recipe') { | ||
| return `feedcraft://recipe/${source.resourceId}`; | ||
| } | ||
| if (source.sourceType === 'topic') { | ||
| return `feedcraft://topic/${source.resourceId}`; | ||
| } | ||
| return source.externalUrl.trim(); | ||
| }; |
Contributor
There was a problem hiding this comment.
If a user selects 'Recipe' or 'Topic' but clears the selection (via allow-clear), resourceId will be an empty string. The current implementation would return feedcraft://recipe/ or feedcraft://topic/, which are invalid URIs. It's better to return an empty string in these cases so they can be filtered out during payload normalization.
const sourceToUri = (source: InputSourceItem): string => {
if (source.sourceType === 'recipe') {
return source.resourceId ? `feedcraft://recipe/${source.resourceId}` : '';
}
if (source.sourceType === 'topic') {
return source.resourceId ? `feedcraft://topic/${source.resourceId}` : '';
}
return source.externalUrl.trim();
};
Comment on lines
+423
to
+437
| const loadPickerData = async () => { | ||
| pickerLoading.value = true; | ||
| try { | ||
| const [recipesRes, topicsRes] = await Promise.all([ | ||
| getCustomRecipes(), | ||
| listTopicFeeds(), | ||
| ]); | ||
| availableRecipes.value = recipesRes.data ?? []; | ||
| availableTopics.value = topicsRes.data ?? []; | ||
| } catch { | ||
| // non-fatal: picker continues with empty lists | ||
| } finally { | ||
| pickerLoading.value = false; | ||
| } | ||
| }; |
Contributor
There was a problem hiding this comment.
Using Promise.all means that if either getCustomRecipes() or listTopicFeeds() fails, the entire data loading process fails, and both lists remain empty. Using Promise.allSettled allows each request to succeed or fail independently, ensuring that if one API is down, the other picker can still function.
const loadPickerData = async () => {
pickerLoading.value = true;
try {
const [recipesRes, topicsRes] = await Promise.allSettled([
getCustomRecipes(),
listTopicFeeds(),
]);
if (recipesRes.status === 'fulfilled') {
availableRecipes.value = recipesRes.value.data ?? [];
}
if (topicsRes.status === 'fulfilled') {
availableTopics.value = topicsRes.value.data ?? [];
}
} catch {
// non-fatal
} finally {
pickerLoading.value = false;
}
};
WalkthroughTopic feed feature is enabled by re-activating routes and updating observability dashboard visibility. The topic input editor is refactored from flat input URIs to a structured model supporting external URLs, custom recipes, and internal topics, with new locale strings, type definitions, and helpers for parsing and serializing between formats. ChangesTopic Feed Feature Unhiding and Input Source Redesign
Possibly Related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@web/admin/src/views/dashboard/topic_feed/topic_feed.vue`:
- Around line 413-421: The sourceToUri function is generating malformed internal
URIs when resourceId is empty (e.g., producing "feedcraft://recipe/"), so update
sourceToUri to validate resourceId before building internal URIs: for
source.sourceType === 'recipe' or 'topic' check that source.resourceId is
non-empty (after trimming) and only then return
`feedcraft://recipe/${resourceId}` or `feedcraft://topic/${resourceId}`;
otherwise return an empty string (keep using source.externalUrl.trim() for
external sources) so the existing uri !== '' filter will correctly block
empty/invalid URIs. Ensure you reference and use sourceToUri, source.sourceType,
and source.resourceId in the change.
- Around line 423-433: loadPickerData currently uses Promise.all so a failure in
getCustomRecipes or listTopicFeeds cancels both results; instead call the two
APIs independently and handle failures separately: call getCustomRecipes() in
its own try/catch to set availableRecipes.value (default to []) and call
listTopicFeeds() in its own try/catch to set availableTopics.value (default to
[]), keep pickerLoading.value toggling (set true at start and false in finally),
and preserve non-fatal behavior so one endpoint failing doesn't wipe the other's
data; refer to loadPickerData, getCustomRecipes, listTopicFeeds,
availableRecipes, availableTopics, and pickerLoading.value when making the
changes.
- Around line 143-144: Replace the hardcoded English labels inside the a-radio
elements with the corresponding i18n translations (use the Vue $t call in the
template) so the UI uses the new locale keys; e.g., change <a-radio
value="recipe">Recipe</a-radio> and <a-radio value="topic">Topic</a-radio> to
use {{$t('...')}} (the existing locale keys for recipe and topic) while keeping
the value attributes intact so the selection logic (the a-radio group) continues
to work.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d2b35a1d-7b58-43b3-8873-145a115418e1
📒 Files selected for processing (5)
web/admin/src/locale/en-US/topic.tsweb/admin/src/locale/zh-CN/topic.tsweb/admin/src/router/routes/modules/worktable.tsweb/admin/src/views/dashboard/observability/index.vueweb/admin/src/views/dashboard/topic_feed/topic_feed.vue
Comment on lines
+143
to
+144
| <a-radio value="recipe">Recipe</a-radio> | ||
| <a-radio value="topic">Topic</a-radio> |
There was a problem hiding this comment.
Localize source-type labels instead of hardcoding English text.
The new locale keys exist, but Line 143 and Line 144 still render hardcoded labels, so Chinese UI won’t fully localize here.
💡 Suggested fix
- <a-radio value="recipe">Recipe</a-radio>
- <a-radio value="topic">Topic</a-radio>
+ <a-radio value="recipe">{{
+ t('topic.sourceType.recipe')
+ }}</a-radio>
+ <a-radio value="topic">{{
+ t('topic.sourceType.topic')
+ }}</a-radio>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| <a-radio value="recipe">Recipe</a-radio> | |
| <a-radio value="topic">Topic</a-radio> | |
| <a-radio value="recipe">{{ | |
| t('topic.sourceType.recipe') | |
| }}</a-radio> | |
| <a-radio value="topic">{{ | |
| t('topic.sourceType.topic') | |
| }}</a-radio> |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/admin/src/views/dashboard/topic_feed/topic_feed.vue` around lines 143 -
144, Replace the hardcoded English labels inside the a-radio elements with the
corresponding i18n translations (use the Vue $t call in the template) so the UI
uses the new locale keys; e.g., change <a-radio value="recipe">Recipe</a-radio>
and <a-radio value="topic">Topic</a-radio> to use {{$t('...')}} (the existing
locale keys for recipe and topic) while keeping the value attributes intact so
the selection logic (the a-radio group) continues to work.
Comment on lines
+413
to
+421
| const sourceToUri = (source: InputSourceItem): string => { | ||
| if (source.sourceType === 'recipe') { | ||
| return `feedcraft://recipe/${source.resourceId}`; | ||
| } | ||
| if (source.sourceType === 'topic') { | ||
| return `feedcraft://topic/${source.resourceId}`; | ||
| } | ||
| return source.externalUrl.trim(); | ||
| }; |
There was a problem hiding this comment.
Avoid emitting malformed internal URIs for empty source selections.
sourceToUri currently serializes empty resourceId values to feedcraft://recipe/ or feedcraft://topic/, which then bypasses the uri !== '' filter and reaches validation/save paths.
💡 Suggested fix
const sourceToUri = (source: InputSourceItem): string => {
+ const id = source.resourceId?.trim();
if (source.sourceType === 'recipe') {
- return `feedcraft://recipe/${source.resourceId}`;
+ return id ? `feedcraft://recipe/${id}` : '';
}
if (source.sourceType === 'topic') {
- return `feedcraft://topic/${source.resourceId}`;
+ return id ? `feedcraft://topic/${id}` : '';
}
return source.externalUrl.trim();
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const sourceToUri = (source: InputSourceItem): string => { | |
| if (source.sourceType === 'recipe') { | |
| return `feedcraft://recipe/${source.resourceId}`; | |
| } | |
| if (source.sourceType === 'topic') { | |
| return `feedcraft://topic/${source.resourceId}`; | |
| } | |
| return source.externalUrl.trim(); | |
| }; | |
| const sourceToUri = (source: InputSourceItem): string => { | |
| const id = source.resourceId?.trim(); | |
| if (source.sourceType === 'recipe') { | |
| return id ? `feedcraft://recipe/${id}` : ''; | |
| } | |
| if (source.sourceType === 'topic') { | |
| return id ? `feedcraft://topic/${id}` : ''; | |
| } | |
| return source.externalUrl.trim(); | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/admin/src/views/dashboard/topic_feed/topic_feed.vue` around lines 413 -
421, The sourceToUri function is generating malformed internal URIs when
resourceId is empty (e.g., producing "feedcraft://recipe/"), so update
sourceToUri to validate resourceId before building internal URIs: for
source.sourceType === 'recipe' or 'topic' check that source.resourceId is
non-empty (after trimming) and only then return
`feedcraft://recipe/${resourceId}` or `feedcraft://topic/${resourceId}`;
otherwise return an empty string (keep using source.externalUrl.trim() for
external sources) so the existing uri !== '' filter will correctly block
empty/invalid URIs. Ensure you reference and use sourceToUri, source.sourceType,
and source.resourceId in the change.
Comment on lines
+423
to
+433
| const loadPickerData = async () => { | ||
| pickerLoading.value = true; | ||
| try { | ||
| const [recipesRes, topicsRes] = await Promise.all([ | ||
| getCustomRecipes(), | ||
| listTopicFeeds(), | ||
| ]); | ||
| availableRecipes.value = recipesRes.data ?? []; | ||
| availableTopics.value = topicsRes.data ?? []; | ||
| } catch { | ||
| // non-fatal: picker continues with empty lists |
There was a problem hiding this comment.
Load recipe/topic pickers independently to preserve partial success.
Using Promise.all means one failed request drops both picker datasets. That degrades edit UX when only one endpoint is temporarily failing.
💡 Suggested fix
const loadPickerData = async () => {
pickerLoading.value = true;
try {
- const [recipesRes, topicsRes] = await Promise.all([
+ const [recipesRes, topicsRes] = await Promise.allSettled([
getCustomRecipes(),
listTopicFeeds(),
]);
- availableRecipes.value = recipesRes.data ?? [];
- availableTopics.value = topicsRes.data ?? [];
+ availableRecipes.value =
+ recipesRes.status === 'fulfilled' ? recipesRes.value.data ?? [] : [];
+ availableTopics.value =
+ topicsRes.status === 'fulfilled' ? topicsRes.value.data ?? [] : [];
} catch {
// non-fatal: picker continues with empty lists
} finally {
pickerLoading.value = false;
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| const loadPickerData = async () => { | |
| pickerLoading.value = true; | |
| try { | |
| const [recipesRes, topicsRes] = await Promise.all([ | |
| getCustomRecipes(), | |
| listTopicFeeds(), | |
| ]); | |
| availableRecipes.value = recipesRes.data ?? []; | |
| availableTopics.value = topicsRes.data ?? []; | |
| } catch { | |
| // non-fatal: picker continues with empty lists | |
| const loadPickerData = async () => { | |
| pickerLoading.value = true; | |
| try { | |
| const [recipesRes, topicsRes] = await Promise.allSettled([ | |
| getCustomRecipes(), | |
| listTopicFeeds(), | |
| ]); | |
| availableRecipes.value = | |
| recipesRes.status === 'fulfilled' ? recipesRes.value.data ?? [] : []; | |
| availableTopics.value = | |
| topicsRes.status === 'fulfilled' ? topicsRes.value.data ?? [] : []; | |
| } catch { | |
| // non-fatal: picker continues with empty lists | |
| } finally { | |
| pickerLoading.value = false; | |
| } | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@web/admin/src/views/dashboard/topic_feed/topic_feed.vue` around lines 423 -
433, loadPickerData currently uses Promise.all so a failure in getCustomRecipes
or listTopicFeeds cancels both results; instead call the two APIs independently
and handle failures separately: call getCustomRecipes() in its own try/catch to
set availableRecipes.value (default to []) and call listTopicFeeds() in its own
try/catch to set availableTopics.value (default to []), keep pickerLoading.value
toggling (set true at start and false in finally), and preserve non-fatal
behavior so one endpoint failing doesn't wipe the other's data; refer to
loadPickerData, getCustomRecipes, listTopicFeeds, availableRecipes,
availableTopics, and pickerLoading.value when making the changes.
This was referenced May 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
变更说明
1. 取消隐藏 Topic Feed 页面
worktable.ts中两条路由的注释（TopicFeed和TopicFeedDetail）v-if条件（移除false &&）2. 重设计输入源编辑器
每条输入源行现在包含三个模式：
/api/admin/recipes加载）feedcraft://recipe/:id/api/admin/topics加载，排除自身）feedcraft://topic/:idfeedcraft://URI 并回填到对应类型+选择器3. 布局优化
a-divider将表单分为「输入源」/「聚合规则」两个视觉区段auto 1fr auto) 布局，确保输入/选择控件填充剩余宽度测试结果
topic_feed_input_redesign_test.mp4
新版输入源行：外部RSS 模式，带完整宽度输入框
切换为 Recipe 类型时的下拉选择器
创建成功后的列表页
task fix、go test ./...、task frontend-build全部通过To show artifacts inline, enable in settings.
Summary by Sourcery
Expose the Topic Feed feature in the admin UI and redesign the topic input source editor to support typed sources with better layout and selection UX.
New Features:
Enhancements: