-
Notifications
You must be signed in to change notification settings - Fork 86
CLI: Implement auth status command
#2028
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
999b2a8
8650b02
fc661e0
c8b228c
9fcc45f
36e6059
a228ae9
b838c0c
4634320
c1799fe
23aa26c
ed4f216
a68c998
49966e3
e13fd34
c72a70d
edb407f
6ba6a05
6946324
2908170
fc816bf
5748109
7b1c743
d8feb6b
20c7566
fdb06df
1cc1fe0
9df3824
3233720
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,43 @@ | ||
| import { __, sprintf } from '@wordpress/i18n'; | ||
| import { AuthCommandLoggerAction as LoggerAction } from 'common/logger-actions'; | ||
| import { getUserInfo } from 'cli/lib/api'; | ||
| import { getAuthToken } from 'cli/lib/appdata'; | ||
| import { Logger, LoggerError } from 'cli/logger'; | ||
| import { StudioArgv } from 'cli/types'; | ||
|
|
||
| export async function runCommand(): Promise< void > { | ||
| const logger = new Logger< LoggerAction >(); | ||
|
|
||
| logger.reportStart( LoggerAction.STATUS_CHECK, __( 'Checking authentication status…' ) ); | ||
| let token: Awaited< ReturnType< typeof getAuthToken > >; | ||
|
|
||
| try { | ||
| token = await getAuthToken(); | ||
| } catch ( error ) { | ||
| logger.reportError( new LoggerError( __( 'Authentication token is invalid or expired' ) ) ); | ||
| return; | ||
| } | ||
|
|
||
| try { | ||
| const userData = await getUserInfo( token.accessToken ); | ||
| logger.reportSuccess( | ||
| sprintf( __( 'Successfully authenticated with WordPress.com as `%s`' ), userData.username ) | ||
| ); | ||
| } catch ( error ) { | ||
| if ( error instanceof LoggerError ) { | ||
| logger.reportError( error ); | ||
| } else { | ||
| logger.reportError( new LoggerError( __( 'Failed to check authentication status' ), error ) ); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| export const registerCommand = ( yargs: StudioArgv ) => { | ||
| return yargs.command( { | ||
| command: 'status', | ||
| describe: __( 'Check authentication status' ), | ||
| handler: async () => { | ||
| await runCommand(); | ||
| }, | ||
| } ); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,87 @@ | ||
| import { getUserInfo } from 'cli/lib/api'; | ||
| import { getAuthToken } from 'cli/lib/appdata'; | ||
| import { Logger, LoggerError } from 'cli/logger'; | ||
|
|
||
| jest.mock( 'cli/lib/api' ); | ||
| jest.mock( 'cli/lib/appdata' ); | ||
| jest.mock( 'cli/logger' ); | ||
|
|
||
| describe( 'Auth Status Command', () => { | ||
| const mockToken = { | ||
| accessToken: 'existing-token', | ||
| id: 999, | ||
| email: 'existing@example.com', | ||
| displayName: 'Existing User', | ||
| expiresIn: 1209600, | ||
| expirationTime: Date.now() + 1209600000, | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Testing - Edge Case Coverage - Great test coverage overall! Consider adding one more test case for better completeness: it( 'should report error when token is expired (past expirationTime)', async () => {
const expiredToken = {
...mockToken,
expirationTime: Date.now() - 1000, // Expired 1 second ago
};
( getAuthToken as jest.Mock ).mockRejectedValue( new Error( 'Token expired' ) );
const { runCommand } = await import( '../status' );
await runCommand();
expect( mockLogger.reportError ).toHaveBeenCalledWith( expect.any( LoggerError ) );
expect( getUserInfo ).not.toHaveBeenCalled();
} );This explicitly tests the expiration time check logic that's in |
||
| }; | ||
| const mockUserData = { | ||
| username: 'testuser', | ||
| }; | ||
|
|
||
| let mockLogger: { | ||
| reportStart: jest.Mock; | ||
| reportSuccess: jest.Mock; | ||
| reportError: jest.Mock; | ||
| }; | ||
|
|
||
| beforeEach( () => { | ||
| jest.clearAllMocks(); | ||
|
|
||
| mockLogger = { | ||
| reportStart: jest.fn(), | ||
| reportSuccess: jest.fn(), | ||
| reportError: jest.fn(), | ||
| }; | ||
|
|
||
| ( Logger as unknown as jest.Mock ).mockReturnValue( mockLogger ); | ||
| ( getAuthToken as jest.Mock ).mockResolvedValue( mockToken ); | ||
| ( getUserInfo as jest.Mock ).mockResolvedValue( mockUserData ); | ||
| } ); | ||
|
|
||
| afterEach( () => { | ||
| jest.restoreAllMocks(); | ||
| } ); | ||
|
|
||
| it( 'should report success when authenticated', async () => { | ||
| const { runCommand } = await import( '../status' ); | ||
| await runCommand(); | ||
|
|
||
| expect( mockLogger.reportStart ).toHaveBeenCalled(); | ||
| expect( getAuthToken ).toHaveBeenCalled(); | ||
| expect( getUserInfo ).toHaveBeenCalledWith( mockToken.accessToken ); | ||
| expect( mockLogger.reportSuccess ).toHaveBeenCalledWith( | ||
| expect.stringContaining( 'Successfully authenticated with WordPress.com as `testuser`' ) | ||
| ); | ||
| } ); | ||
|
|
||
| it( 'should report error when token is invalid', async () => { | ||
| ( getAuthToken as jest.Mock ).mockRejectedValue( new Error( 'Token error' ) ); | ||
|
|
||
| const { runCommand } = await import( '../status' ); | ||
| await runCommand(); | ||
|
|
||
| expect( mockLogger.reportError ).toHaveBeenCalled(); | ||
| expect( mockLogger.reportError ).toHaveBeenCalledWith( expect.any( LoggerError ) ); | ||
| expect( getUserInfo ).not.toHaveBeenCalled(); | ||
| } ); | ||
|
|
||
| it( 'should forward LoggerError from getUserInfo', async () => { | ||
| const apiError = new LoggerError( 'API error' ); | ||
| ( getUserInfo as jest.Mock ).mockRejectedValue( apiError ); | ||
|
|
||
| const { runCommand } = await import( '../status' ); | ||
| await runCommand(); | ||
|
|
||
| expect( mockLogger.reportError ).toHaveBeenCalledWith( apiError ); | ||
| } ); | ||
|
|
||
| it( 'should wrap unknown error when getUserInfo fails', async () => { | ||
| ( getUserInfo as jest.Mock ).mockRejectedValue( new Error( 'Unknown error' ) ); | ||
|
|
||
| const { runCommand } = await import( '../status' ); | ||
| await runCommand(); | ||
|
|
||
| expect( mockLogger.reportError ).toHaveBeenCalledWith( expect.any( LoggerError ) ); | ||
| } ); | ||
| } ); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -149,6 +149,7 @@ const userResponseSchema = z.object( { | |
| ID: z.number(), | ||
| email: z.string().email(), | ||
| display_name: z.string(), | ||
| username: z.string(), | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Code Reusability & Architecture - I agree with @youknowriad's earlier comment. The This would:
Consider creating |
||
| } ); | ||
|
|
||
| export async function getUserInfo( | ||
|
|
@@ -157,7 +158,7 @@ export async function getUserInfo( | |
| const wpcom = wpcomFactory( token, wpcomXhrRequest ); | ||
| try { | ||
| const rawResponse = await wpcom.req.get( '/me', { | ||
| fields: 'ID,login,email,display_name', | ||
| fields: 'ID,username,email,display_name', | ||
| } ); | ||
| return userResponseSchema.parse( rawResponse ); | ||
| } catch ( error ) { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Style - The type annotation
Awaited< ReturnType< typeof getAuthToken > >is correct but creates an unused variable declaration sincetokenis reassigned in the try block. Consider simplifying:This would eliminate the uninitialized variable and flatten the nesting slightly. However, the current approach with two separate try-catch blocks does make the error handling more explicit, so this is a minor style preference.