feat: optimize host onboarding - #2230

src/pages/hosts/pages/List/CollectSetup/index.tsx (2)

160-164: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value

Read the remembered IDs once with a lazy state initializer.

useRef(readVerifiedDatasourceIds()) evaluates its argument on every render. The ref keeps only the first value, so each later render performs a localStorage.getItem plus JSON.parse and discards the result. A lazy useState initializer runs the read once.

♻️ Proposed refactor

-  const rememberedDatasourceIds = useRef(readVerifiedDatasourceIds()).current;
+  const [rememberedDatasourceIds] = useState(readVerifiedDatasourceIds);

🤖 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 `@src/pages/hosts/pages/List/CollectSetup/index.tsx` around lines 160 - 164,
Replace the useRef(readVerifiedDatasourceIds()) initialization with a lazy
useState initializer so readVerifiedDatasourceIds executes only once, while
preserving the rememberedDatasourceIds value used by defaultFromRemembered.

---

196-207: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Declare the effect dependencies or add the lint exemption.

The effect reads arrivalDatasources, arrival.hitDatasourceNames, and datasourceIds, but the dependency array lists only arrival.status. The verifiedMarkedRef guard makes the single-run behavior intentional, so state the intent explicitly. The same file already uses this pattern at Line 291.

♻️ Proposed change

     writeVerifiedDatasourceIds(hitIds.length > 0 ? hitIds : datasourceIds);
+    // 只在 detected 的那一次写入，其余依赖有意省略
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [arrival.status]);

🤖 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 `@src/pages/hosts/pages/List/CollectSetup/index.tsx` around lines 196 - 207,
Update the useEffect handling arrival verification to explicitly document its
intentional single-run behavior, following the existing pattern around the later
effect in the same file. Either add the referenced values to the dependency
array while preserving the verifiedMarkedRef guard, or add the established lint
exemption for intentional omitted dependencies; ensure arrivalDatasources,
arrival.hitDatasourceNames, and datasourceIds are covered by the chosen
approach.

src/components/OnboardingProgress/detect.test.ts (1)

80-84: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Make the storage-unavailable test exercise the catch branch.

The test relies on delete globalThis.localStorage in afterEach. Under jsdom, localStorage is defined on window, so the delete may not remove it. The assertions then still pass because the key is absent, and the try/catch in detect.ts is never exercised. Install a throwing stub to test the failure path directly.

