Skip to content

Commit d999631

Browse files
vanch3dvanch3d
authored
Merge pull request #1240
* refactor(33168): Refactor the detection of duplicate combiner/mapper * refactor(33168): refactor the logic * refactor(33168): refactor the duplication UX * test(33168): add e2e tests * test(33168): add e2e tests * test(33168): fix tests and POs * test(33168): fix tests * chore(33168): update CLAUDE context * chore(33168): trigger CI * chore(33168): linting * chore(33168): a bit of cleaning
1 parent eabc73c commit d999631

32 files changed

+5774
-125
lines changed

‎.github/workflows/trigger.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
Again again. I know ...
1+
last time

‎hivemq-edge-frontend/.github/copilot-instructions.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,4 +50,9 @@ This directory contains comprehensive task documentation, progress tracking, and
5050

5151
**When working on tasks, AI agents should consult this directory first.**
5252

53+
## Important Guidelines
54+
55+
- **Design Guidelines**: [.tasks/DESIGN_GUIDELINES.md](../.tasks/DESIGN_GUIDELINES.md) - UI component patterns, button variants, styling conventions
56+
- **Testing Guidelines**: [.tasks/TESTING_GUIDELINES.md](../.tasks/TESTING_GUIDELINES.md) - Mandatory accessibility testing, component test patterns
57+
5358
See [README.md](.tasks/README.md) for full documentation structure.

‎hivemq-edge-frontend/.tasks/33168-duplicate-combiner/ACCESSIBILITY_TEST_FIX.md‎

Whitespace-only changes.

‎hivemq-edge-frontend/.tasks/33168-duplicate-combiner/BUG_FIXES.md‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
# Bug Fixes for E2E Test File
2+
3+
**Date:** October 24, 2025
4+
**File:** `cypress/e2e/workspace/duplicate-combiner.spec.cy.ts`
5+
6+
---
7+
8+
## Issues Fixed
9+
10+
### 1. TypeScript Error (Line 54) ✅
11+
12+
**Problem:**
13+
14+
```typescript
15+
const sourceIds = combiner.sources?.items?.map((s: { identifier: string }) => s.identifier).join('-') || ''
16+
```
17+
18+
Error: `Property 'identifier' is missing in type 'EntityReference'`
19+
20+
**Root Cause:**
21+
The `EntityReference` type has an `id` property, not `identifier`. The type is defined as:
22+
23+
```typescript
24+
export type EntityReference = {
25+
type: EntityType
26+
id: string // ← Not 'identifier'
27+
}
28+
```
29+
30+
**Fix:**
31+
Changed `identifier` to `id`:
32+
33+
```typescript
34+
const sourceIds = combiner.sources?.items?.map((s) => s.id).join('-') || ''
35+
```
36+
37+
### 2. ESLint Error - Unused Variable ✅
38+
39+
**Problem:**
40+
41+
```typescript
42+
const COMBINER_ID_1 = '9e975b62-6f8d-410f-9007-3f83719aec6f'
43+
const COMBINER_ID_2 = 'combiner-duplicate-attempt'
44+
```
45+
46+
Error: `'COMBINER_ID_1' is assigned a value but never used`
47+
48+
**Fix:**
49+
Removed `COMBINER_ID_1` and simplified to use a single `COMBINER_ID` constant matching the existing combiner test pattern.
50+
51+
### 3. ESLint Warnings - Unused Aliases ✅
52+
53+
**Problem:**
54+
Multiple unused `cy.as()` aliases:
55+
56+
- `getProtocols`
57+
- `getAdapters`
58+
- `getTopicFilters`
59+
- `deleteCombiner`
60+
61+
**Fix:**
62+
Removed all unused `.as()` calls from intercepts that don't need to be waited on.
63+
64+
### 4. Test Failures - Incorrect Combiner ID Format ✅
65+
66+
**Problem:**
67+
Tests were using incorrect combiner ID format:
68+
69+
```typescript
70+
const firstCombinerId = `combiner-adapter@opcua-pump-adapter@opcua-boiler`
71+
```
72+
73+
This format doesn't match how the mock API generates IDs, causing tests to fail when looking for combiner nodes.
74+
75+
**Fix:**
76+
Simplified the ID generation approach to use a single fixed `COMBINER_ID` constant throughout all tests, matching the pattern used in the existing `combiner.spec.cy.ts` test file. This makes the tests more predictable and maintainable.
77+
78+
**Changes:**
79+
80+
- Removed dynamic ID generation based on source IDs
81+
- Used fixed `COMBINER_ID = '9e975b62-6f8d-410f-9007-3f83719aec6f'`
82+
- Updated all test assertions to use `COMBINER_ID` constant
83+
- Removed redundant `firstCombinerId` variables
84+
85+
---
86+
87+
## Summary of Changes
88+
89+
**Lines Modified:**
90+
91+
- Line 11: Removed unused `COMBINER_ID_1` constant
92+
- Line 32-48: Removed `.as()` aliases from unused intercepts
93+
- Line 52-61: Simplified POST combiner intercept to use fixed ID
94+
- Line 94: Removed unused `.as('deleteCombiner')`
95+
- Lines 118, 164, 189, 234, 254, 321: Removed local `firstCombinerId` variables
96+
- All tests: Updated to use global `COMBINER_ID` constant
97+
98+
**Result:**
99+
100+
- ✅ All TypeScript errors resolved
101+
- ✅ All ESLint errors resolved
102+
- ✅ Tests now use consistent, predictable IDs
103+
- ✅ Code follows existing test patterns
104+
105+
---
106+
107+
## Test Execution
108+
109+
To run the fixed tests:
110+
111+
```bash
112+
# Terminal 1: Start dev server
113+
pnpm dev
114+
115+
# Terminal 2: Run E2E tests
116+
pnpm exec cypress run --spec "cypress/e2e/workspace/duplicate-combiner.spec.cy.ts"
117+
```
118+
119+
Note: E2E tests require the dev server to be running to access the application at `http://localhost:3000`.

