diff --git a/.vscode/settings.json b/.vscode/settings.json index 15e3474..0b49b50 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -16,8 +16,11 @@ }, "json.schemas": [ { - "fileMatch": ["**/console-extensions.json"], + "fileMatch": [ + "**/console-extensions.json" + ], "url": "./node_modules/@openshift-console/dynamic-plugin-sdk-webpack/schema/console-extensions.json" } - ] + ], + "typescript.preferences.importModuleSpecifier": "relative" } diff --git a/docs/claude-progress.txt b/docs/claude-progress.txt index 0299689..cfe2b2f 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-28 | Session: Runtime PAT entry and user avatar +Worked on: Replace compile-time PAT injection with runtime modal entry, add user avatar to page headers +Completed: +- Runtime PAT entry via PatModal (PF6 Modal, password input, validation, error alert, small variant) +- UserAvatar component in ListPageHeader on all pages (list: clickable, create/edit: disabled) +- PatModal and useUserAvatar consolidated into UserAvatar as private internals, usePatModal hook extracted +- ForgeConnectionProvider context for cross-component PAT state sharing +- GithubService refactored: lazy auth via cached Octokit getter, fetchUserInfo on SourceControlService interface +- ForgeUser type (forge-agnostic naming), PAT and user stored in sessionStorage +- EmptyState simplified to single isCreateDisabled prop (PAT hint text internal, no Tooltip) +- FunctionsListPage wired: autoOpen modal, disabled Create when unauthenticated, skip API call when no PAT +- FunctionCreatePage protected against unauthenticated URL access (warning alert, form hidden) +- Removed DefinePlugin for __GITHUB_PAT__ from webpack.config.ts, deleted src/globals.d.ts +- Extracted errorMessage utility (src/utils/errorMessage.ts) +- Cancel and modal close disabled while validating +- 9 suites, 55 tests, all passing, zero lint errors +Left off: Squash and merge PR #15. +Blockers: None + --- ## 2026-04-20 | Session: CI/CD pipeline and ESLint fixes Worked on: GitHub Actions CI/CD pipeline, ESLint config fixes, README restructure diff --git a/docs/features.json b/docs/features.json index 7e3eb2b..1eae9b9 100644 --- a/docs/features.json +++ b/docs/features.json @@ -103,14 +103,13 @@ "PatModal opens automatically on first visit to Function List Page when no PAT is stored in session", "PatModal is dismissable — user can close it without entering a PAT", "PatModal validates the PAT against GitHub API and on success stores it in session and closes", - "SourceControlService interface extended with init(pat: string) and isInitialized(): boolean methods; GithubService implementation updated accordingly", "UserAvatar component renders inside ListPageHeader at the top-right corner on every page (list, create, edit)", "UserAvatar has two states: not initialized shows 'Connect to GitHub' with a key/lock icon (clickable, opens PatModal); initialized shows GitHub username with avatar icon", "UserAvatar is clickable only on the Function List Page, clicking reopens PatModal to change the PAT and associated GitHub user", "On Create and Edit pages, UserAvatar displays the GitHub user but is not clickable (PAT cannot be changed from those pages)", "When PatModal is dismissed without a PAT, the Function List Page (both empty state and table view) shows a hint text directing the user to click 'Connect to GitHub' in the top-right corner to set a PAT", "PAT change logic is encapsulated in a useUserAvatar custom hook following layered architecture (Hook imports Service, Component imports Hook)", - "If the GH PAT is not set in the session then the Create button is inactive/disabled. The tooltiop of the button should say that PAT is required.", + "If the GH PAT is not set in the session then the Create button is inactive/disabled.", "The GH PAT must not be compiled/hardcode into code at compile time." ], "passes": false diff --git a/locales/en/plugin__console-functions-plugin.json b/locales/en/plugin__console-functions-plugin.json index 05f3ce7..5a1d2af 100644 --- a/locales/en/plugin__console-functions-plugin.json +++ b/locales/en/plugin__console-functions-plugin.json @@ -1,13 +1,18 @@ { + "A GitHub Personal Access Token is required to create functions. Click 'Connect to GitHub' in the top-right corner to connect. Once connected, the create button will be enabled.": "A GitHub Personal Access Token is required to create functions. Click 'Connect to GitHub' in the top-right corner to connect. Once connected, the create button will be enabled.", + "A GitHub Personal Access Token is required to create functions. Go to the Functions page and click 'Connect to GitHub' to connect.": "A GitHub Personal Access Token is required to create functions. Go to the Functions page and click 'Connect to GitHub' to connect.", "Actions": "Actions", "Branch": "Branch", "Cancel": "Cancel", + "Connect": "Connect", + "Connect to GitHub": "Connect to GitHub", "Create": "Create", "Create a serverless function to get started.": "Create a serverless function to get started.", "Create function": "Create function", "Create new function": "Create new function", "Delete": "Delete", "Edit": "Edit", + "Enter your GitHub Personal Access Token to connect your repositories.": "Enter your GitHub Personal Access Token to connect your repositories.", "Error creating function": "Error creating function", "Function Settings": "Function Settings", "FaaS": "FaaS", diff --git a/src/components/EmptyState.test.tsx b/src/components/EmptyState.test.tsx index fa01905..f516404 100644 --- a/src/components/EmptyState.test.tsx +++ b/src/components/EmptyState.test.tsx @@ -31,4 +31,32 @@ describe('FunctionsEmptyState', () => { const link = screen.getByRole('link', { name: 'Create function' }); expect(link).toHaveAttribute('href', '/faas/create'); }); + + it('shows PAT hint and disabled button when isCreateDisabled is true', () => { + render( + + + , + ); + + expect( + screen.getByText(/A GitHub Personal Access Token is required to create functions/), + ).toBeInTheDocument(); + expect( + screen.queryByText('Create a serverless function to get started.'), + ).not.toBeInTheDocument(); + expect(screen.getByRole('button', { name: 'Create function' })).toBeDisabled(); + }); + + it('shows standard body text when isCreateDisabled is false', () => { + render( + + + , + ); + + expect(screen.getByText('Create a serverless function to get started.')).toBeInTheDocument(); + const link = screen.getByRole('link', { name: 'Create function' }); + expect(link).toHaveAttribute('href', '/faas/create'); + }); }); diff --git a/src/components/EmptyState.tsx b/src/components/EmptyState.tsx index dc204cc..d27bd97 100644 --- a/src/components/EmptyState.tsx +++ b/src/components/EmptyState.tsx @@ -9,17 +9,33 @@ import { CubesIcon } from '@patternfly/react-icons'; import { useTranslation } from 'react-i18next'; import { Link } from 'react-router-dom-v5-compat'; -export function FunctionsEmptyState() { +interface FunctionsEmptyStateProps { + isCreateDisabled?: boolean; +} + +export function FunctionsEmptyState({ isCreateDisabled }: FunctionsEmptyStateProps) { const { t } = useTranslation('plugin__console-functions-plugin'); return ( - {t('Create a serverless function to get started.')} + + {isCreateDisabled + ? t( + "A GitHub Personal Access Token is required to create functions. Click 'Connect to GitHub' in the top-right corner to connect. Once connected, the create button will be enabled.", + ) + : t('Create a serverless function to get started.')} + - + {isCreateDisabled ? ( + + ) : ( + + )} diff --git a/src/components/UserAvatar.test.tsx b/src/components/UserAvatar.test.tsx new file mode 100644 index 0000000..017f29b --- /dev/null +++ b/src/components/UserAvatar.test.tsx @@ -0,0 +1,178 @@ +import { render, screen, waitFor } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; +import { UserAvatar } from './UserAvatar'; +import { PAT_KEY, USER_KEY } from '../services/types'; +import { ForgeConnectionContext } from '../context/ForgeConnectionProvider'; +import { ReactNode } from 'react'; + +jest.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +const mockFetchUserInfo = jest.fn(); +jest.mock('../services/source-control/useSourceControlService', () => ({ + useSourceControlService: () => ({ + fetchUserInfo: mockFetchUserInfo, + }), +})); + +const testUser = { name: 'twoGiants' }; + +function renderWithContext( + ui: ReactNode, + contextValue = { isActive: false, connectToForge: jest.fn() }, +) { + return render( + {ui}, + ); +} + +describe('UserAvatar', () => { + beforeEach(() => { + sessionStorage.clear(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + afterAll(() => { + sessionStorage.clear(); + }); + + describe('rendering', () => { + it('renders "Connect to GitHub" when no user is stored', () => { + renderWithContext(); + + expect(screen.getByText('Connect to GitHub')).toBeInTheDocument(); + }); + + it('renders username when user is stored in sessionStorage', () => { + sessionStorage.setItem(PAT_KEY, 'ghp_test'); + sessionStorage.setItem(USER_KEY, JSON.stringify(testUser)); + + renderWithContext(); + + expect(screen.getByText('twoGiants')).toBeInTheDocument(); + }); + + it('button is clickable when enableReconnect is true', async () => { + const user = userEvent.setup(); + sessionStorage.setItem(PAT_KEY, 'ghp_test'); + sessionStorage.setItem(USER_KEY, JSON.stringify(testUser)); + + renderWithContext(); + + const button = screen.getByRole('button', { name: 'twoGiants' }); + await user.click(button); + + expect(screen.getByText('Personal Access Token')).toBeInTheDocument(); + }); + + it('button is disabled when enableReconnect is false', async () => { + const user = userEvent.setup(); + + renderWithContext(); + + const button = screen.getByRole('button', { name: 'Connect to GitHub' }); + expect(button).toBeDisabled(); + + await user.click(button); + expect(screen.queryByText('Personal Access Token')).not.toBeInTheDocument(); + }); + }); + + describe('modal auto-open', () => { + it('opens modal automatically when enableReconnect is true and no PAT stored', () => { + renderWithContext(); + + expect(screen.getByText('Personal Access Token')).toBeInTheDocument(); + }); + + it('does not auto-open modal when PAT is already stored', () => { + sessionStorage.setItem(PAT_KEY, 'ghp_test'); + sessionStorage.setItem(USER_KEY, JSON.stringify(testUser)); + + renderWithContext(); + + expect(screen.queryByText('Personal Access Token')).not.toBeInTheDocument(); + }); + + it('does not auto-open modal when enableReconnect is false', () => { + renderWithContext(); + + expect(screen.queryByText('Personal Access Token')).not.toBeInTheDocument(); + }); + }); + + describe('PAT modal', () => { + it('Connect button disabled when input is empty', () => { + renderWithContext(); + + expect(screen.getByRole('button', { name: 'Connect' })).toBeDisabled(); + }); + + it('calls fetchUserInfo with PAT and updates UI on successful connect', async () => { + const user = userEvent.setup(); + const connectToForge = jest.fn(); + mockFetchUserInfo.mockResolvedValue(testUser); + + renderWithContext(, { isActive: false, connectToForge }); + + await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_valid'); + await user.click(screen.getByRole('button', { name: 'Connect' })); + + await waitFor(() => { + expect(mockFetchUserInfo).toHaveBeenCalledWith('ghp_valid'); + }); + + expect(screen.getByText('twoGiants')).toBeInTheDocument(); + expect(sessionStorage.getItem(PAT_KEY)).toBe('ghp_valid'); + expect(JSON.parse(sessionStorage.getItem(USER_KEY)!)).toEqual(testUser); + expect(connectToForge).toHaveBeenCalled(); + }); + + it('shows error alert when fetchUserInfo rejects', async () => { + const user = userEvent.setup(); + mockFetchUserInfo.mockRejectedValue(new Error('Bad credentials')); + + renderWithContext(); + + await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_bad'); + await user.click(screen.getByRole('button', { name: 'Connect' })); + + expect(await screen.findByText('Bad credentials')).toBeInTheDocument(); + }); + + it('closes modal when Cancel is clicked', async () => { + const user = userEvent.setup(); + + renderWithContext(); + + expect(screen.getByText('Personal Access Token')).toBeInTheDocument(); + + await user.click(screen.getByRole('button', { name: 'Cancel' })); + + expect(screen.queryByText('Personal Access Token')).not.toBeInTheDocument(); + }); + + it('disables Cancel button while validating', async () => { + const user = userEvent.setup(); + let resolveConnect: () => void; + mockFetchUserInfo.mockReturnValue( + new Promise((resolve) => { + resolveConnect = resolve; + }), + ); + + renderWithContext(); + + await user.type(screen.getByLabelText('Personal Access Token'), 'ghp_slow'); + await user.click(screen.getByRole('button', { name: 'Connect' })); + + expect(screen.getByRole('button', { name: 'Cancel' })).toBeDisabled(); + + resolveConnect!(); + }); + }); +}); diff --git a/src/components/UserAvatar.tsx b/src/components/UserAvatar.tsx new file mode 100644 index 0000000..b44ccbd --- /dev/null +++ b/src/components/UserAvatar.tsx @@ -0,0 +1,149 @@ +import { + Alert, + Button, + Form, + FormGroup, + Modal, + ModalBody, + ModalFooter, + ModalHeader, + TextInput, +} from '@patternfly/react-core'; +import { KeyIcon, UserIcon } from '@patternfly/react-icons'; +import { useTranslation } from 'react-i18next'; +import { ForgeUser, PAT_KEY, USER_KEY } from '../services/types'; +import { useContext, useState } from 'react'; +import { ForgeConnectionContext } from '../context/ForgeConnectionProvider'; +import { useSourceControlService } from '../services/source-control/useSourceControlService'; +import { errorMessage } from '../utils/errorMessage'; + +interface UserAvatarProps { + enableReconnect: boolean; +} + +export function UserAvatar({ enableReconnect }: UserAvatarProps) { + const { t } = useTranslation('plugin__console-functions-plugin'); + const { user, isModalOpen, openModal, closeModal, login } = useUserAvatar(enableReconnect); + + const icon = user ? : ; + const label = user ? user.name : t('Connect to GitHub'); + + return ( + <> + + + + ); +} + +function useUserAvatar(enableReconnect: boolean) { + const sourceControlService = useSourceControlService(); + const connectToForge = useContext(ForgeConnectionContext).connectToForge; + const [user, setUser] = useState(() => readStoredUser()); + const [isModalOpen, setIsModalOpen] = useState( + () => enableReconnect && !sessionStorage.getItem(PAT_KEY), + ); + + const login = async (pat: string) => { + const forgeUser = await sourceControlService.fetchUserInfo(pat); + sessionStorage.setItem(PAT_KEY, pat); + sessionStorage.setItem(USER_KEY, JSON.stringify(forgeUser)); + setUser(forgeUser); + setIsModalOpen(false); + connectToForge(); + }; + + const openModal = () => setIsModalOpen(true); + const closeModal = () => setIsModalOpen(false); + + return { user, isModalOpen, openModal, closeModal, login }; +} + +function readStoredUser(): ForgeUser | null { + const pat = sessionStorage.getItem(PAT_KEY); + const userJson = sessionStorage.getItem(USER_KEY); + + if (!pat || !userJson) { + return null; + } + + try { + return JSON.parse(userJson) as ForgeUser; + } catch { + return null; + } +} + +interface PatModalProps { + isOpen: boolean; + onClose: () => void; + onConnect: (pat: string) => Promise; +} + +function PatModal({ isOpen, onClose, onConnect }: PatModalProps) { + const { t } = useTranslation('plugin__console-functions-plugin'); + const { pat, isValidating, error, setPat, handleConnect } = usePatModal(onConnect); + + return ( + + + + {error && ( + + )} +
+ + setPat(value)} + /> + +
+ {t('Enter your GitHub Personal Access Token to connect your repositories.')} + + + + + + + ); +} + +function usePatModal(onConnect: (pat: string) => Promise) { + const [pat, setPat] = useState(''); + const [isValidating, setIsValidating] = useState(false); + const [error, setError] = useState(null); + + const handleConnect = async () => { + setIsValidating(true); + setError(null); + try { + await onConnect(pat); + } catch (err) { + setError(errorMessage(err)); + } finally { + setIsValidating(false); + } + }; + + return { pat, isValidating, error, setPat, handleConnect }; +} diff --git a/src/context/ForgeConnectionProvider.tsx b/src/context/ForgeConnectionProvider.tsx new file mode 100644 index 0000000..81d7a78 --- /dev/null +++ b/src/context/ForgeConnectionProvider.tsx @@ -0,0 +1,26 @@ +import { createContext, ReactNode, useState } from 'react'; +import { PAT_KEY } from '../services/types'; + +interface ForgeConnection { + isActive: boolean; + connectToForge: () => void; +} + +export const ForgeConnectionContext = createContext({ + isActive: false, + connectToForge: () => {}, +}); + +export function ForgeConnectionProvider({ children }: Readonly<{ children: ReactNode }>) { + const [isActive, setIsActive] = useState(() => !!sessionStorage.getItem(PAT_KEY)); + + const connectToForge = () => { + setIsActive(true); + }; + + return ( + + {children} + + ); +} 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/services/source-control/GithubService.ts b/src/services/source-control/GithubService.ts index ad9a1ff..3133af9 100644 --- a/src/services/source-control/GithubService.ts +++ b/src/services/source-control/GithubService.ts @@ -1,23 +1,36 @@ import { Octokit } from '@octokit/rest'; -import { FileEntry, RepoInfo, SourceRepo } from '../types'; +import { FileEntry, ForgeUser, RepoInfo, SourceRepo } from '../types'; import { SourceControlService } from './SourceControlService'; export class GithubService implements SourceControlService { - private octokit: Octokit; - private username: string | undefined; + private getToken: () => string; + private cachedOctokit: Octokit | null = null; + private cachedToken: string = ''; - constructor(pat: string) { - this.octokit = new Octokit({ auth: pat }); + constructor(getToken: () => string) { + this.getToken = getToken; } - async listFunctionRepos(): Promise { - if (!this.username) { - const { data: user } = await this.octokit.users.getAuthenticated(); - this.username = user.login; + private get octokit(): Octokit { + const token = this.getToken(); + if (token !== this.cachedToken) { + this.cachedToken = token; + this.cachedOctokit = new Octokit({ auth: token }); } + return this.cachedOctokit!; + } + + async fetchUserInfo(pat: string): Promise { + const octokit = new Octokit({ auth: pat }); + const { data } = await octokit.users.getAuthenticated(); + return { name: data.login }; + } + + async listFunctionRepos(): Promise { + const { data: user } = await this.octokit.users.getAuthenticated(); const { data } = await this.octokit.search.repos({ - q: `topic:serverless-function user:${this.username}`, + q: `topic:serverless-function user:${user.login}`, }); return data.items.map((item) => ({ diff --git a/src/services/source-control/SourceControlService.test.ts b/src/services/source-control/SourceControlService.test.ts index c989b74..33a8037 100644 --- a/src/services/source-control/SourceControlService.test.ts +++ b/src/services/source-control/SourceControlService.test.ts @@ -1,6 +1,7 @@ import { GithubService } from './GithubService'; import { FileEntry, RepoInfo, SourceRepo } from '../types'; +const mockGetAuthenticated = jest.fn(); const mockSearch = jest.fn(); const mockGetContent = jest.fn(); const mockCreateBlob = jest.fn(); @@ -10,7 +11,7 @@ const mockCreateRef = jest.fn(); jest.mock('@octokit/rest', () => ({ Octokit: jest.fn().mockImplementation(() => ({ - users: { getAuthenticated: jest.fn().mockResolvedValue({ data: { login: 'twoGiants' } }) }, + users: { getAuthenticated: mockGetAuthenticated }, search: { repos: mockSearch }, repos: { getContent: mockGetContent }, git: { @@ -27,6 +28,12 @@ afterEach(() => { }); describe('GithubService', () => { + beforeEach(() => { + mockGetAuthenticated.mockResolvedValue({ + data: { login: 'twoGiants', avatar_url: 'https://avatars.githubusercontent.com/u/123' }, + }); + }); + it('lists function repos tagged with serverless-function topic', async () => { mockSearch.mockResolvedValue({ data: { @@ -41,7 +48,7 @@ describe('GithubService', () => { }, }); - const svc = new GithubService('fake-token'); + const svc = new GithubService(() => 'fake-token'); const repos: SourceRepo[] = await svc.listFunctionRepos(); expect(repos).toEqual([ @@ -60,7 +67,7 @@ describe('GithubService', () => { data: { content: btoa('name: my-func\nruntime: go\n'), encoding: 'base64' }, }); - const svc = new GithubService('fake-token'); + const svc = new GithubService(() => 'fake-token'); const content = await svc.fetchFileContent( { owner: 'twoGiants', @@ -93,7 +100,7 @@ describe('GithubService', () => { }); it('creates an initial commit with the provided files', async () => { - const svc = new GithubService('fake-token'); + const svc = new GithubService(() => 'fake-token'); await svc.push(repoInfo, files, 'Initialize function'); expect(mockCreateBlob).toHaveBeenCalledWith({ @@ -124,11 +131,27 @@ describe('GithubService', () => { it('propagates errors from intermediate API calls', async () => { mockCreateTree.mockRejectedValue(new Error('Validation Failed')); - const svc = new GithubService('fake-token'); + const svc = new GithubService(() => 'fake-token'); await expect(svc.push(repoInfo, files, 'Initialize function')).rejects.toThrow( 'Validation Failed', ); }); }); + + describe('fetchUserInfo', () => { + it('returns ForgeUser on valid token', async () => { + const svc = new GithubService(() => ''); + const user = await svc.fetchUserInfo('valid-token'); + + expect(user).toEqual({ name: 'twoGiants' }); + }); + + it('throws on invalid token', async () => { + mockGetAuthenticated.mockRejectedValue(new Error('Bad credentials')); + const svc = new GithubService(() => ''); + + await expect(svc.fetchUserInfo('bad-token')).rejects.toThrow('Bad credentials'); + }); + }); }); diff --git a/src/services/source-control/SourceControlService.ts b/src/services/source-control/SourceControlService.ts index 27d2959..c7555d8 100644 --- a/src/services/source-control/SourceControlService.ts +++ b/src/services/source-control/SourceControlService.ts @@ -1,7 +1,8 @@ -import { FileEntry, RepoInfo, SourceRepo } from '../types'; +import { FileEntry, ForgeUser as ForgeUserInfo, RepoInfo, SourceRepo } from '../types'; export interface SourceControlService { listFunctionRepos(): Promise; fetchFileContent(repo: SourceRepo, path: string): Promise; push(repo: RepoInfo, files: FileEntry[], message: string): Promise; + fetchUserInfo(pat: string): Promise; } diff --git a/src/services/source-control/useSourceControlService.ts b/src/services/source-control/useSourceControlService.ts index 77fd910..4f484f2 100644 --- a/src/services/source-control/useSourceControlService.ts +++ b/src/services/source-control/useSourceControlService.ts @@ -1,11 +1,7 @@ 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); +const instance = new GithubService(() => sessionStorage.getItem('func-console-pat') || ''); export function useSourceControlService(): SourceControlService { return instance; diff --git a/src/services/types.ts b/src/services/types.ts index 562d77c..f7cb21d 100644 --- a/src/services/types.ts +++ b/src/services/types.ts @@ -1,3 +1,6 @@ +export const PAT_KEY = 'func-console-pat'; +export const USER_KEY = 'func-console-user'; + export interface FileEntry { path: string; mode: '100644' | '100755' | '120000'; @@ -27,3 +30,7 @@ export interface SourceRepo { url: string; defaultBranch: string; } + +export interface ForgeUser { + name: string; +} diff --git a/src/utils/errorMessage.ts b/src/utils/errorMessage.ts new file mode 100644 index 0000000..7db4add --- /dev/null +++ b/src/utils/errorMessage.ts @@ -0,0 +1,3 @@ +export function errorMessage(err: unknown): string { + return err instanceof Error ? err.message : String(err); +} diff --git a/src/views/FunctionCreatePage.test.tsx b/src/views/FunctionCreatePage.test.tsx index 009ca85..7d57ffe 100644 --- a/src/views/FunctionCreatePage.test.tsx +++ b/src/views/FunctionCreatePage.test.tsx @@ -2,6 +2,7 @@ import { render, screen, waitFor } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; import { MemoryRouter } from 'react-router-dom'; import FunctionCreatePage from './FunctionCreatePage'; +import { PAT_KEY } from '../services/types'; const mockGenerateFunction = jest.fn(); const mockPush = jest.fn(); @@ -13,7 +14,12 @@ jest.mock('react-i18next', () => ({ jest.mock('@openshift-console/dynamic-plugin-sdk', () => ({ DocumentTitle: ({ children }: { children: string }) => children, - ListPageHeader: ({ title }: { title: string }) => title, + ListPageHeader: ({ title, children }: { title: string; children?: React.ReactNode }) => ( + <> + {title} + {children} + + ), })); jest.mock('../services/function/useFunctionService', () => ({ @@ -32,10 +38,24 @@ jest.mock('react-router-dom-v5-compat', () => ({ useNavigate: () => mockNavigate, })); +jest.mock('../components/UserAvatar', () => ({ + UserAvatar: ({ enableReconnect }: { enableReconnect: boolean }) => ( + {enableReconnect ? 'reconnect' : 'no-reconnect'} + ), +})); + +beforeEach(() => { + sessionStorage.clear(); +}); + afterEach(() => { jest.clearAllMocks(); }); +afterAll(() => { + sessionStorage.clear(); +}); + const fillForm = async (user: ReturnType) => { await user.type(screen.getByRole('textbox', { name: /Owner/ }), 'testuser'); await user.type(screen.getByRole('textbox', { name: /Repository/ }), 'my-repo'); @@ -47,6 +67,8 @@ const fillForm = async (user: ReturnType) => { describe('FunctionCreatePage', () => { it('renders CreateFunctionForm', () => { + sessionStorage.setItem(PAT_KEY, 'ghp_test'); + render( @@ -58,6 +80,7 @@ describe('FunctionCreatePage', () => { }); it('calls generateFunction then push on submit, and navigates on success', async () => { + sessionStorage.setItem(PAT_KEY, 'ghp_test'); const user = userEvent.setup(); const files = [{ path: 'func.yaml', mode: '100644', content: 'name: f', type: 'blob' }]; mockGenerateFunction.mockResolvedValue(files); @@ -96,6 +119,7 @@ describe('FunctionCreatePage', () => { }); it('shows an alert on error', async () => { + sessionStorage.setItem(PAT_KEY, 'ghp_test'); const user = userEvent.setup(); mockGenerateFunction.mockRejectedValue(new Error('Backend error')); @@ -112,4 +136,27 @@ describe('FunctionCreatePage', () => { expect(screen.getByText('Backend error')).toBeInTheDocument(); }); }); + + it('renders UserAvatar in header', () => { + render( + + + , + ); + + expect(screen.getByTestId('user-avatar')).toBeInTheDocument(); + }); + + it('shows warning and hides form when no PAT is set', () => { + render( + + + , + ); + + expect( + screen.getByText(/A GitHub Personal Access Token is required to create functions/), + ).toBeInTheDocument(); + expect(screen.queryByRole('textbox', { name: /Owner/ })).not.toBeInTheDocument(); + }); }); diff --git a/src/views/FunctionCreatePage.tsx b/src/views/FunctionCreatePage.tsx index bff5d3b..f35b75d 100644 --- a/src/views/FunctionCreatePage.tsx +++ b/src/views/FunctionCreatePage.tsx @@ -6,26 +6,43 @@ import { useNavigate } from 'react-router-dom-v5-compat'; import { CreateFunctionForm, CreateFunctionFormData } from '../components/CreateFunctionForm'; import { useFunctionService } from '../services/function/useFunctionService'; import { useSourceControlService } from '../services/source-control/useSourceControlService'; +import { UserAvatar } from '../components/UserAvatar'; +import { PAT_KEY } from '../services/types'; +import { errorMessage } from '../utils/errorMessage'; export default function FunctionCreatePage() { const { t } = useTranslation('plugin__console-functions-plugin'); + const isConnected = !!sessionStorage.getItem(PAT_KEY); const { isSubmitting, error, handleSubmit, handleCancel } = useFunctionCreatePage(); return ( <> {t('Create function')} - + + + + {!isConnected && ( + + )} {error && ( {error} )} - + {isConnected && ( + + )} ); @@ -60,7 +77,7 @@ function useFunctionCreatePage() { navigate('/faas'); } catch (err) { - setError(err instanceof Error ? err.message : String(err)); + setError(errorMessage(err)); } finally { setIsSubmitting(false); } diff --git a/src/views/FunctionEditPage.tsx b/src/views/FunctionEditPage.tsx index 5ca0f43..7d901e2 100644 --- a/src/views/FunctionEditPage.tsx +++ b/src/views/FunctionEditPage.tsx @@ -2,6 +2,7 @@ import { DocumentTitle, ListPageHeader } from '@openshift-console/dynamic-plugin import { PageSection } from '@patternfly/react-core'; import { useTranslation } from 'react-i18next'; import { useParams } from 'react-router-dom-v5-compat'; +import { UserAvatar } from '../components/UserAvatar'; export default function FunctionEditPage() { const { t } = useTranslation('plugin__console-functions-plugin'); @@ -10,7 +11,9 @@ export default function FunctionEditPage() { return ( <> {t('Edit function')} - + + + {t('Coming soon.')} ); diff --git a/src/views/FunctionsListPage.test.tsx b/src/views/FunctionsListPage.test.tsx index c1a677b..108bd63 100644 --- a/src/views/FunctionsListPage.test.tsx +++ b/src/views/FunctionsListPage.test.tsx @@ -1,6 +1,7 @@ import { render, screen } from '@testing-library/react'; import { MemoryRouter } from 'react-router-dom-v5-compat'; import FunctionsListPage from './FunctionsListPage'; +import { PAT_KEY } from '../services/types'; jest.mock('react-i18next', () => ({ useTranslation: () => ({ t: (key: string) => key }), @@ -31,12 +32,31 @@ jest.mock('../components/FunctionTable', () => ({ functions.map((f) => f.name).join(','), })); -afterEach(() => { - jest.restoreAllMocks(); -}); +jest.mock('../components/UserAvatar', () => ({ + UserAvatar: ({ enableReconnect }: { enableReconnect: boolean }) => ( + {enableReconnect ? 'reconnect' : 'no-reconnect'} + ), +})); + +function renderAuthenticated() { + sessionStorage.setItem(PAT_KEY, 'ghp_test'); +} describe('FunctionsListPage', () => { + beforeEach(() => { + sessionStorage.clear(); + }); + + afterEach(() => { + jest.restoreAllMocks(); + }); + + afterAll(() => { + sessionStorage.clear(); + }); + it('renders a spinner while loading', () => { + renderAuthenticated(); mockUseSourceControl.mockReturnValue({ listFunctionRepos: jest.fn().mockResolvedValue([]), fetchFileContent: jest.fn(), @@ -53,6 +73,7 @@ describe('FunctionsListPage', () => { }); it('renders the empty state when loaded with no functions', async () => { + renderAuthenticated(); mockUseSourceControl.mockReturnValue({ listFunctionRepos: jest.fn().mockResolvedValue([]), fetchFileContent: jest.fn(), @@ -69,6 +90,7 @@ describe('FunctionsListPage', () => { }); it('renders table when functions are loaded', async () => { + renderAuthenticated(); mockUseSourceControl.mockReturnValue({ listFunctionRepos: jest.fn().mockResolvedValue([ { @@ -110,6 +132,7 @@ describe('FunctionsListPage', () => { }); it('shows NotDeployed status for repos without cluster deployment', async () => { + renderAuthenticated(); mockUseSourceControl.mockReturnValue({ listFunctionRepos: jest.fn().mockResolvedValue([ { @@ -135,6 +158,7 @@ describe('FunctionsListPage', () => { }); it('renders empty state when GitHub API fails', async () => { + renderAuthenticated(); mockUseSourceControl.mockReturnValue({ listFunctionRepos: jest.fn().mockRejectedValue(new Error('Requires authentication')), fetchFileContent: jest.fn(), @@ -149,4 +173,59 @@ describe('FunctionsListPage', () => { expect(await screen.findByRole('heading', { name: 'No functions found' })).toBeInTheDocument(); }); + + it('does not call listFunctionRepos when not authenticated', async () => { + const mockListRepos = jest.fn().mockResolvedValue([]); + mockUseSourceControl.mockReturnValue({ + listFunctionRepos: mockListRepos, + fetchFileContent: jest.fn(), + }); + mockUseClusterService.mockReturnValue({ deployments: [], loaded: true, error: null }); + + render( + + + , + ); + + await screen.findByRole('heading', { name: 'No functions found' }); + + expect(mockListRepos).not.toHaveBeenCalled(); + }); + + it('renders UserAvatar in header', () => { + renderAuthenticated(); + mockUseSourceControl.mockReturnValue({ + listFunctionRepos: jest.fn().mockResolvedValue([]), + fetchFileContent: jest.fn(), + }); + mockUseClusterService.mockReturnValue({ deployments: [], loaded: true, error: null }); + + render( + + + , + ); + + expect(screen.getByTestId('user-avatar')).toBeInTheDocument(); + }); + + it('empty state receives hint and isCreateDisabled when not authenticated', async () => { + mockUseSourceControl.mockReturnValue({ + listFunctionRepos: jest.fn().mockResolvedValue([]), + fetchFileContent: jest.fn(), + }); + mockUseClusterService.mockReturnValue({ deployments: [], loaded: true, error: null }); + + render( + + + , + ); + + await screen.findByRole('heading', { name: 'No functions found' }); + + const button = screen.getByRole('button', { name: 'Create function' }); + expect(button).toBeDisabled(); + }); }); diff --git a/src/views/FunctionsListPage.tsx b/src/views/FunctionsListPage.tsx index 2607c8f..e6e6018 100644 --- a/src/views/FunctionsListPage.tsx +++ b/src/views/FunctionsListPage.tsx @@ -6,25 +6,42 @@ import { import { Button, Content, ContentVariants, PageSection, Spinner } from '@patternfly/react-core'; import { Link, useNavigate } from 'react-router-dom-v5-compat'; import { useTranslation } from 'react-i18next'; -import { useEffect, useState, useMemo } from 'react'; +import { useEffect, useState, useMemo, useContext } from 'react'; import { FunctionsEmptyState } from '../components/EmptyState'; import { FunctionStatus, FunctionTable, FunctionTableItem } from '../components/FunctionTable'; import { useSourceControlService } from '../services/source-control/useSourceControlService'; import { useClusterService } from '../services/cluster/useClusterService'; +import { + ForgeConnectionProvider, + ForgeConnectionContext, +} from '../context/ForgeConnectionProvider'; +import { UserAvatar } from '../components/UserAvatar'; export default function FunctionsListPage() { + return ( + + + + ); +} + +function FunctionsListPageContent() { const { t } = useTranslation('plugin__console-functions-plugin'); - const { functions, loaded, onEdit } = useFunctionListPage(); + const { functions, loaded, onEdit, isConnectedToForge } = useFunctionListPage(); return ( <> {t('Functions')} - + + + {!loaded && ( )} - {loaded && functions.length === 0 && } + {loaded && functions.length === 0 && ( + + )} {loaded && functions.length > 0 && ( <> @@ -33,12 +50,18 @@ export default function FunctionsListPage() { )} - + {!isConnectedToForge ? ( + + ) : ( + + )} @@ -52,15 +75,31 @@ function useFunctionListPage(): { functions: FunctionTableItem[]; loaded: boolean; onEdit: (name: string) => void; + isConnectedToForge: boolean; } { + const isConnectedToForge = useContext(ForgeConnectionContext).isActive; const sourceControl = useSourceControlService(); const { deployments, loaded: clusterLoaded } = useClusterService(); const navigate = useNavigate(); const [functionItems, setFunctionItems] = useState([]); - const [reposLoaded, setReposLoaded] = useState(false); + const [reposLoaded, setReposLoaded] = useState(!isConnectedToForge); + const [wasConnectedToForge, setWasConnectedToForge] = useState(isConnectedToForge); + + // Reset state when authentication status changes (render-time adjustment) + if (isConnectedToForge !== wasConnectedToForge) { + setWasConnectedToForge(isConnectedToForge); + if (isConnectedToForge) { + setReposLoaded(false); + } else { + setFunctionItems([]); + setReposLoaded(true); + } + } useEffect(() => { + if (!isConnectedToForge) return; + let ignore = false; async function loadFunctionTableItems() { @@ -79,14 +118,12 @@ function useFunctionListPage(): { } loadFunctionTableItems().catch(() => { - if (!ignore) { - setReposLoaded(true); - } + if (!ignore) setReposLoaded(true); }); return () => { ignore = true; }; - }, [sourceControl]); + }, [sourceControl, isConnectedToForge]); const functions = useMemo( () => @@ -102,7 +139,7 @@ function useFunctionListPage(): { const loaded = reposLoaded && clusterLoaded; const onEdit = (name: string) => navigate(`/faas/edit/${name}`); - return { functions, loaded, onEdit }; + return { functions, loaded, onEdit, isConnectedToForge }; } function parseNamespaceAndRuntime( 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' }],