🧪 Proposed test change

   it('reports false instead of throwing when storage is unavailable', () => {
     // 隐私模式 / 禁用存储：读写都不能把整轮进度探测带崩
+    (globalThis as { localStorage?: unknown }).localStorage = {
+      getItem: () => {
+        throw new Error('storage disabled');
+      },
+      setItem: () => {
+        throw new Error('storage disabled');
+      },
+    };
     expect(() => writeOnboardingMarker('testDelivered')).not.toThrow();
     expect(readOnboardingMarker('testDelivered')).toBe(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 `@src/components/OnboardingProgress/detect.test.ts` around lines 80 - 84,
Update the storage-unavailable test around writeOnboardingMarker and
readOnboardingMarker to replace localStorage with a stub whose access methods
throw, ensuring both operations enter the catch path directly. Preserve the
existing no-throw and false-result assertions, and restore the original storage
state through the test cleanup.

src/components/OnboardingActions/SendTestAlert/index.tsx (1)

98-101: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use a native type check instead of _.isString.

Line 99 uses _.isString(res?.dat). Replace it with typeof res?.dat === 'string'.

As per coding guidelines: "For type narrowing, prefer native checks such as Array.isArray, typeof x === 'number', and x == null over lodash type-guard functions."

♻️ Proposed fix

-          (res) => ({ index, label, ok: true, detail: _.isString(res?.dat) ? res.dat : undefined } as SendResult),
+          (res) => ({ index, label, ok: true, detail: typeof res?.dat === 'string' ? res.dat : undefined } as SendResult),

🤖 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 `@src/components/OnboardingActions/SendTestAlert/index.tsx` around lines 98 -
101, In the notifyRuleTest success handler within the SendResult mapping,
replace the lodash _.isString(res?.dat) check with the native typeof res?.dat
=== 'string' type guard, preserving the existing detail fallback and error
handling.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

src/components/OnboardingActions/HostMonitorPack/index.tsx (1)

178-199: 🎯 Functional Correctness | 🟠 Major

Rule parse failures still disappear from results (regression).

buildAlertRuleImportBody failures return null in the catch block. _.compact drops that entry from ruleBodies before it reaches existedRules, rulesToImport, or skippedResults. The failing rule never appears in results.rules, unlike the board path, which keeps a 'failed' entry with t('pack.bad_template') (Line 170).

This is the same defect flagged in a prior review round and marked addressed. The current refactor, which adds the bodyName partition logic, dropped the failure-tracking fix. Reintroduce it: collect parse failures separately and merge them back into the final ruleTask result.

🐛 Proposed fix

+      const ruleParseFailures: ImportItemResult[] = [];
       const ruleBodies = _.compact(
         _.map(selectedRules, (payload) => {
           try {
             const body = buildAlertRuleImportBody(payload.content, { datasourceQueries: values.datasource_queries, notifyRuleIds });
             // bodyName 才是后端落库与响应 map 的 key（payload.name 只是列表展示名，两者通常一致）
             return { name: payload.name, bodyName: body.name ?? payload.name, body };
           } catch (e) {
-            return null;
+            ruleParseFailures.push({ name: payload.name, status: 'failed', detail: t('pack.bad_template') });
+            return null;
           }
         }),
       );
       // 与大盘同一口径：重名跳过、不覆盖。不能走后端 force 导入 —— 那是按 (group_id, name)
       // 的整行覆盖，会把用户改过阈值 / 主动停用 / 换过通知绑定的同名规则静默还原（见 services.ts）
       const [existedRules, rulesToImport] = _.partition(ruleBodies, (item) => existing.rules[item.bodyName]);
       const skippedResults = _.map(existedRules, (item) => ({ name: item.name, status: 'skipped' } as ImportItemResult));
       const ruleTask: Promise<ImportItemResult[]> = _.isEmpty(rulesToImport)
-        ? Promise.resolve(skippedResults)
+        ? Promise.resolve<ImportItemResult[]>(skippedResults)
         : importAlertRules(targetBgid, _.map(rulesToImport, 'body')).then(
             (res) => [..._.map(rulesToImport, (item) => ({ name: item.name, status: res?.[item.bodyName] ? 'failed' : 'ok', detail: res?.[item.bodyName] } as ImportItemResult)), ...skippedResults],
             (err) => [..._.map(rulesToImport, (item) => ({ name: item.name, status: 'failed', detail: err?.message || t('pack.unknown_error') } as ImportItemResult)), ...skippedResults],
-          );
+          );
+      const ruleTaskWithParseFailures = ruleTask.then((items) => [...ruleParseFailures, ...items]);

Replace the later ruleTask reference in Promise.all([Promise.all(boardTasks), ruleTask]) with ruleTaskWithParseFailures.

🤖 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 `@src/components/OnboardingActions/HostMonitorPack/index.tsx` around lines 178
- 199, Update the rule-processing flow around buildAlertRuleImportBody to
preserve parse failures as failed results instead of dropping them via
_.compact. Collect failed payloads with the existing bad-template result shape
and merge them into the final rule task, then use ruleTaskWithParseFailures in
the Promise.all result assembly so each failed rule appears in results.rules
alongside imported and skipped rules.

src/components/OnboardingActions/SendTestAlert/interpretResponse.test.ts (1)

12-12: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Narrow the provider-response fixtures with as const.

Apply as const to these literal fixtures. This preserves their narrow literal types.

Proposed change

-    const dat = 'status_code:200, response:{"errcode":310000,"errmsg":"keywords not in content"}';
+    const dat = 'status_code:200, response:{"errcode":310000,"errmsg":"keywords not in content"}' as const;

As per coding guidelines, “Prefer as const for test data literals to preserve narrow literal types.”

Also applies to: 17-17, 22-22, 28-28

🤖 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 `@src/components/OnboardingActions/SendTestAlert/interpretResponse.test.ts` at
line 12, Update the provider-response fixture literals in
interpretResponse.test.ts, including dat and the additional fixtures at the
referenced locations, by applying as const so their narrow literal types are
preserved. Do not change the fixture values or test behavior.

Source: Coding guidelines

src/components/OnboardingActions/SendTestAlert/interpretResponse.ts (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Use native checks for type narrowing.

Replace _.isString(dat) with typeof dat !== 'string'. Replace _.isNumber(errcode) with typeof errcode === 'number'. Then remove the Lodash import.

Proposed change

-import _ from 'lodash';
-
 export interface TestSendOutcome {
@@
-  if (!_.isString(dat) || dat === '' || dat === 'success') {
+  if (typeof dat !== 'string' || dat === '' || dat === 'success') {
@@
-    if (_.isNumber(errcode) && errcode !== 0) {
+    if (typeof errcode === 'number' && errcode !== 0) {

As per coding guidelines, “For type narrowing in JavaScript and TypeScript code, prefer native checks … over equivalent Lodash predicates.”

Also applies to: 23-31

🤖 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 `@src/components/OnboardingActions/SendTestAlert/interpretResponse.ts` at line
1, In interpretResponse, replace the Lodash predicates used for dat and errcode
type narrowing with native typeof checks, using typeof dat !== 'string' and
typeof errcode === 'number' semantics. Remove the now-unused Lodash import while
preserving the surrounding response handling.

Source: Coding guidelines

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes