diff --git a/backend/main.go b/backend/main.go index 8c935c0..f122c74 100644 --- a/backend/main.go +++ b/backend/main.go @@ -13,6 +13,7 @@ import ( "os" "path/filepath" "regexp" + "strings" "sync" "github.com/ory/viper" @@ -163,7 +164,7 @@ func handleFuncCreate(w http.ResponseWriter, r *http.Request) { return } - if err := generateCIWorkflow(root, cfg.Branch); err != nil { + if err := generateCIWorkflow(root, cfg.Branch, cfg.Registry); err != nil { http.Error(w, "failed to generate CI workflow: "+err.Error(), http.StatusInternalServerError) return } @@ -214,10 +215,14 @@ func handleFuncCreate(w http.ResponseWriter, r *http.Request) { } } -func generateCIWorkflow(root, branch string) error { +const ocpInternalRegistry = "image-registry.openshift-image-registry.svc:5000/" + +func generateCIWorkflow(root, branch, registry string) error { ciMu.Lock() defer ciMu.Unlock() + useRegistryLogin := !strings.HasPrefix(registry, ocpInternalRegistry) + viper.Set(ci.PlatformFlag, ci.DefaultPlatform) viper.Set(ci.PathFlag, root) viper.Set(ci.BranchFlag, branch) @@ -227,7 +232,7 @@ func generateCIWorkflow(root, branch string) error { viper.Set(ci.RegistryUserVariableNameFlag, ci.DefaultRegistryUserVariableName) viper.Set(ci.RegistryPassSecretNameFlag, ci.DefaultRegistryPassSecretName) viper.Set(ci.RegistryUrlVariableNameFlag, ci.DefaultRegistryUrlVariableName) - viper.Set(ci.UseRegistryLoginFlag, ci.DefaultUseRegistryLogin) + viper.Set(ci.UseRegistryLoginFlag, useRegistryLogin) viper.Set(ci.WorkflowDispatchFlag, ci.DefaultWorkflowDispatch) viper.Set(ci.UseRemoteBuildFlag, ci.DefaultUseRemoteBuild) viper.Set(ci.UseSelfHostedRunnerFlag, ci.DefaultUseSelfHostedRunner) diff --git a/docs/claude-progress.txt b/docs/claude-progress.txt index 0103661..26b1ee7 100644 --- a/docs/claude-progress.txt +++ b/docs/claude-progress.txt @@ -1,6 +1,26 @@ # Claude Progress Log # Newest entries first. Agents: append your entry at the top after the header. +--- +## 2026-04-13 | Session: Session-Wide GitHub PAT +Worked on: Replace build-time compiled GitHub PAT with runtime session-wide PAT (plan 004) +Completed: +- Design spec: brainstormed approaches, chose React hook + sessionStorage + PatModal, wrote spec to docs/design/ +- Implementation plan: 9-task bottom-up plan in docs/plans/active/004, executed via subagent-driven development +- `usePat` hook (`src/hooks/usePat.ts`) backed by sessionStorage key `func-console-gh-pat` +- `PatModal` component (`src/components/PatModal.tsx`) — non-dismissable PF6 modal, validates PAT via Octokit `GET /user` +- `useSourceControl` refactored to accept `pat` parameter with `useMemo` (no more module-level singleton) +- `FunctionsListPage` integrated: `usePat()` + `PatModal` gate, `useFunctionListPage(pat)` skips API when empty +- `FunctionCreatePage` integrated: `usePat()` + `PatModal` gate, passes session PAT to `pushFiles()` +- `CreateFunctionForm` PAT field removed (interface, state, validation, JSX) +- `__GITHUB_PAT__` removed from webpack DefinePlugin and `src/globals.d.ts` deleted +- i18n strings added, webpack build verified +- 9 commits, 46 tests across 11 suites, all passing +- Feature marked as passing in features.json, plan moved to completed/ +- feat: do not use login step for OCP internal registry in the generated function GH WF +Left off: Feature complete. Ready for manual testing and PR. +Blockers: None + --- ## 2026-04-02 | Session: Function Create Page Worked on: Full-stack Function Create Page feature (plan 003) diff --git a/docs/design/2026-04-13-session-wide-gh-pat-design.md b/docs/design/2026-04-13-session-wide-gh-pat-design.md new file mode 100644 index 0000000..f1eb8fd --- /dev/null +++ b/docs/design/2026-04-13-session-wide-gh-pat-design.md @@ -0,0 +1,122 @@ +# Session-Wide GitHub Personal Access Token + +**Date:** 2026-04-13 +**Status:** Draft +**Feature:** Session wide GitHub Personal Access Token (features.json) + +## Problem + +The GitHub PAT is currently injected at build time via webpack `DefinePlugin` from the `GITHUB_PAT` environment variable. This compiles the PAT as a string literal into the distributed JavaScript bundle — anyone with access to the bundle can extract it. Additionally, the Function Create form has a separate PAT text input, creating two inconsistent PAT entry points. + +## Solution + +Replace the build-time PAT with a session-wide runtime PAT. On first visit to any plugin page, a modal prompts the user to enter their PAT. The PAT is validated against the GitHub API, stored in `sessionStorage` (survives refresh, cleared on tab close), and used by all services that need GitHub access. + +## Architecture + +### PAT Storage Hook — `usePat` + +Location: `src/hooks/usePat.ts` + +A custom hook backed by `sessionStorage`. Returns `{ pat, setPat, clearPat }`. + +- Reads initial value from `sessionStorage` key `func-console-gh-pat` on mount. +- `setPat(value)` writes to `sessionStorage` and updates React state. +- `clearPat()` removes from `sessionStorage` and resets state to empty string. + +No React Context is needed. OCP dynamic plugins load each page as an independent module with no shared parent component. Since `sessionStorage` is the actual cross-page shared state, each page calls `usePat()` independently and gets the current value. + +### PAT Modal — `PatModal` + +Location: `src/components/PatModal.tsx` + +A PatternFly `Modal` component displayed when no PAT is set. + +**Props:** +- `isOpen: boolean` — controls visibility +- `onSave: (pat: string) => void` — called with validated PAT + +**Behavior:** +- Non-dismissable: no close button, no backdrop click dismiss. The user must provide a valid PAT to use the plugin. +- Contains a password `TextInput` for the PAT. +- Save button triggers validation: calls GitHub API `GET /user` via Octokit with the entered PAT. +- On success: calls `onSave` with the PAT. +- On failure: shows an inline PatternFly `Alert` with the error message. +- Save button shows a loading spinner (`isLoading`) while validating. + +### Service Layer Changes + +**`useSourceControl(pat: string): SourceControlService`** + +The hook accepts a `pat` parameter. Returns a `GithubService` instance memoized on `pat` via `useMemo`. No more module-level singleton. The `GithubService` class is unchanged — constructor still takes PAT, creates Octokit once per instance. + +**`OctokitGitHubService.pushFiles`** — unchanged. Already accepts PAT as a parameter. + +**`useGitHubService`** — unchanged. The `pushFiles` method already takes PAT as an argument. + +### Webpack Cleanup + +- Remove `DefinePlugin` entry for `__GITHUB_PAT__` from `webpack.config.ts`. +- Remove `declare const __GITHUB_PAT__: string` from `src/globals.d.ts`. + +### Page Integration + +Each page that requires a PAT renders the modal conditionally: + +```tsx +function SomePage() { + const { pat, setPat } = usePat(); + + return ( + <> + + {/* page content */} + + ); +} +``` + +**FunctionsListPage:** +- Add `usePat()` in the page component, pass `pat` to `useFunctionListPage(pat)`. +- `useFunctionListPage(pat)` passes `pat` to `useSourceControl(pat)`. +- The `useEffect` that fetches repos skips the API call when `pat` is empty (avoids a failed request while the modal is visible). +- Render `PatModal` when `pat` is empty. + +**FunctionCreatePage:** +- Add `usePat()`, pass `pat` to `gitHubService.pushFiles()` instead of `data.pat`. +- Render `PatModal` when `pat` is empty. + +**CreateFunctionForm:** +- Remove the PAT `TextInput` field. +- Remove `pat` from `CreateFunctionFormData` interface. +- Remove `pat` from form validation and local state. + +**FunctionEditPage:** Not modified — PAT integration deferred to when that page is built out. + +## Files Changed + +| File | Change | +|------|--------| +| `src/hooks/usePat.ts` | New — sessionStorage-backed PAT hook | +| `src/components/PatModal.tsx` | New — modal with PAT validation | +| `webpack.config.ts` | Remove `__GITHUB_PAT__` DefinePlugin entry | +| `src/globals.d.ts` | Remove `__GITHUB_PAT__` declaration | +| `src/services/source-control/useSourceControl.ts` | Accept `pat` param, `useMemo` instance | +| `src/components/CreateFunctionForm.tsx` | Remove PAT field, update interface | +| `src/views/FunctionCreatePage.tsx` | Use `usePat()`, render `PatModal` | +| `src/views/FunctionsListPage.tsx` | Use `usePat()`, pass to `useSourceControl`, render `PatModal` | + +## Test Strategy + +**New test suites:** + +- `usePat.test.ts` — returns empty string when sessionStorage is empty; `setPat` writes to sessionStorage and updates state; `clearPat` removes from sessionStorage and resets state. +- `PatModal.test.tsx` — renders when `isOpen` is true; does not render when false; shows error on invalid PAT (mock Octokit rejects); calls `onSave` with valid PAT (mock Octokit resolves); Save button shows spinner while validating. + +**Modified test suites:** + +- `CreateFunctionForm.test.tsx` — remove PAT field assertions, update form data expectations. +- `FunctionCreatePage.test.tsx` — remove PAT from form submit data, mock `usePat` to return a token. +- `FunctionsListPage.test.tsx` — mock `usePat`, verify `PatModal` renders when PAT is empty. + +**Mocking:** sessionStorage is available in jsdom. Octokit validation call mocked via jest module mock, consistent with existing patterns. diff --git a/docs/features.json b/docs/features.json index 7b99c50..80cb538 100644 --- a/docs/features.json +++ b/docs/features.json @@ -31,6 +31,16 @@ ], "passes": true }, + { + "category": "functional", + "description": "Session wide GitHub Personal Access Token", + "steps": [ + "If a user visits the /functions plugin and the GH PAT is not set then there will be prompt to enter it and store it into the session", + "The GH PAT it then used where needed (e.g. Function Creation page)", + "The GH PAT text box shall be removed from the Function Creation page as it should be stored in the session" + ], + "passes": true + }, { "category": "functional", "description": "Function List Page renders a table with function entries", diff --git a/docs/plans/completed/004-feat-session-wide-gh-pat.md b/docs/plans/completed/004-feat-session-wide-gh-pat.md new file mode 100644 index 0000000..6b7f898 --- /dev/null +++ b/docs/plans/completed/004-feat-session-wide-gh-pat.md @@ -0,0 +1,1107 @@ +# Session-Wide GitHub PAT Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Replace build-time compiled GitHub PAT with a runtime session-wide PAT entered via modal dialog, validated against GitHub API, and stored in `sessionStorage`. + +**Architecture:** Bottom-up — hook first (`usePat`), then modal component (`PatModal`), then service layer update (`useSourceControl`), then page integration (list page, create page), then webpack cleanup. TDD throughout. + +**Tech Stack:** React 17, TypeScript, PatternFly 6 (`Modal`, `TextInput`, `Alert`, `Button`), Octokit (`@octokit/rest`), Jest + React Testing Library. + +**Docs to read before starting:** + +- `docs/ARCHITECTURE.md` — layered architecture, dependency rules, page/component/hook rules +- `docs/STYLEGUIDE.md` — code style, naming, no `any`, no `console.log` +- `docs/TESTING.md` — TDD flow, mock patterns, forbidden patterns +- `docs/design/2026-04-13-session-wide-gh-pat-design.md` — full spec for this feature + +--- + +## File Structure + +| Action | File | Responsibility | +|--------|------|----------------| +| Create | `src/hooks/usePat.ts` | sessionStorage-backed PAT hook | +| Create | `src/hooks/usePat.test.ts` | Unit tests for usePat | +| Create | `src/components/PatModal.tsx` | Modal for PAT entry with GitHub API validation | +| Create | `src/components/PatModal.test.tsx` | Component tests for PatModal | +| Modify | `src/services/source-control/useSourceControl.ts` | Accept `pat` param, memoize instance | +| Modify | `src/services/source-control/useSourceControl.ts` test references | Existing tests mock this — update mock signatures | +| Modify | `src/views/FunctionsListPage.tsx` | Use `usePat()`, pass pat to hook, render PatModal | +| Modify | `src/views/FunctionsListPage.test.tsx` | Update mocks, add PAT modal tests | +| Modify | `src/components/CreateFunctionForm.tsx` | Remove PAT field | +| Modify | `src/components/CreateFunctionForm.test.tsx` | Remove PAT assertions | +| Modify | `src/views/FunctionCreatePage.tsx` | Use `usePat()`, render PatModal | +| Modify | `src/views/FunctionCreatePage.test.tsx` | Mock usePat, remove PAT from form flow | +| Modify | `webpack.config.ts` | Remove `__GITHUB_PAT__` DefinePlugin entry | +| Modify | `src/globals.d.ts` | Remove `__GITHUB_PAT__` declaration | +| Modify | `locales/en/plugin__console-functions-plugin.json` | Add new i18n strings | + +--- + +### Task 1: Create `usePat` hook + +**Files:** + +- Create: `src/hooks/usePat.ts` +- Create: `src/hooks/usePat.test.ts` + +- [ ] **Step 1: Write failing test — returns empty string when sessionStorage is empty** + +Create `src/hooks/usePat.test.ts`: + +```typescript +import { render, screen, act } from '@testing-library/react'; +import { usePat } from './usePat'; + +beforeEach(() => { + sessionStorage.clear(); +}); + +function TestComponent() { + const { pat } = usePat(); + return {pat}; +} + +describe('usePat', () => { + it('returns empty string when sessionStorage is empty', () => { + render(); + + expect(screen.getByTestId('pat').textContent).toBe(''); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `yarn test src/hooks/usePat.test.ts` +Expected: FAIL — module `./usePat` not found. + +- [ ] **Step 3: Write minimal implementation** + +Create `src/hooks/usePat.ts`: + +```typescript +import { useState } from 'react'; + +const SESSION_KEY = 'func-console-gh-pat'; + +export function usePat(): { pat: string; setPat: (value: string) => void; clearPat: () => void } { + const [pat, setPatState] = useState(() => sessionStorage.getItem(SESSION_KEY) ?? ''); + + const setPat = (value: string) => { + sessionStorage.setItem(SESSION_KEY, value); + setPatState(value); + }; + + const clearPat = () => { + sessionStorage.removeItem(SESSION_KEY); + setPatState(''); + }; + + return { pat, setPat, clearPat }; +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `yarn test src/hooks/usePat.test.ts` +Expected: PASS + +- [ ] **Step 5: Write failing test — reads initial value from sessionStorage** + +Add to `src/hooks/usePat.test.ts`: + +```typescript + it('reads initial value from sessionStorage', () => { + sessionStorage.setItem('func-console-gh-pat', 'ghp_stored'); + + render(); + + expect(screen.getByTestId('pat').textContent).toBe('ghp_stored'); + }); +``` + +- [ ] **Step 6: Run test to verify it passes** (implementation already handles this) + +Run: `yarn test src/hooks/usePat.test.ts` +Expected: PASS + +- [ ] **Step 7: Write failing test — setPat writes to sessionStorage and updates state** + +Add `SetTestComponent` and test to `src/hooks/usePat.test.ts`: + +```typescript +function SetTestComponent() { + const { pat, setPat } = usePat(); + return ( + <> + {pat} + + + ); +} +``` + +```typescript + it('setPat writes to sessionStorage and updates state', () => { + render(); + + act(() => { + screen.getByRole('button', { name: 'set' }).click(); + }); + + expect(screen.getByTestId('pat').textContent).toBe('ghp_new'); + expect(sessionStorage.getItem('func-console-gh-pat')).toBe('ghp_new'); + }); +``` + +- [ ] **Step 8: Run test to verify it passes** + +Run: `yarn test src/hooks/usePat.test.ts` +Expected: PASS + +- [ ] **Step 9: Write failing test — clearPat removes from sessionStorage and resets state** + +Add `ClearTestComponent` and test to `src/hooks/usePat.test.ts`: + +```typescript +function ClearTestComponent() { + const { pat, setPat, clearPat } = usePat(); + return ( + <> + {pat} + + + + ); +} +``` + +```typescript + it('clearPat removes from sessionStorage and resets state', () => { + render(); + + act(() => { + screen.getByRole('button', { name: 'set' }).click(); + }); + expect(screen.getByTestId('pat').textContent).toBe('ghp_temp'); + + act(() => { + screen.getByRole('button', { name: 'clear' }).click(); + }); + expect(screen.getByTestId('pat').textContent).toBe(''); + expect(sessionStorage.getItem('func-console-gh-pat')).toBeNull(); + }); +``` + +- [ ] **Step 10: Run test to verify it passes** + +Run: `yarn test src/hooks/usePat.test.ts` +Expected: PASS + +- [ ] **Step 11: Commit** + +```bash +git add src/hooks/usePat.ts src/hooks/usePat.test.ts +git commit -m "feat: add usePat hook backed by sessionStorage" +``` + +--- + +### Task 2: Create `PatModal` component + +**Files:** + +- Create: `src/components/PatModal.tsx` +- Create: `src/components/PatModal.test.tsx` + +- [ ] **Step 1: Write failing test — does not render when isOpen is false** + +Create `src/components/PatModal.test.tsx`: + +```typescript +import { render, screen } from '@testing-library/react'; +import { PatModal } from './PatModal'; + +jest.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +const mockGetAuthenticated = jest.fn(); + +jest.mock('@octokit/rest', () => ({ + Octokit: jest.fn().mockImplementation(() => ({ + users: { getAuthenticated: mockGetAuthenticated }, + })), +})); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +describe('PatModal', () => { + it('does not render when isOpen is false', () => { + render(); + + expect(screen.queryByText('GitHub Personal Access Token')).not.toBeInTheDocument(); + }); +}); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `yarn test src/components/PatModal.test.tsx` +Expected: FAIL — module `./PatModal` not found. + +- [ ] **Step 3: Write minimal implementation** + +Create `src/components/PatModal.tsx`: + +```typescript +import { useState } from 'react'; +import { + Alert, + Button, + Form, + FormGroup, + Modal, + ModalBody, + ModalFooter, + ModalHeader, + TextInput, +} from '@patternfly/react-core'; +import { Octokit } from '@octokit/rest'; +import { useTranslation } from 'react-i18next'; + +interface PatModalProps { + isOpen: boolean; + onSave: (pat: string) => void; +} + +export function PatModal({ isOpen, onSave }: PatModalProps) { + const { t } = useTranslation('plugin__console-functions-plugin'); + const [token, setToken] = useState(''); + const [isValidating, setIsValidating] = useState(false); + const [error, setError] = useState(''); + + if (!isOpen) return null; + + const handleSave = async () => { + setIsValidating(true); + setError(''); + + try { + const octokit = new Octokit({ auth: token }); + await octokit.users.getAuthenticated(); + onSave(token); + } catch (err) { + setError(err instanceof Error ? err.message : String(err)); + } finally { + setIsValidating(false); + } + }; + + return ( + {}} + > + + + {error && ( + + {error} + + )} +
{ e.preventDefault(); handleSave(); }}> + + setToken(val)} + /> + +
+ + + + + + ); +} +``` + +- [ ] **Step 4: Run test to verify it passes** + +Run: `yarn test src/components/PatModal.test.tsx` +Expected: PASS + +- [ ] **Step 5: Write failing test — renders modal when isOpen is true** + +Add to `src/components/PatModal.test.tsx`: + +```typescript + it('renders modal with token input when isOpen is true', () => { + render(); + + expect(screen.getByText('GitHub Personal Access Token')).toBeInTheDocument(); + expect(screen.getByLabelText(/Personal Access Token/)).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Save/ })).toBeInTheDocument(); + }); +``` + +- [ ] **Step 6: Run test to verify it passes** + +Run: `yarn test src/components/PatModal.test.tsx` +Expected: PASS + +- [ ] **Step 7: Write failing test — Save is disabled when input is empty** + +Add to `src/components/PatModal.test.tsx`: + +```typescript + it('disables Save button when input is empty', () => { + render(); + + expect(screen.getByRole('button', { name: /Save/ })).toBeDisabled(); + }); +``` + +- [ ] **Step 8: Run test to verify it passes** + +Run: `yarn test src/components/PatModal.test.tsx` +Expected: PASS + +- [ ] **Step 9: Write failing test — calls onSave with valid PAT** + +Add to `src/components/PatModal.test.tsx`: + +```typescript +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +``` + +(Update the import at the top of the file to include `waitFor`, and add `userEvent`.) + +```typescript + it('calls onSave when PAT is valid', async () => { + const user = userEvent.setup(); + const onSave = jest.fn(); + mockGetAuthenticated.mockResolvedValue({ data: { login: 'testuser' } }); + + render(); + + await user.type(screen.getByLabelText(/Personal Access Token/), 'ghp_valid'); + await user.click(screen.getByRole('button', { name: /Save/ })); + + await waitFor(() => { + expect(onSave).toHaveBeenCalledWith('ghp_valid'); + }); + }); +``` + +- [ ] **Step 10: Run test to verify it passes** + +Run: `yarn test src/components/PatModal.test.tsx` +Expected: PASS + +- [ ] **Step 11: Write failing test — shows error on invalid PAT** + +Add to `src/components/PatModal.test.tsx`: + +```typescript + it('shows error when PAT is invalid', async () => { + const user = userEvent.setup(); + const onSave = jest.fn(); + mockGetAuthenticated.mockRejectedValue(new Error('Bad credentials')); + + render(); + + await user.type(screen.getByLabelText(/Personal Access Token/), 'ghp_invalid'); + await user.click(screen.getByRole('button', { name: /Save/ })); + + await waitFor(() => { + expect(screen.getByText('Bad credentials')).toBeInTheDocument(); + }); + expect(onSave).not.toHaveBeenCalled(); + }); +``` + +- [ ] **Step 12: Run test to verify it passes** + +Run: `yarn test src/components/PatModal.test.tsx` +Expected: PASS + +- [ ] **Step 13: Commit** + +```bash +git add src/components/PatModal.tsx src/components/PatModal.test.tsx +git commit -m "feat: add PatModal component with GitHub PAT validation" +``` + +--- + +### Task 3: Update `useSourceControl` to accept PAT parameter + +**Files:** + +- Modify: `src/services/source-control/useSourceControl.ts` + +- [ ] **Step 1: Update `useSourceControl` to accept `pat` parameter** + +Replace the full content of `src/services/source-control/useSourceControl.ts`: + +```typescript +import { useMemo } from 'react'; +import { GithubService } from './GithubService'; +import { SourceControlService } from './SourceControlService'; + +export function useSourceControl(pat: string): SourceControlService { + return useMemo(() => new GithubService(pat), [pat]); +} +``` + +- [ ] **Step 2: Run all tests to verify nothing is broken** + +Run: `yarn test` +Expected: Tests that mock `useSourceControl` still pass because they mock the entire module. The mock call sites return mock objects regardless of the argument. + +- [ ] **Step 3: Commit** + +```bash +git add src/services/source-control/useSourceControl.ts +git commit -m "refactor: useSourceControl accepts pat parameter instead of build-time global" +``` + +--- + +### Task 4: Integrate `usePat` and `PatModal` into `FunctionsListPage` + +**Files:** + +- Modify: `src/views/FunctionsListPage.tsx` +- Modify: `src/views/FunctionsListPage.test.tsx` + +- [ ] **Step 1: Write failing test — renders PatModal when PAT is empty** + +Add mock for `usePat` and new test to `src/views/FunctionsListPage.test.tsx`. + +Add these mock declarations near the top with the other mocks: + +```typescript +const mockUsePat = jest.fn(); +jest.mock('../hooks/usePat', () => ({ + usePat: () => mockUsePat(), +})); + +jest.mock('../components/PatModal', () => ({ + PatModal: ({ isOpen }: { isOpen: boolean }) => + isOpen ? 'PatModal-open' : null, +})); +``` + +Add test: + +```typescript + it('renders PatModal when PAT is empty', () => { + mockUsePat.mockReturnValue({ pat: '', setPat: jest.fn(), clearPat: jest.fn() }); + mockUseSourceControl.mockReturnValue({ + listFunctionRepos: jest.fn().mockResolvedValue([]), + fetchFileContent: jest.fn(), + }); + mockUseClusterService.mockReturnValue({ deployments: [], loaded: true, error: null }); + + render( + + + , + ); + + expect(screen.getByText('PatModal-open')).toBeInTheDocument(); + }); +``` + +- [ ] **Step 2: Run test to verify it fails** + +Run: `yarn test src/views/FunctionsListPage.test.tsx` +Expected: FAIL — module `../hooks/usePat` not found or PatModal not rendered. + +- [ ] **Step 3: Update FunctionsListPage to use `usePat` and render `PatModal`** + +In `src/views/FunctionsListPage.tsx`, add imports: + +```typescript +import { usePat } from '../hooks/usePat'; +import { PatModal } from '../components/PatModal'; +``` + +Update the `FunctionsListPage` default export to use `usePat` and pass `pat` to `useFunctionListPage`: + +```typescript +export default function FunctionsListPage() { + const { t } = useTranslation('plugin__console-functions-plugin'); + const { pat, setPat } = usePat(); + const { functions, loaded, onEdit } = useFunctionListPage(pat); + + return ( + <> + + {t('Functions')} + + + {!loaded && ( + + )} + {loaded && functions.length === 0 && } + {loaded && functions.length > 0 && ( + <> + + {t( + 'Serverless functions in your repository and deployed to your cluster. Manage lifecycle, monitor status, and scale on demand.', + )} + + + + + + + )} + + + ); +} +``` + +Update `useFunctionListPage` to accept `pat` and pass it to `useSourceControl`: + +```typescript +function useFunctionListPage(pat: string): { + functions: FunctionTableItem[]; + loaded: boolean; + onEdit: (name: string) => void; +} { + const sourceControl = useSourceControl(pat); + const { deployments, loaded: clusterLoaded } = useClusterService(); + const navigate = useNavigate(); + + const [functionItems, setFunctionItems] = useState([]); + const [reposLoaded, setReposLoaded] = useState(false); + + useEffect(() => { + if (!pat) { + setReposLoaded(true); + return; + } + + let ignore = false; + + async function loadFunctionTableItems() { + const repos = await sourceControl.listFunctionRepos(); + const items = await Promise.all( + repos.map(async (repo) => { + const funcYaml = await sourceControl.fetchFileContent(repo, 'func.yaml'); + const { namespace, runtime } = parseNamespaceAndRuntime(funcYaml, repo.name); + return newItem(repo.name, namespace, runtime); + }), + ); + if (!ignore) { + setFunctionItems(items); + setReposLoaded(true); + } + } + + loadFunctionTableItems().catch(() => { + if (!ignore) { + setReposLoaded(true); + } + }); + return () => { + ignore = true; + }; + }, [sourceControl, pat]); + + const functions = useMemo( + () => + functionItems.map((item) => { + const deployment = deployments.find( + (d) => d.metadata?.labels?.['function.knative.dev/name'] === item.name, + ); + return deployment ? enrichItem(item, deployment) : item; + }), + [functionItems, deployments], + ); + + const loaded = reposLoaded && clusterLoaded; + + const onEdit = (name: string) => navigate(`/functions/edit/${name}`); + return { functions, loaded, onEdit }; +} +``` + +- [ ] **Step 4: Update existing tests to mock `usePat` with a valid token** + +In `src/views/FunctionsListPage.test.tsx`, add to each existing test's setup (inside each `it` block, before `render`): + +```typescript + mockUsePat.mockReturnValue({ pat: 'ghp_test', setPat: jest.fn(), clearPat: jest.fn() }); +``` + +Also update the `useSourceControl` mock to accept a parameter (the mock already ignores it since it returns `mockUseSourceControl()`, so this requires no functional change to the mock definition). + +- [ ] **Step 5: Run all tests to verify they pass** + +Run: `yarn test src/views/FunctionsListPage.test.tsx` +Expected: PASS — all existing tests pass plus the new PatModal test. + +- [ ] **Step 6: Write failing test — does not render PatModal when PAT is set** + +Add to `src/views/FunctionsListPage.test.tsx`: + +```typescript + it('does not render PatModal when PAT is set', () => { + mockUsePat.mockReturnValue({ pat: 'ghp_valid', setPat: jest.fn(), clearPat: jest.fn() }); + mockUseSourceControl.mockReturnValue({ + listFunctionRepos: jest.fn().mockResolvedValue([]), + fetchFileContent: jest.fn(), + }); + mockUseClusterService.mockReturnValue({ deployments: [], loaded: true, error: null }); + + render( + + + , + ); + + expect(screen.queryByText('PatModal-open')).not.toBeInTheDocument(); + }); +``` + +- [ ] **Step 7: Run test to verify it passes** + +Run: `yarn test src/views/FunctionsListPage.test.tsx` +Expected: PASS + +- [ ] **Step 8: Commit** + +```bash +git add src/views/FunctionsListPage.tsx src/views/FunctionsListPage.test.tsx +git commit -m "feat: integrate session PAT into FunctionsListPage with PatModal gate" +``` + +--- + +### Task 5: Remove PAT field from `CreateFunctionForm` + +**Files:** + +- Modify: `src/components/CreateFunctionForm.tsx` +- Modify: `src/components/CreateFunctionForm.test.tsx` + +- [ ] **Step 1: Update test — remove PAT field assertion from 'renders all form fields'** + +In `src/components/CreateFunctionForm.test.tsx`, remove this line from the 'renders all form fields' test: + +```typescript + expect(screen.getByLabelText(/Personal Access Token/)).toBeInTheDocument(); +``` + +- [ ] **Step 2: Update test — remove PAT from 'calls onSubmit with form data'** + +In `src/components/CreateFunctionForm.test.tsx`, in the 'calls onSubmit with form data when form is filled and Create is clicked' test: + +Remove: + +```typescript + await user.type(screen.getByLabelText(/Personal Access Token/), 'ghp_token'); +``` + +Update the `toHaveBeenCalledWith` expectation — remove `pat: 'ghp_token'`: + +```typescript + expect(onSubmit).toHaveBeenCalledWith({ + owner: 'testuser', + repo: 'my-repo', + branch: 'main', + name: 'my-func', + runtime: 'node', + registry: 'quay.io/test', + namespace: 'default', + }); +``` + +- [ ] **Step 3: Run tests to verify they fail** (PAT field still exists in component) + +Run: `yarn test src/components/CreateFunctionForm.test.tsx` +Expected: FAIL — the onSubmit assertion fails because the component still sends `pat`. + +- [ ] **Step 4: Remove PAT from `CreateFunctionForm` component** + +In `src/components/CreateFunctionForm.tsx`: + +Remove `pat` from the `CreateFunctionFormData` interface: + +```typescript +export interface CreateFunctionFormData { + owner: string; + repo: string; + branch: string; + name: string; + runtime: FunctionRuntime; + registry: string; + namespace: string; +} +``` + +Remove the PAT state line: + +```typescript + const [pat, setPat] = useState(''); +``` + +Update validation — remove `pat` from the condition: + +```typescript + const isValid = owner && repo && branch && name && registry && namespace; +``` + +Update handleSubmit — remove `pat` from the data object: + +```typescript + const handleSubmit = () => { + onSubmit({ owner, repo, branch, name, runtime, registry, namespace }); + }; +``` + +Remove the PAT FormGroup JSX block (lines 84-92 in the original file): + +```tsx + + setPat(val)} + /> + +``` + +- [ ] **Step 5: Run tests to verify they pass** + +Run: `yarn test src/components/CreateFunctionForm.test.tsx` +Expected: PASS + +- [ ] **Step 6: Commit** + +```bash +git add src/components/CreateFunctionForm.tsx src/components/CreateFunctionForm.test.tsx +git commit -m "refactor: remove PAT field from CreateFunctionForm" +``` + +--- + +### Task 6: Integrate `usePat` and `PatModal` into `FunctionCreatePage` + +**Files:** + +- Modify: `src/views/FunctionCreatePage.tsx` +- Modify: `src/views/FunctionCreatePage.test.tsx` + +- [ ] **Step 1: Update test — add usePat mock and remove PAT from form fill** + +In `src/views/FunctionCreatePage.test.tsx`, add these mocks near the top with the other mocks: + +```typescript +const mockUsePat = jest.fn(); +jest.mock('../hooks/usePat', () => ({ + usePat: () => mockUsePat(), +})); + +jest.mock('../components/PatModal', () => ({ + PatModal: ({ isOpen }: { isOpen: boolean }) => + isOpen ? 'PatModal-open' : null, +})); +``` + +Update the `fillForm` helper — remove the PAT line: + +```typescript +const fillForm = async (user: ReturnType) => { + await user.type(screen.getByRole('textbox', { name: /Owner/ }), 'testuser'); + await user.type(screen.getByRole('textbox', { name: /Repository/ }), 'my-repo'); + await user.type(screen.getByRole('textbox', { name: /Branch/ }), 'main'); + await user.type(screen.getByRole('textbox', { name: /^Name$/ }), 'my-func'); + await user.type(screen.getByRole('textbox', { name: /Registry/ }), 'quay.io/test'); + await user.type(screen.getByRole('textbox', { name: /Namespace/ }), 'default'); +}; +``` + +Add `mockUsePat` setup to each existing test (before `render`): + +```typescript + mockUsePat.mockReturnValue({ pat: 'ghp_session', setPat: jest.fn(), clearPat: jest.fn() }); +``` + +Update the `pushFiles` assertion — the PAT should now be `'ghp_session'` (from usePat), not `'ghp_token'` (from form): + +```typescript + await waitFor(() => { + expect(mockPushFiles).toHaveBeenCalledWith( + { owner: 'testuser', repo: 'my-repo', branch: 'main' }, + 'ghp_session', + files, + 'Initialize Knative function project', + ); + }); +``` + +- [ ] **Step 2: Run tests to verify they fail** + +Run: `yarn test src/views/FunctionCreatePage.test.tsx` +Expected: FAIL — `FunctionCreatePage` doesn't use `usePat` yet, so pushFiles still receives the old PAT. + +- [ ] **Step 3: Update FunctionCreatePage to use `usePat` and `PatModal`** + +Replace `src/views/FunctionCreatePage.tsx`: + +```typescript +import { useState } from 'react'; +import { DocumentTitle, ListPageHeader } from '@openshift-console/dynamic-plugin-sdk'; +import { Alert, PageSection } from '@patternfly/react-core'; +import { useTranslation } from 'react-i18next'; +import { useNavigate } from 'react-router-dom-v5-compat'; +import { CreateFunctionForm, CreateFunctionFormData } from '../components/CreateFunctionForm'; +import { useFunctionService } from '../services/function/useFunctionService'; +import { useGitHubService } from '../services/github/useGitHubService'; +import { usePat } from '../hooks/usePat'; +import { PatModal } from '../components/PatModal'; + +export default function FunctionCreatePage() { + const { t } = useTranslation('plugin__console-functions-plugin'); + const navigate = useNavigate(); + const functionService = useFunctionService(); + const gitHubService = useGitHubService(); + const { pat, setPat } = usePat(); + + const [isSubmitting, setIsSubmitting] = useState(false); + const [error, setError] = useState(null); + + const handleSubmit = async (data: CreateFunctionFormData) => { + setIsSubmitting(true); + setError(null); + + try { + const files = await functionService.generateFunction({ + name: data.name, + runtime: data.runtime, + registry: data.registry, + namespace: data.namespace, + branch: data.branch, + }); + + await gitHubService.pushFiles( + { owner: data.owner, repo: data.repo, branch: data.branch }, + pat, + files, + 'Initialize Knative function project', + ); + + navigate('/functions'); + } catch (err) { + setError(err instanceof Error ? err.message : String(err)); + } finally { + setIsSubmitting(false); + } + }; + + const handleCancel = () => { + navigate('/functions'); + }; + + return ( + <> + + {t('Create function')} + + + {error && ( + + {error} + + )} + + + + ); +} +``` + +- [ ] **Step 4: Run tests to verify they pass** + +Run: `yarn test src/views/FunctionCreatePage.test.tsx` +Expected: PASS + +- [ ] **Step 5: Write failing test — renders PatModal when PAT is empty** + +Add to `src/views/FunctionCreatePage.test.tsx`: + +```typescript + it('renders PatModal when PAT is empty', () => { + mockUsePat.mockReturnValue({ pat: '', setPat: jest.fn(), clearPat: jest.fn() }); + + render( + + + , + ); + + expect(screen.getByText('PatModal-open')).toBeInTheDocument(); + }); +``` + +- [ ] **Step 6: Run test to verify it passes** + +Run: `yarn test src/views/FunctionCreatePage.test.tsx` +Expected: PASS + +- [ ] **Step 7: Commit** + +```bash +git add src/views/FunctionCreatePage.tsx src/views/FunctionCreatePage.test.tsx +git commit -m "feat: integrate session PAT into FunctionCreatePage with PatModal gate" +``` + +--- + +### Task 7: Remove `__GITHUB_PAT__` from webpack and globals + +**Files:** + +- Modify: `webpack.config.ts` +- Modify: `src/globals.d.ts` + +- [ ] **Step 1: Remove DefinePlugin entry from `webpack.config.ts`** + +In `webpack.config.ts`, change the plugins array. Remove the `__GITHUB_PAT__` entry from `DefinePlugin`. If `DefinePlugin` has no other entries, remove the entire `DefinePlugin` usage and its import. + +Current code: + +```typescript + plugins: [ + new DefinePlugin({ + __GITHUB_PAT__: JSON.stringify(process.env.GITHUB_PAT || ''), + }), + new ConsoleRemotePlugin(), +``` + +Replace with: + +```typescript + plugins: [ + new ConsoleRemotePlugin(), +``` + +Also remove `DefinePlugin` from the webpack import. Change: + +```typescript +import { Configuration as WebpackConfiguration, DefinePlugin } from 'webpack'; +``` + +To: + +```typescript +import { Configuration as WebpackConfiguration } from 'webpack'; +``` + +- [ ] **Step 2: Remove `__GITHUB_PAT__` declaration from `src/globals.d.ts`** + +Delete the file `src/globals.d.ts` — it only contains the `__GITHUB_PAT__` declaration and nothing else. + +- [ ] **Step 3: Run all tests to verify nothing is broken** + +Run: `yarn test` +Expected: PASS — all tests pass. No code references `__GITHUB_PAT__` anymore. + +- [ ] **Step 4: Commit** + +```bash +git add webpack.config.ts src/globals.d.ts +git commit -m "chore: remove build-time __GITHUB_PAT__ from webpack DefinePlugin and globals" +``` + +--- + +### Task 8: Add i18n strings and run final verification + +**Files:** + +- Modify: `locales/en/plugin__console-functions-plugin.json` + +- [ ] **Step 1: Add new i18n keys** + +Add these keys to `locales/en/plugin__console-functions-plugin.json` (in alphabetical order): + +``` +"GitHub Personal Access Token": "GitHub Personal Access Token", +"Invalid token": "Invalid token", +"Save": "Save", +``` + +Note: `"Personal Access Token"` already exists in the locale file. + +- [ ] **Step 2: Run full test suite** + +Run: `yarn test` +Expected: PASS — all tests across all suites pass. + +- [ ] **Step 3: Verify webpack build succeeds** + +Run: `yarn build-dev` +Expected: Build completes without errors. + +- [ ] **Step 4: Commit** + +```bash +git add locales/en/plugin__console-functions-plugin.json +git commit -m "chore: add i18n strings for PatModal" +``` + +--- + +### Task 9: Update `features.json` + +**Files:** + +- Modify: `docs/features.json` + +- [ ] **Step 1: Mark the feature as passing** + +In `docs/features.json`, find the "Session wide GitHub Personal Access Token" entry and set `"passes": true`. + +- [ ] **Step 2: Commit** + +```bash +git add docs/features.json +git commit -m "chore: mark session-wide GH PAT feature as passing" +``` diff --git a/locales/en/plugin__console-functions-plugin.json b/locales/en/plugin__console-functions-plugin.json index a116e80..728197f 100644 --- a/locales/en/plugin__console-functions-plugin.json +++ b/locales/en/plugin__console-functions-plugin.json @@ -11,7 +11,9 @@ "Error creating function": "Error creating function", "Function Settings": "Function Settings", "Functions": "Functions", + "GitHub Personal Access Token": "GitHub Personal Access Token", "GitHub Settings": "GitHub Settings", + "Invalid token": "Invalid token", "Language": "Language", "Loading": "Loading", "Name": "Name", @@ -24,6 +26,7 @@ "Repository": "Repository", "Runtime": "Runtime", "Serverless functions in your repository and deployed to your cluster. Manage lifecycle, monitor status, and scale on demand.": "Serverless functions in your repository and deployed to your cluster. Manage lifecycle, monitor status, and scale on demand.", + "Save": "Save", "Status": "Status", "URL": "URL", "Undeploy": "Undeploy" diff --git a/src/components/CreateFunctionForm.test.tsx b/src/components/CreateFunctionForm.test.tsx index 19ff14d..b45b85c 100644 --- a/src/components/CreateFunctionForm.test.tsx +++ b/src/components/CreateFunctionForm.test.tsx @@ -27,7 +27,6 @@ describe('CreateFunctionForm', () => { expect(screen.getByRole('textbox', { name: /Owner/ })).toBeInTheDocument(); expect(screen.getByRole('textbox', { name: /Repository/ })).toBeInTheDocument(); expect(screen.getByRole('textbox', { name: /Branch/ })).toBeInTheDocument(); - expect(screen.getByLabelText(/Personal Access Token/)).toBeInTheDocument(); expect(screen.getByRole('textbox', { name: /^Name$/ })).toBeInTheDocument(); expect(screen.getByRole('combobox', { name: /Language/ })).toBeInTheDocument(); expect(screen.getByRole('textbox', { name: /Registry/ })).toBeInTheDocument(); @@ -80,7 +79,6 @@ describe('CreateFunctionForm', () => { await user.type(screen.getByRole('textbox', { name: /Owner/ }), 'testuser'); await user.type(screen.getByRole('textbox', { name: /Repository/ }), 'my-repo'); await user.type(screen.getByRole('textbox', { name: /Branch/ }), 'main'); - await user.type(screen.getByLabelText(/Personal Access Token/), 'ghp_token'); await user.type(screen.getByRole('textbox', { name: /^Name$/ }), 'my-func'); await user.type(screen.getByRole('textbox', { name: /Registry/ }), 'quay.io/test'); await user.type(screen.getByRole('textbox', { name: /Namespace/ }), 'default'); @@ -91,7 +89,6 @@ describe('CreateFunctionForm', () => { owner: 'testuser', repo: 'my-repo', branch: 'main', - pat: 'ghp_token', name: 'my-func', runtime: 'node', registry: 'quay.io/test', diff --git a/src/components/CreateFunctionForm.tsx b/src/components/CreateFunctionForm.tsx index 5a143b1..3074748 100644 --- a/src/components/CreateFunctionForm.tsx +++ b/src/components/CreateFunctionForm.tsx @@ -16,7 +16,6 @@ export interface CreateFunctionFormData { owner: string; repo: string; branch: string; - pat: string; name: string; runtime: FunctionRuntime; registry: string; @@ -42,16 +41,15 @@ export function CreateFunctionForm({ onSubmit, onCancel, isSubmitting }: CreateF const [owner, setOwner] = useState(''); const [repo, setRepo] = useState(''); const [branch, setBranch] = useState(''); - const [pat, setPat] = useState(''); const [name, setName] = useState(''); const [runtime, setRuntime] = useState('node'); const [registry, setRegistry] = useState(''); const [namespace, setNamespace] = useState(''); - const isValid = owner && repo && branch && pat && name && registry && namespace; + const isValid = owner && repo && branch && name && registry && namespace; const handleSubmit = () => { - onSubmit({ owner, repo, branch, pat, name, runtime, registry, namespace }); + onSubmit({ owner, repo, branch, name, runtime, registry, namespace }); }; return ( @@ -81,15 +79,6 @@ export function CreateFunctionForm({ onSubmit, onCancel, isSubmitting }: CreateF onChange={(_e, val) => setBranch(val)} /> - - setPat(val)} - /> - diff --git a/src/components/PatModal.test.tsx b/src/components/PatModal.test.tsx new file mode 100644 index 0000000..4785ac4 --- /dev/null +++ b/src/components/PatModal.test.tsx @@ -0,0 +1,74 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { PatModal } from './PatModal'; + +jest.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +const mockGetAuthenticated = jest.fn(); + +jest.mock('@octokit/rest', () => ({ + Octokit: jest.fn().mockImplementation(() => ({ + users: { getAuthenticated: mockGetAuthenticated }, + })), +})); + +afterEach(() => { + jest.restoreAllMocks(); +}); + +describe('PatModal', () => { + it('does not render when isOpen is false', () => { + render(); + + expect(screen.queryByText('GitHub Personal Access Token')).not.toBeInTheDocument(); + }); + + it('renders modal with token input when isOpen is true', () => { + render(); + + expect(screen.getByText('GitHub Personal Access Token')).toBeInTheDocument(); + expect(screen.getByText('Personal Access Token')).toBeInTheDocument(); + expect(screen.getByRole('button', { name: /Save/ })).toBeInTheDocument(); + }); + + it('disables Save button when input is empty', () => { + render(); + + expect(screen.getByRole('button', { name: /Save/ })).toBeDisabled(); + }); + + it('calls onSave when PAT is valid', async () => { + const user = userEvent.setup(); + const onSave = jest.fn(); + mockGetAuthenticated.mockResolvedValue({ data: { login: 'testuser' } }); + + render(); + + const input = document.getElementById('pat-input') as HTMLInputElement; + await user.type(input, 'ghp_valid'); + await user.click(screen.getByRole('button', { name: /Save/ })); + + await waitFor(() => { + expect(onSave).toHaveBeenCalledWith('ghp_valid'); + }); + }); + + it('shows error when PAT is invalid', async () => { + const user = userEvent.setup(); + const onSave = jest.fn(); + mockGetAuthenticated.mockRejectedValue(new Error('Bad credentials')); + + render(); + + const input = document.getElementById('pat-input') as HTMLInputElement; + await user.type(input, 'ghp_invalid'); + await user.click(screen.getByRole('button', { name: /Save/ })); + + await waitFor(() => { + expect(screen.getByText('Bad credentials')).toBeInTheDocument(); + }); + expect(onSave).not.toHaveBeenCalled(); + }); +}); diff --git a/src/components/PatModal.tsx b/src/components/PatModal.tsx new file mode 100644 index 0000000..0734cac --- /dev/null +++ b/src/components/PatModal.tsx @@ -0,0 +1,81 @@ +import { useState } from 'react'; +import { + Alert, + Button, + Form, + FormGroup, + Modal, + ModalBody, + ModalFooter, + ModalHeader, + TextInput, +} from '@patternfly/react-core'; +import { Octokit } from '@octokit/rest'; +import { useTranslation } from 'react-i18next'; + +interface PatModalProps { + isOpen: boolean; + onSave: (pat: string) => void; +} + +export function PatModal({ isOpen, onSave }: PatModalProps) { + const { t } = useTranslation('plugin__console-functions-plugin'); + const [token, setToken] = useState(''); + const [isValidating, setIsValidating] = useState(false); + const [error, setError] = useState(''); + + if (!isOpen) return null; + + const handleSave = async () => { + setIsValidating(true); + setError(''); + + try { + const octokit = new Octokit({ auth: token }); + await octokit.users.getAuthenticated(); + onSave(token); + } catch (err) { + setError(err instanceof Error ? err.message : String(err)); + } finally { + setIsValidating(false); + } + }; + + return ( + + + + {error && ( + + {error} + + )} +
{ e.preventDefault(); handleSave(); }}> + + setToken(val)} + /> + +
+ + + + + + ); +} diff --git a/src/globals.d.ts b/src/globals.d.ts deleted file mode 100644 index aac78e0..0000000 --- a/src/globals.d.ts +++ /dev/null @@ -1 +0,0 @@ -declare const __GITHUB_PAT__: string; diff --git a/src/hooks/usePat.test.tsx b/src/hooks/usePat.test.tsx new file mode 100644 index 0000000..48d0221 --- /dev/null +++ b/src/hooks/usePat.test.tsx @@ -0,0 +1,74 @@ +import { render, screen, act } from '@testing-library/react'; +import { usePat } from './usePat'; + +beforeEach(() => { + sessionStorage.clear(); +}); + +function TestComponent() { + const { pat } = usePat(); + return {pat}; +} + +function SetTestComponent() { + const { pat, setPat } = usePat(); + return ( + <> + {pat} + + + ); +} + +function ClearTestComponent() { + const { pat, setPat, clearPat } = usePat(); + return ( + <> + {pat} + + + + ); +} + +describe('usePat', () => { + it('returns empty string when sessionStorage is empty', () => { + render(); + + expect(screen.getByTestId('pat').textContent).toBe(''); + }); + + it('setPat writes to sessionStorage and updates state', () => { + render(); + + act(() => { + screen.getByRole('button', { name: 'set' }).click(); + }); + + expect(screen.getByTestId('pat').textContent).toBe('ghp_new'); + expect(sessionStorage.getItem('func-console-gh-pat')).toBe('ghp_new'); + }); + + it('reads initial value from sessionStorage', () => { + sessionStorage.setItem('func-console-gh-pat', 'ghp_stored'); + + render(); + + expect(screen.getByTestId('pat').textContent).toBe('ghp_stored'); + }); + + it('clearPat removes from sessionStorage and resets state', () => { + render(); + + act(() => { + screen.getByRole('button', { name: 'set' }).click(); + }); + expect(screen.getByTestId('pat').textContent).toBe('ghp_temp'); + + act(() => { + screen.getByRole('button', { name: 'clear' }).click(); + }); + expect(screen.getByTestId('pat').textContent).toBe(''); + expect(sessionStorage.getItem('func-console-gh-pat')).toBeNull(); + }); +}); diff --git a/src/hooks/usePat.ts b/src/hooks/usePat.ts new file mode 100644 index 0000000..b075cff --- /dev/null +++ b/src/hooks/usePat.ts @@ -0,0 +1,19 @@ +import { useState } from 'react'; + +const SESSION_KEY = 'func-console-gh-pat'; + +export function usePat(): { pat: string; setPat: (value: string) => void; clearPat: () => void } { + const [pat, setPatState] = useState(() => sessionStorage.getItem(SESSION_KEY) ?? ''); + + const setPat = (value: string) => { + sessionStorage.setItem(SESSION_KEY, value); + setPatState(value); + }; + + const clearPat = () => { + sessionStorage.removeItem(SESSION_KEY); + setPatState(''); + }; + + return { pat, setPat, clearPat }; +} diff --git a/src/services/source-control/useSourceControl.ts b/src/services/source-control/useSourceControl.ts index 7f204ff..4eaa4f7 100644 --- a/src/services/source-control/useSourceControl.ts +++ b/src/services/source-control/useSourceControl.ts @@ -1,12 +1,7 @@ +import { useMemo } from 'react'; import { GithubService } from './GithubService'; import { SourceControlService } from './SourceControlService'; -// PAT injected via webpack DefinePlugin from GITHUB_PAT env variable. -// For dev/testing: export GITHUB_PAT=ghp_... before running yarn start. -// DO NOT hardcode a real PAT here — this file is committed. -const pat = typeof __GITHUB_PAT__ !== 'undefined' ? __GITHUB_PAT__ : ''; -const instance = new GithubService(pat); - -export function useSourceControl(): SourceControlService { - return instance; +export function useSourceControl(pat: string): SourceControlService { + return useMemo(() => new GithubService(pat), [pat]); } diff --git a/src/views/FunctionCreatePage.test.tsx b/src/views/FunctionCreatePage.test.tsx index 2d0e32d..0c8dd23 100644 --- a/src/views/FunctionCreatePage.test.tsx +++ b/src/views/FunctionCreatePage.test.tsx @@ -6,6 +6,7 @@ import FunctionCreatePage from './FunctionCreatePage'; const mockGenerateFunction = jest.fn(); const mockPushFiles = jest.fn(); const mockNavigate = jest.fn(); +const mockUsePat = jest.fn(); jest.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }), @@ -28,6 +29,15 @@ jest.mock('react-router-dom-v5-compat', () => ({ useNavigate: () => mockNavigate, })); +jest.mock('../hooks/usePat', () => ({ + usePat: () => mockUsePat(), +})); + +jest.mock('../components/PatModal', () => ({ + PatModal: ({ isOpen }: { isOpen: boolean }) => + isOpen ?
PatModal-open
: null, +})); + afterEach(() => { jest.clearAllMocks(); }); @@ -36,7 +46,6 @@ const fillForm = async (user: ReturnType) => { await user.type(screen.getByRole('textbox', { name: /Owner/ }), 'testuser'); await user.type(screen.getByRole('textbox', { name: /Repository/ }), 'my-repo'); await user.type(screen.getByRole('textbox', { name: /Branch/ }), 'main'); - await user.type(screen.getByLabelText(/Personal Access Token/), 'ghp_token'); await user.type(screen.getByRole('textbox', { name: /^Name$/ }), 'my-func'); await user.type(screen.getByRole('textbox', { name: /Registry/ }), 'quay.io/test'); await user.type(screen.getByRole('textbox', { name: /Namespace/ }), 'default'); @@ -44,6 +53,8 @@ const fillForm = async (user: ReturnType) => { describe('FunctionCreatePage', () => { it('renders CreateFunctionForm', () => { + mockUsePat.mockReturnValue({ pat: 'ghp_session', setPat: jest.fn(), clearPat: jest.fn() }); + render( @@ -59,6 +70,7 @@ describe('FunctionCreatePage', () => { const files = [{ path: 'func.yaml', mode: '100644', content: 'name: f', type: 'blob' }]; mockGenerateFunction.mockResolvedValue(files); mockPushFiles.mockResolvedValue(undefined); + mockUsePat.mockReturnValue({ pat: 'ghp_session', setPat: jest.fn(), clearPat: jest.fn() }); render( @@ -82,7 +94,7 @@ describe('FunctionCreatePage', () => { await waitFor(() => { expect(mockPushFiles).toHaveBeenCalledWith( { owner: 'testuser', repo: 'my-repo', branch: 'main' }, - 'ghp_token', + 'ghp_session', files, 'Initialize Knative function project', ); @@ -96,6 +108,7 @@ describe('FunctionCreatePage', () => { it('shows an alert on error', async () => { const user = userEvent.setup(); mockGenerateFunction.mockRejectedValue(new Error('Backend error')); + mockUsePat.mockReturnValue({ pat: 'ghp_session', setPat: jest.fn(), clearPat: jest.fn() }); render( @@ -110,4 +123,16 @@ describe('FunctionCreatePage', () => { expect(screen.getByText('Backend error')).toBeInTheDocument(); }); }); + + it('renders PatModal when PAT is empty', () => { + mockUsePat.mockReturnValue({ pat: '', setPat: jest.fn(), clearPat: jest.fn() }); + + render( + + + , + ); + + expect(screen.getByText('PatModal-open')).toBeInTheDocument(); + }); }); diff --git a/src/views/FunctionCreatePage.tsx b/src/views/FunctionCreatePage.tsx index aa7348d..b5cf846 100644 --- a/src/views/FunctionCreatePage.tsx +++ b/src/views/FunctionCreatePage.tsx @@ -6,12 +6,15 @@ import { useNavigate } from 'react-router-dom-v5-compat'; import { CreateFunctionForm, CreateFunctionFormData } from '../components/CreateFunctionForm'; import { useFunctionService } from '../services/function/useFunctionService'; import { useGitHubService } from '../services/github/useGitHubService'; +import { usePat } from '../hooks/usePat'; +import { PatModal } from '../components/PatModal'; export default function FunctionCreatePage() { const { t } = useTranslation('plugin__console-functions-plugin'); const navigate = useNavigate(); const functionService = useFunctionService(); const gitHubService = useGitHubService(); + const { pat, setPat } = usePat(); const [isSubmitting, setIsSubmitting] = useState(false); const [error, setError] = useState(null); @@ -31,7 +34,7 @@ export default function FunctionCreatePage() { await gitHubService.pushFiles( { owner: data.owner, repo: data.repo, branch: data.branch }, - data.pat, + pat, files, 'Initialize Knative function project', ); @@ -50,6 +53,7 @@ export default function FunctionCreatePage() { return ( <> + {t('Create function')} diff --git a/src/views/FunctionsListPage.test.tsx b/src/views/FunctionsListPage.test.tsx index 4384da2..37e4264 100644 --- a/src/views/FunctionsListPage.test.tsx +++ b/src/views/FunctionsListPage.test.tsx @@ -18,7 +18,7 @@ jest.mock('@openshift-console/dynamic-plugin-sdk', () => ({ const mockUseSourceControl = jest.fn(); jest.mock('../services/source-control/useSourceControl', () => ({ - useSourceControl: () => mockUseSourceControl(), + useSourceControl: (pat: string) => mockUseSourceControl(pat), })); const mockUseClusterService = jest.fn(); @@ -26,6 +26,16 @@ jest.mock('../services/cluster/useClusterService', () => ({ useClusterService: () => mockUseClusterService(), })); +const mockUsePat = jest.fn(); +jest.mock('../hooks/usePat', () => ({ + usePat: () => mockUsePat(), +})); + +jest.mock('../components/PatModal', () => ({ + PatModal: ({ isOpen }: { isOpen: boolean }) => + isOpen ?
PatModal-open
: null, +})); + jest.mock('../components/FunctionTable', () => ({ FunctionTable: ({ functions }: { functions: { name: string }[] }) => functions.map((f) => f.name).join(','), @@ -37,6 +47,7 @@ afterEach(() => { describe('FunctionsListPage', () => { it('renders a spinner while loading', () => { + mockUsePat.mockReturnValue({ pat: 'ghp_test', setPat: jest.fn(), clearPat: jest.fn() }); mockUseSourceControl.mockReturnValue({ listFunctionRepos: jest.fn().mockResolvedValue([]), fetchFileContent: jest.fn(), @@ -53,6 +64,7 @@ describe('FunctionsListPage', () => { }); it('renders the empty state when loaded with no functions', async () => { + mockUsePat.mockReturnValue({ pat: 'ghp_test', setPat: jest.fn(), clearPat: jest.fn() }); mockUseSourceControl.mockReturnValue({ listFunctionRepos: jest.fn().mockResolvedValue([]), fetchFileContent: jest.fn(), @@ -69,6 +81,7 @@ describe('FunctionsListPage', () => { }); it('renders table when functions are loaded', async () => { + mockUsePat.mockReturnValue({ pat: 'ghp_test', setPat: jest.fn(), clearPat: jest.fn() }); mockUseSourceControl.mockReturnValue({ listFunctionRepos: jest.fn().mockResolvedValue([ { @@ -110,6 +123,7 @@ describe('FunctionsListPage', () => { }); it('shows NotDeployed status for repos without cluster deployment', async () => { + mockUsePat.mockReturnValue({ pat: 'ghp_test', setPat: jest.fn(), clearPat: jest.fn() }); mockUseSourceControl.mockReturnValue({ listFunctionRepos: jest.fn().mockResolvedValue([ { @@ -135,6 +149,7 @@ describe('FunctionsListPage', () => { }); it('renders empty state when GitHub API fails', async () => { + mockUsePat.mockReturnValue({ pat: 'ghp_test', setPat: jest.fn(), clearPat: jest.fn() }); mockUseSourceControl.mockReturnValue({ listFunctionRepos: jest.fn().mockRejectedValue(new Error('Requires authentication')), fetchFileContent: jest.fn(), @@ -149,4 +164,38 @@ describe('FunctionsListPage', () => { expect(await screen.findByRole('heading', { name: 'No functions found' })).toBeInTheDocument(); }); + + it('renders PatModal when PAT is empty', () => { + mockUsePat.mockReturnValue({ pat: '', setPat: jest.fn(), clearPat: jest.fn() }); + mockUseSourceControl.mockReturnValue({ + listFunctionRepos: jest.fn().mockResolvedValue([]), + fetchFileContent: jest.fn(), + }); + mockUseClusterService.mockReturnValue({ deployments: [], loaded: true, error: null }); + + render( + + + , + ); + + expect(screen.getByText('PatModal-open')).toBeInTheDocument(); + }); + + it('does not render PatModal when PAT is set', () => { + mockUsePat.mockReturnValue({ pat: 'ghp_valid', setPat: jest.fn(), clearPat: jest.fn() }); + mockUseSourceControl.mockReturnValue({ + listFunctionRepos: jest.fn().mockResolvedValue([]), + fetchFileContent: jest.fn(), + }); + mockUseClusterService.mockReturnValue({ deployments: [], loaded: true, error: null }); + + render( + + + , + ); + + expect(screen.queryByText('PatModal-open')).not.toBeInTheDocument(); + }); }); diff --git a/src/views/FunctionsListPage.tsx b/src/views/FunctionsListPage.tsx index e043396..e88b96f 100644 --- a/src/views/FunctionsListPage.tsx +++ b/src/views/FunctionsListPage.tsx @@ -11,13 +11,17 @@ import { FunctionsEmptyState } from '../components/EmptyState'; import { FunctionStatus, FunctionTable, FunctionTableItem } from '../components/FunctionTable'; import { useSourceControl } from '../services/source-control/useSourceControl'; import { useClusterService } from '../services/cluster/useClusterService'; +import { usePat } from '../hooks/usePat'; +import { PatModal } from '../components/PatModal'; export default function FunctionsListPage() { const { t } = useTranslation('plugin__console-functions-plugin'); - const { functions, loaded, onEdit } = useFunctionListPage(); + const { pat, setPat } = usePat(); + const { functions, loaded, onEdit } = useFunctionListPage(pat); return ( <> + {t('Functions')} @@ -48,12 +52,12 @@ export default function FunctionsListPage() { ); } -function useFunctionListPage(): { +function useFunctionListPage(pat: string): { functions: FunctionTableItem[]; loaded: boolean; onEdit: (name: string) => void; } { - const sourceControl = useSourceControl(); + const sourceControl = useSourceControl(pat); const { deployments, loaded: clusterLoaded } = useClusterService(); const navigate = useNavigate(); @@ -61,6 +65,11 @@ function useFunctionListPage(): { const [reposLoaded, setReposLoaded] = useState(false); useEffect(() => { + if (!pat) { + setReposLoaded(true); + return; + } + let ignore = false; async function loadFunctionTableItems() { @@ -86,7 +95,7 @@ function useFunctionListPage(): { return () => { ignore = true; }; - }, [sourceControl]); + }, [sourceControl, pat]); const functions = useMemo( () => diff --git a/webpack.config.ts b/webpack.config.ts index 548da90..1e91fbe 100644 --- a/webpack.config.ts +++ b/webpack.config.ts @@ -1,7 +1,7 @@ /* eslint-env node */ import * as path from 'path'; -import { Configuration as WebpackConfiguration, DefinePlugin } from 'webpack'; +import { Configuration as WebpackConfiguration } from 'webpack'; import { Configuration as WebpackDevServerConfiguration } from 'webpack-dev-server'; import { ConsoleRemotePlugin } from '@openshift-console/dynamic-plugin-sdk-webpack'; @@ -74,9 +74,6 @@ const config: Configuration = { }, }, plugins: [ - new DefinePlugin({ - __GITHUB_PAT__: JSON.stringify(process.env.GITHUB_PAT || ''), - }), new ConsoleRemotePlugin(), new CopyWebpackPlugin({ patterns: [{ from: path.resolve(__dirname, 'locales'), to: 'locales' }],