Skip to content
This repository was archived by the owner on Jun 26, 2026. It is now read-only.

Commit 9a8b3e5

Browse files
matejvasekclaude
andcommitted
refactor: co-locate page components, extract shared code
Restructure src/ into pages/ and common/ directories. Page-specific components (CreateFunctionForm, FileTreeView, FunctionTable, EmptyState) move next to their page. Shared code (services, context, utils, UserAvatar) moves to common/. Extract EditToolbar (with LeaveModal) from FunctionEditPage into its own file with tests. Replace runtime import of @patternfly/react-code-editor with import type to avoid pulling Monaco workers into the bundle. Update ARCHITECTURE.md, TESTING.md, and WORKFLOW.md with co-location convention and related rules. Also add yarn ci script, rework init-session startup sequence, and clean up features.json. Co-Authored-By: Claude <noreply@anthropic.com> Signed-off-by: Matej Vašek <matejvasek@gmail.com> Signed-off-by: Stanislav Jakuschevskij <sjakusch@redhat.com> Signed-off-by: Matej Vašek <matejvasek@gmail.com>
1 parent 5ca7a87 commit 9a8b3e5

43 files changed

Lines changed: 944 additions & 356 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.claude/commands/init-session.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,10 @@
11
---
22
allowed-tools: Bash(git log:*), Bash(pwd), Bash(./init.sh), Bash(yarn test*), Bash(cat .dev-env.json), Read
3-
description: Run startup sequence (steps 1-6 from docs/WORKFLOW.md)
3+
description: Run startup sequence and pick a story from the Jira epic
44
---
55

66
# Session Onboard
77

8-
Execute the startup sequence from `docs/WORKFLOW.md`. AGENTS.md is always in context — no need to read it explicitly.
9-
108
## Steps
119

1210
1. **Confirm working directory** — run `pwd`.
@@ -17,8 +15,10 @@ Execute the startup sequence from `docs/WORKFLOW.md`. AGENTS.md is always in con
1715
```
1816

1917
3. **Check struggles** — read `docs/agent-struggles.json`. If unresolved entries exist, present to user.
20-
4. **Pick feature** — read `docs/features.json`, find first `"passes": false` entry.
21-
5. **Start dev env** — run `./init.sh`.
22-
6. **Read ports** — read `.dev-env.json` and note the backend, plugin, and console ports.
23-
7. **Run tests** — run `yarn test` and verify app is healthy.
24-
8. **Wait** — tell the user you're oriented, report the picked feature and which step of the Feature Development Sequence you'd start at. When the user says to proceed, follow the Feature Development Sequence in `docs/WORKFLOW.md` step by step. Do NOT start any work autonomously.
18+
4. **CI check** — run `yarn ci` (lint, test, build) and verify the project is healthy.
19+
5. **Start dev env** — run `./init.sh`. If it fails (e.g. nono sandbox blocks `oc`), tell the user to start it manually from their terminal.
20+
6. **Read ports** — read `.dev-env.json` and note the backend, plugin, and console ports. If init.sh failed, skip this step.
21+
7. **Pick story** — tell the user you're oriented and propose picking a story from the PoC epic: <https://redhat.atlassian.net/browse/SRVOCF-810>. Ask the user to provide the story description (title, acceptance criteria, or a Jira link). The user may pick one story or a few small ones.
22+
8. **Create feature entry** — once the user provides a story, create a new entry in `docs/features.json` for it (append to the array, `"passes": false`).
23+
9. **Branch** — create a feature branch per [Branching](docs/WORKFLOW.md#branching) convention and open a draft PR (`gh pr create --draft`).
24+
10. **Propose planning** — tell the user the branch is ready and propose to start planning (step 2 of the Feature Development Sequence in `docs/WORKFLOW.md`). Do NOT start any work autonomously.

docs/ARCHITECTURE.md

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ flowchart TB
1111
TYPES[Types] ---|cross-cutting| UTILS[Utils]
1212
SERVICES[Services] ---|cross-cutting| UTILS
1313
COMPONENTS[Components] ---|cross-cutting| UTILS
14-
VIEWS[Views] --> COMPONENTS[Components]
15-
VIEWS --> HOOKS[Hooks]
14+
PAGES[Pages] --> COMPONENTS[Components]
15+
PAGES --> HOOKS[Hooks]
1616
COMPONENTS --> HOOKS
1717
COMPONENTS --> TYPES
1818
HOOKS --> SERVICES[Services]
@@ -24,34 +24,47 @@ Arrows mean "imports / depends on."
2424

2525
| Layer | Maps to | Depends on |
2626
|-------|---------|------------|
27-
| **Types** | `services/types.ts` | nothing |
28-
| **Services** | `services/*/Service.ts` + implementations | Types, Utils |
29-
| **Hooks** | `services/*/use*.ts` — wiring layer | Services, Types, Utils |
30-
| **Components** | `components/` — FunctionTable, CreateForm, etc. | Hooks, Types, Utils |
31-
| **Views** | `views/` — page-level components | Components, Hooks, Utils |
32-
| **Utils** | `utils/` — constants, helpers | nothing (cross-cutting) |
27+
| **Types** | `common/services/types.ts` | nothing |
28+
| **Services** | `common/services/*/Service.ts` + implementations | Types, Utils |
29+
| **Hooks** | `common/services/*/use*.ts`, `common/hooks/`, `pages/<name>/hooks/` | Services, Types, Utils |
30+
| **Components** | `common/components/` (shared), `pages/<name>/components/` (page-specific) | Hooks, Types, Utils |
31+
| **Pages** | `pages/<name>/` | Components, Hooks, Utils |
32+
| **Utils** | `common/utils/` | nothing (cross-cutting) |
3333

3434
### Dependency Rules
3535

36-
- Unidirectional: Types <- Services <- Hooks <- Components <- Views
36+
- Unidirectional: Types <- Services <- Hooks <- Components <- Pages
3737
- Utils can be imported by any layer
38-
- Views never import Services directly (always through Hooks)
39-
- Services never import Components or Views
38+
- Pages never import Services directly (always through Hooks)
39+
- Services never import Components or Pages
4040
- No circular dependencies
4141

42+
### Co-location Convention
43+
44+
- `src/pages/<name>/` contains the page component, its test, and a `components/` subdir
45+
- `src/pages/<name>/components/` contains components used only by that page
46+
- `src/common/` contains everything shared across pages (components, services, utils, context)
47+
- **Ownership rule:** if a component is imported by only one page (test imports don't count), it lives in `pages/<name>/components/`. If imported by multiple pages, it lives in `common/components/`.
48+
4249
## React
4350

44-
### Page / Component / Hook Rules
51+
### Pages
52+
53+
- **Smart for page-specific data** — pages use central hooks (e.g. `useClusterService`, `useSourceControl`) to fetch, prepare, and transform all data needed for downstream components.
54+
55+
### Components
4556

46-
**Components are simple by default** — they receive data via props, render it, and call callbacks. No logic at the top of a component.
57+
- **Simple by default** — they receive data via props, render it, and call callbacks. No logic at the top of a component.
58+
- **May own data when self-contained** — a component may own its own data and state when it encapsulates a self-contained capability that is not specific to any one page (e.g., forge connection, auth flows, notification subscriptions). The component becomes the single owner of that concern. Pages consume it without orchestrating its internals.
59+
- **Sub-components** — if a sub-component is only used by one parent, keep it in the parent's file, unexported. Extract to its own file only when the sub-component is used by multiple siblings.
4760

48-
**A component may own its own data and state when it encapsulates a self-contained capability that is not specific to any one page** (e.g., forge connection, auth flows, notification subscriptions). The component becomes the single owner of that concern. Pages consume it without orchestrating its internals.
61+
### Hooks
4962

50-
**Pages are smart for page-specific data**they use central hooks (e.g. `useClusterService`, `useSourceControl`) to fetch, prepare, and transform all data needed for downstream components.
63+
- **Extract logic into hooks**if a page or component has any logic (state management, data transformation, side effects), extract it into a custom hook. If the hook is only used by one component, keep it in the same file, do not export it. If the hook is reused by multiple components within one page, put it in `src/pages/<name>/hooks/`. If reused across pages, put it in `src/common/hooks/`. If there is no logic, no hook is needed.
5164

52-
**Extract logic into hooks** — if a page or component has any logic (state management, data transformation, side effects), extract it into a custom hook. If the hook is reused by multiple components, put it in a separate file: `useFunctionTable.ts`. If the hook is only used by one component, keep it in the same file, do not export it. If there is no logic, no hook is needed.
65+
### File Ordering
5366

54-
**File ordering** — within a file, put the exported component at the top, then its hook below, then helper functions at the bottom. Readers see the main thing first and can drill down.
67+
Within a file, put the exported component at the top, then its hook below, then sub-components, then helper functions at the bottom. Readers see the main thing first and can drill down.
5568

5669
### Performance
5770

@@ -60,6 +73,7 @@ Arrows mean "imports / depends on."
6073
## Architectural Guidance
6174

6275
- PatternFly components preferred over custom HTML
76+
- PatternFly styling and styling rules over custom CSS
6377
- Error handling through ErrorProvider/addError pattern
64-
- Shared utilities in `utils/`, not hand-rolled per component
78+
- Shared utilities in `common/utils/`, not hand-rolled per component
6579
- Services consumed through hooks, never imported directly

docs/TESTING.md

Lines changed: 51 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -15,23 +15,31 @@ Do NOT write all test cases first and then implement everything at once.
1515

1616
| Layer | Tool | Scope |
1717
|-------|------|-------|
18-
| Unit / Component | Jest + React Testing Library | Hooks, services, component rendering, form logic |
18+
| Unit / Component | Vitest + React Testing Library | Hooks, services, component rendering, form logic |
1919
| E2e / Feature validation | Cypress | Validate features.json entries in real browser |
2020
| API mocking | MSW (Mock Service Worker) | GitHub API + K8s API — mock everything first, real cluster later |
2121

2222
## Mock Strategy
2323

24-
Use MSW for all API mocking (GitHub + K8s). If SDK hooks require module-level mocks in unit tests (because they depend on Console runtime internals), fall back to Jest mocks — but try MSW first.
24+
MSW is the primary mocking strategy for anything that hits the network (GitHub API, K8s API, Go backend). K8s API mocking uses MSW WebSocket capability.
2525

26-
- **Start:** Mock everything via MSW (GitHub API, K8s API)
27-
- **Later:** Real cluster for e2e — SDK hooks work natively, GitHub API remains mocked
28-
- **Fallback:** If SDK hooks can't be driven by MSW alone in unit tests, use Jest module mocks
26+
`vi.mock` is only for framework and library internals that have no external service:
27+
28+
- `react-i18next` (translation hook)
29+
- `@openshift-console/dynamic-plugin-sdk` (console shell runtime components like DocumentTitle, ListPageHeader, consoleFetchJSON)
30+
- `@patternfly/react-icons` (UI library)
31+
- `react-router-dom-v5-compat` (framework routing)
32+
- `libsodium-wrappers` (WASM crypto library)
33+
34+
If it makes an HTTP or WebSocket call, mock it with MSW, not `vi.mock`.
2935

3036
## File Conventions
3137

3238
| Type | Location |
3339
|------|----------|
34-
| Unit / Component tests | `src/**/*.test.ts\|tsx` |
40+
| Component tests | `src/pages/<name>/components/*.test.ts\|tsx`, `src/common/components/*.test.ts\|tsx` |
41+
| Page tests | `src/pages/<name>/*.test.ts\|tsx` |
42+
| Service / Hook / Util tests | `src/common/**/*.test.ts\|tsx` |
3543
| E2e specs | `e2e/<feature-name>/*.test.ts` |
3644
| MSW handlers | `testing/msw/handlers.ts` |
3745

@@ -42,9 +50,28 @@ Use MSW for all API mocking (GitHub + K8s). If SDK hooks require module-level mo
4250
| Service interfaces | Unit | `FunctionService.generateFunction()` returns expected files |
4351
| React hooks | Unit | `useFunctionService()` returns service instance |
4452
| Components | Component | `CreateForm` renders all fields, validates input |
45-
| Views | Component + E2e | `FunctionListPage` shows empty state, table |
53+
| Pages | Component + E2e | `FunctionsListPage` shows empty state, table |
4654
| User flows | E2e | Create form → submit → list shows new function |
4755

56+
## Component vs. Page Tests
57+
58+
Every component gets its own exhaustive test file. Every page gets its own test file that tests the page's orchestration and integration with its components.
59+
60+
**Component tests** cover:
61+
62+
- Rendering based on props (all states and variants)
63+
- User interactions that trigger callbacks (clicks, input, form validation)
64+
- Internal state (expand/collapse, selection)
65+
66+
**Page tests** cover:
67+
68+
- Component is present on the page and wired correctly
69+
- Data flows from hooks/services to components (correct props)
70+
- User actions that trigger cross-component effects or service calls (e.g., form submit calls service, then navigates)
71+
- Page-level states: loading, error, empty
72+
73+
Overlap between component tests and page tests is expected and acceptable. They test at different levels: component tests verify the component works in isolation, page tests verify the page's orchestration logic works correctly.
74+
4875
## Testing Best Practices
4976

5077
1. **User-Centric Testing** — Test what users see and interact with.
@@ -61,50 +88,54 @@ Use MSW for all API mocking (GitHub + K8s). If SDK hooks require module-level mo
6188
- **Act:** Perform user actions
6289
- **Assert:** Verify expected state
6390

91+
6. **Scoping** — Place beforeEach, afterEach, and afterAll inside describe blocks.
92+
6493
## Mocking Patterns
6594

95+
MSW is the primary approach. `vi.mock` is rare (see Mock Strategy above).
96+
6697
Use ESM `import` at top of file. Never use `require('react')` or `React.createElement()` in mocks.
67-
Prefer `jest.mock()` for modules, `jest.fn()` for components. Keep mocks simple.
98+
Keep mocks simple.
6899

69-
**Correct patterns:**
100+
**Correct patterns (for the rare `vi.mock` cases):**
70101

71102
```typescript
72103
// Return null
73-
jest.mock('../MyComponent', () => () => null);
104+
vi.mock('../MyComponent', () => () => null);
74105

75106
// Return string
76-
jest.mock('../LoadingSpinner', () => () => 'Loading...');
107+
vi.mock('../LoadingSpinner', () => () => 'Loading...');
77108

78109
// Return children directly
79-
jest.mock('../Wrapper', () => ({ children }) => children);
110+
vi.mock('../Wrapper', () => ({ children }) => children);
80111

81-
// Track calls with jest.fn
82-
jest.mock('../ButtonBar', () => jest.fn(({ children }) => children));
112+
// Track calls with vi.fn
113+
vi.mock('../ButtonBar', () => vi.fn(({ children }) => children));
83114

84-
// Mock custom hooks
85-
jest.mock('../useCustomHook', () => ({
86-
useCustomHook: jest.fn(() => [/* mock data */]),
115+
// Mock framework hooks
116+
vi.mock('react-i18next', () => ({
117+
useTranslation: () => ({ t: (key: string) => key }),
87118
}));
88119
```
89120

90121
**Forbidden patterns:**
91122

92123
```typescript
93124
// NEVER - require() in mocks
94-
jest.mock('../Component', () => {
125+
vi.mock('../Component', () => {
95126
const React = require('react');
96127
return () => React.createElement('div');
97128
});
98129

99130
// NEVER - JSX in mocks
100-
jest.mock('../Component', () => () => <div>Mock</div>);
131+
vi.mock('../Component', () => () => <div>Mock</div>);
101132
```
102133

103134
**Clean up mocks:**
104135

105136
```typescript
106137
afterEach(() => {
107-
jest.restoreAllMocks();
138+
vi.restoreAllMocks();
108139
});
109140
```
110141

docs/WORKFLOW.md

Lines changed: 3 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,23 +2,14 @@
22

33
## Startup Sequence
44

5-
Every session, before doing any work:
6-
7-
1. `pwd` — confirm working directory
8-
2. Read `docs/claude-progress.txt` + `git log --oneline -10` — orient
9-
3. Read `docs/agent-struggles.json` — if unresolved entries exist, present to user
10-
4. Read `docs/features.json` — pick first `"passes": false` entry
11-
5. Run `./init.sh` — start dev env
12-
6. Read `.dev-env.json` — note the dev server ports (backend, plugin, console)
13-
7. Run tests — verify app is healthy
14-
8. If broken → fix first. If clean → start [Feature Development Sequence](#feature-development-sequence).
5+
Handled by the `init-session` command (`.claude/commands/init-session.md`).
156

167
## Feature Development Sequence
178

189
After [Startup Sequence](#startup-sequence), work through the picked feature:
1910

20-
1. **Plan**read `docs/ARCHITECTURE.md` + `docs/STYLEGUIDE.md` + `docs/TESTING.md`, then use `/brainstorming` to design the chosen feature from `docs/features.json`, then use `/writing-plans` to create implementation plan → `docs/plans/active/<NNN>-<type>-<short-name>.md`
21-
2. **Branch**create feature branch per [Branching](#branching) convention. Immediately push and open a **draft PR** (`gh pr create --draft`) to reserve the PR number for other contributors' branch numbering.
11+
1. **Branch**create feature branch per [Branching](#branching) convention. Immediately push and open a **draft PR** (`gh pr create --draft`) to reserve the PR number for other contributors' branch numbering.
12+
2. **Plan**read `docs/ARCHITECTURE.md` + `docs/STYLEGUIDE.md` + `docs/TESTING.md`, then use `/brainstorming` to design the chosen feature from `docs/features.json`, then use `/writing-plans` to create implementation plan → `docs/plans/active/<NNN>-<type>-<short-name>.md`
2213
3. **Implement** — using `/executing-plans` skill
2314
4. **Review** — code review using `/requesting-code-review` skill, fix found issues
2415
5. **Manual Test** — use browser automation and validate it works in the browser

docs/claude-progress.txt

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,42 @@
11
# Claude Progress Log
22
# Newest entries first. Agents: append your entry at the top after the header.
33

4+
---
5+
## 2026-05-21 | Session: Page co-location restructure (continued)
6+
Worked on: Follow-up fixes from review, test cleanup, docs
7+
Completed:
8+
- Folded LeaveModal back into EditToolbar.tsx (sub-component rule: single consumer = inline unexported)
9+
- Documented sub-component rule in ARCHITECTURE.md
10+
- Restructured ARCHITECTURE.md Page/Component/Hook rules into subsections (Pages, Components, Hooks, File Ordering)
11+
- Added hooks directory convention to ARCHITECTURE.md (common/hooks/, pages/<name>/hooks/)
12+
- Added EditToolbar tests (9 tests: render, disabled state, save success with auto-dismiss, save failure, back navigation, leave modal flow)
13+
- Moved all beforeEach/afterEach/afterAll into describe blocks across all test files
14+
- Merged duplicate beforeEach blocks in OcpClusterService.test.ts
15+
- Added scoping rule to TESTING.md (beforeEach/afterEach inside describe, vi.mock outside)
16+
- Fixed useClusterService.test.tsx afterEach scoping
17+
- Added yarn ci script to package.json (lint + test + build)
18+
- Updated init-session.md: use yarn ci, handle init.sh failure in sandbox
19+
- Fixed Monaco editor error: replaced runtime import of @patternfly/react-code-editor with import type (prevents bundling Monaco workers)
20+
- 14 suites, 121 tests passing, zero lint errors, webpack builds clean
21+
Left off: PR #31 ready for review.
22+
Blockers: Monaco "Unexpected usage" error in OCP console editor page (pre-existing, deferred to separate session)
23+
24+
---
25+
## 2026-05-20 | Session: Page co-location restructure + workflow updates
26+
Worked on: Restructure src/ into pages/ and common/, update workflow and docs
27+
Completed:
28+
- Updated .pi/prompts/init-session.md and docs/WORKFLOW.md: dropped old step 4 (pick from features.json), added Jira epic story selection flow, swapped Feature Dev steps 1 and 2 (Branch before Plan), removed duplicate startup sequence from WORKFLOW.md
29+
- Created features.json entry for SRVOCF-845, branch 031-refactor-page-colocation, draft PR #31
30+
- Moved all shared code to src/common/ (services, utils, context, UserAvatar)
31+
- Moved pages to src/pages/function-list/, function-create/, function-edit/ with components/ subdirs
32+
- Extracted EditToolbar and LeaveModal from FunctionEditPage into separate files
33+
- Updated package.json exposedModules paths
34+
- Updated ARCHITECTURE.md with co-location convention and ownership rule
35+
- Updated TESTING.md: component vs. page test rules, Vitest/MSW migration, updated file conventions
36+
- 13 suites, 112 tests passing, zero lint errors, webpack builds clean
37+
Left off: Three follow-up items for next day.
38+
Blockers: oc login not accessible from agent sandbox (permission denied on ~/.kube/config)
39+
440
---
541
## 2026-05-18 | Session: Error server and JSON error responses
642
Worked on: Errserver for failed Go compilation, JSON error responses for backend

0 commit comments

Comments
 (0)