‎hivemq-edge-frontend/.tasks/33168-duplicate-combiner/CONVERSATION_SUBTASK_1.md‎

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,152 @@
1+
3. **`findExistingCombiner(allNodes, targetReferences)`**
2+
3+
- Finds existing combiner with matching sources
4+
- Uses existing `arrayWithSameObjects` utility
5+
- Handles sources in any order
6+
- Pure function, fully testable
7+
8+
4. **`filterCombinerCandidates(selectedNodes, adapterTypes)`**
9+
10+
- Filters nodes to only combiner-eligible ones
11+
- Wraps `isNodeCombinerCandidate` for array filtering
12+
- Returns undefined if no candidates found
13+
14+
5. **`isAssetMapperCombiner(nodes)`**
15+
- Checks if combiner should be an asset mapper
16+
- Detects presence of pulse nodes
17+
- Simple boolean check
18+
19+
**Type Exports:**
20+
21+
- `CombinerEligibleNode` - Union type for valid combiner source nodes
22+
23+
#### 2. Created Comprehensive Tests ✅
24+
25+
**File**: `src/modules/Workspace/utils/toolbar.utils.spec.ts`
26+
27+
**Test Coverage:**
28+
29+
- ✅ 29 test cases covering all functions
30+
- ✅ All tests passing
31+
- ✅ Edge cases covered:
32+
- Nodes with/without COMBINE capability
33+
- Missing adapter types
34+
- Different node types (adapters, bridges, pulse, edge)
35+
- Empty arrays
36+
- Undefined parameters
37+
- Source order variations
38+
- Different source lengths
39+
- Multiple matching combiners
40+
41+
**Test Results:**
42+
43+
```
44+
Test Files 1 passed (1)
45+
Tests 29 passed (29)
46+
Duration 1.04s
47+
```
48+
49+
#### 3. Refactored ContextualToolbar Component ✅
50+
51+
**File**: `src/modules/Workspace/components/nodes/ContextualToolbar.tsx`
52+
53+
**Changes Made:**
54+
55+
1. **Updated Imports**
56+
57+
- Removed: `EntityType`, `arrayWithSameObjects`, `CombinerEligibleNode` type
58+
- Added: All new toolbar utility functions
59+
60+
2. **Simplified `selectedCombinerCandidates` Memoized Selector**
61+
62+
- **Before**: Complex inline filter with adapter type checking (8 lines)
63+
- **After**: Single line call to `filterCombinerCandidates(selectedNodes, data?.items)`
64+
65+
3. **Simplified `isAssetManager` Memoized Selector**
66+
67+
- **Before**: `.find()` check for pulse nodes (3 lines)
68+
- **After**: Single line call to `isAssetMapperCombiner(selectedCombinerCandidates)`
69+
70+
4. **Simplified `onManageTransformationNode` Function**
71+
- **Before**: Complex inline logic for:
72+
- Building entity references (15 lines)
73+
- Finding existing combiner (11 lines)
74+
- **After**: Clean, readable function calls:
75+
```typescript
76+
const entityReferences = buildEntityReferencesFromNodes(selectedCombinerCandidates)
77+
const existingCombiner = findExistingCombiner(nodes, entityReferences)
78+
```
79+
80+
**Benefits Achieved:**
81+
82+
- ✅ Reduced complexity in component
83+
- ✅ Extracted logic is now unit testable
84+
- ✅ Better separation of concerns
85+
- ✅ More maintainable code
86+
- ✅ Preserved all existing functionality
87+
- ✅ No breaking changes
88+
89+
#### 4. Validation ✅
90+
91+
- ✅ All utility tests passing (29/29)
92+
- ✅ TypeScript type checking passed
93+
- ✅ No compilation errors
94+
- ✅ Component successfully refactored
95+
- ✅ All functions properly exported and imported
96+
97+
---
98+
99+
## Summary of Phase 1
100+
101+
**Files Created:**
102+
103+
1. `src/modules/Workspace/utils/toolbar.utils.ts` (100 lines)
104+
2. `src/modules/Workspace/utils/toolbar.utils.spec.ts` (500+ lines)
105+
106+
**Files Modified:**
107+
108+
1. `src/modules/Workspace/components/nodes/ContextualToolbar.tsx` (simplified ~40 lines)
109+
110+
**Test Coverage:**
111+
112+
- 5 new utility functions
113+
- 29 comprehensive test cases
114+
- 100% coverage of utility functions
115+
- All tests passing ✅
116+
117+
**Code Quality Improvements:**
118+
119+
- Reduced cyclomatic complexity in component
120+
- Improved readability with descriptive function names
121+
- Better testability with pure functions
122+
- Maintained existing UX and functionality
123+
124+
---
125+
126+
## Next Steps
127+
128+
Phase 1 is complete! Ready to move to Phase 2 when user is ready:
129+
130+
**Phase 2: Improve UX**
131+
132+
- Enhance duplicate detection messaging
133+
- Improve toast notifications
134+
- Consider visual feedback options
135+
- Possibly add user choice modal
136+
137+
**Phase 3: Add Component Tests**
138+
139+
- Cypress component tests for ContextualToolbar
140+
- Test toolbar rendering with different selections
141+
- Test button states and interactions
142+
- Test toast notifications
143+
- Test navigation behavior
144+
145+
---
146+
147+
## Notes
148+
149+
- All existing functionality preserved
150+
- No breaking changes introduced
151+
- UX remains the same (as requested)
152+
- Ready for Phase 2 UX improvements

0 commit comments

Comments
 (0)