-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Feat/GitHub provider #624
base: main
Are you sure you want to change the base?
Feat/GitHub provider #624
Conversation
…nd GithubSettingsDto classes with validation decorators to define the structure and validation rules for GitHub provider settings in posts module
…sSettings to include support for GitHub provider settings in the application
…lity and maintainability feat(create.post.dto.ts): add GithubSettingsDto to the list of supported providers for posts
…ocial integration feat(github.provider.ts): implement methods for token refresh, authentication, generating auth URL, fetching discussions, posting discussions, fetching analytics, and posting comments feat(github.provider.ts): add support for GitHub API endpoints to interact with GitHub repositories and discussions
…ngs components feat(frontend): update ShowAllProviders component to include GitHubProvider feat(nestjs-libraries): add GitHubProvider to socialIntegrationList for GitHub integration support refactor(github.provider.ts): refactor GitHubProvider class to improve code readability and maintainability, including renaming variables for clarity, adding missing headers in fetch requests, and restructuring the post method for better organization and error handling.
…ository and Scopes in GitHubPreview component style(github.provider.tsx): comment out Last Synced display in GitHubSettings component to remove unnecessary information from the UI
…t' for better clarity feat(github.provider.tsx): add functionality to fetch repositories from GitHub API and log the fetched repositories or any errors to the console
@sdgsnehal is attempting to deploy a commit to the Listinai Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis pull request introduces GitHub integration across both the front-end and back-end. In the frontend, new React components for GitHub preview and settings have been added, with updates to provider listings and formatting. In the backend, DTOs and type unions are extended to support GitHub-specific settings, and a new GitHubProvider class is implemented with methods for authentication, discussion management, analytics, and commenting. These changes enable GitHub-related features to be utilized in posts and social integrations. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant FE as GitHubSettings
participant IM as IntegrationManager
participant GP as GitHubProvider
participant API as GitHubAPI
User->>FE: Click "Fetch Repositories"
FE->>FE: Call getRepos function
FE->>IM: Request GitHub integration operation
IM->>GP: Invoke authentication/fetch repository flow
GP->>API: Request repository data (with OAuth token)
API-->>GP: Return repository data
GP-->>IM: Forward data
IM-->>FE: Relay repository data
Suggested reviewers
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 11
🧹 Nitpick comments (3)
apps/frontend/src/components/launches/providers/github/github.provider.tsx (2)
7-18
: Remove or implement commented code in GitHubPreview componentThe GitHubPreview component contains commented-out code that appears to be intended for displaying repository and scope information. Either implement this functionality or remove the commented code to maintain cleaner implementation.
const GitHubPreview: FC = () => { const { value } = useIntegration(); const settings = useSettings(); return ( <div> <h2>GitHub Integration Preview</h2> - {/* <p>Connected Repository: {value?.content || 'No repository connected'}</p> - <p>Scopes: {settings?.scopes?.join(', ') || 'Default Scopes'}</p> */} + <p>Connected Repository: {value?.content || 'No repository connected'}</p> + <p>Scopes: {settings?.scopes?.join(', ') || 'Default Scopes'}</p> </div> ); };
34-40
: Improve GitHub settings UI and implement commented functionalityThe current UI is minimal and includes commented-out code. Consider enhancing it with:
- A proper form for repository selection
- Implementation of the "Last Synced" display
- Loading state and error message display
return ( <div> <h2>GitHub Integration Settings</h2> - <button onClick={getRepos}>Fetch Repositories</button> - {/* <p>Last Synced: {date || 'Not Synced'}</p> */} + <button + onClick={getRepos} + disabled={loading} + > + {loading ? 'Fetching...' : 'Fetch Repositories'} + </button> + {error && <p className="text-red-500">{error}</p>} + <p>Last Synced: {date || 'Not Synced'}</p> + {/* Add repository selection form here */} </div> );libraries/nestjs-libraries/src/dtos/posts/providers-settings/github.provider-settings.dto.ts.ts (1)
1-10
: Inconsistent indentation in import statements.The import statements have extra indentation that doesn't match the rest of the file. Also, the filename has a redundant
.ts
extension (github.provider-settings.dto.ts.ts
).Consider fixing the indentation and the filename:
-import { - IsArray, - IsDateString, - IsDefined, - IsOptional, - IsString, - MinLength, - ValidateNested, - } from 'class-validator'; - import { Type } from 'class-transformer'; +import { + IsArray, + IsDateString, + IsDefined, + IsOptional, + IsString, + MinLength, + ValidateNested, +} from 'class-validator'; +import { Type } from 'class-transformer';
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/frontend/public/icons/platforms/github.png
is excluded by!**/*.png
📒 Files selected for processing (7)
apps/frontend/src/components/launches/providers/github/github.provider.tsx
(1 hunks)apps/frontend/src/components/launches/providers/show.all.providers.tsx
(2 hunks)libraries/nestjs-libraries/src/dtos/posts/create.post.dto.ts
(3 hunks)libraries/nestjs-libraries/src/dtos/posts/providers-settings/all.providers.settings.ts
(2 hunks)libraries/nestjs-libraries/src/dtos/posts/providers-settings/github.provider-settings.dto.ts.ts
(1 hunks)libraries/nestjs-libraries/src/integrations/integration.manager.ts
(2 hunks)libraries/nestjs-libraries/src/integrations/social/github.provider.ts
(1 hunks)
🔇 Additional comments (9)
libraries/nestjs-libraries/src/integrations/integration.manager.ts (1)
29-29
: GitHub provider integration looks good!The GitHubProvider has been successfully imported and added to the socialIntegrationList following the established pattern for provider integration. This implementation allows the application to now use GitHub as a social platform integration.
Also applies to: 52-52
libraries/nestjs-libraries/src/dtos/posts/providers-settings/all.providers.settings.ts (1)
17-18
: GitHub settings successfully added to type unionThe GithubSettingsDto has been properly added to the AllProvidersSettings type union, making it available for use in the application.
libraries/nestjs-libraries/src/dtos/posts/create.post.dto.ts (1)
89-89
: GitHub settings successfully added to discriminator subtypesThe GithubSettingsDto has been properly added to the subTypes array with the appropriate name ('github'), enabling proper validation and type discrimination for GitHub settings.
libraries/nestjs-libraries/src/dtos/posts/providers-settings/github.provider-settings.dto.ts.ts (2)
12-20
: LGTM! Good use of validation decorators.The
GithubRepositorySettings
class correctly validates the required repository properties with appropriate decorators.
22-41
: Proper validation and well-structured DTO.The
GithubSettingsDto
class includes appropriate validations for all properties, including nested validation for the repositories array.apps/frontend/src/components/launches/providers/show.all.providers.tsx (3)
24-24
: LGTM! Import for GitHub provider added correctly.The GitHub provider import has been added in a consistent style with the other imports.
50-50
: GitHub provider properly added to the providers list.The GitHub provider entry follows the same pattern as other providers in the list.
53-57
: LGTM! Improved type formatting.The component signature has been reformatted for better readability, with consistent spacing and semicolons.
libraries/nestjs-libraries/src/integrations/social/github.provider.ts (1)
14-21
: LGTM! Good class setup with clear GitHub-specific properties.The class properties are well-defined with appropriate scopes for GitHub API access.
@@ -5,6 +5,7 @@ import { RedditSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/provider | |||
import { PinterestSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/pinterest.dto'; | |||
import { YoutubeSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/youtube.settings.dto'; | |||
import { TikTokDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/tiktok.dto'; | |||
import { GithubSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/github.provider-settings.dto.ts'; |
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.
🛠️ Refactor suggestion
Remove file extension from import path
The .ts
file extension in the import path is unnecessary and may cause issues in certain build environments. TypeScript imports typically don't include file extensions.
-import { GithubSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/github.provider-settings.dto.ts';
+import { GithubSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/github.provider-settings.dto';
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { GithubSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/github.provider-settings.dto.ts'; | |
import { GithubSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/github.provider-settings.dto'; |
@@ -15,6 +26,7 @@ import { TikTokDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settin | |||
import { DiscordDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/discord.dto'; | |||
import { SlackDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/slack.dto'; | |||
import { LemmySettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/lemmy.dto'; | |||
import { GithubSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/github.provider-settings.dto.ts'; |
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.
🛠️ Refactor suggestion
Remove file extension from import path
The .ts
file extension in the import path is unnecessary and may cause issues in certain build environments. TypeScript imports typically don't include file extensions.
-import { GithubSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/github.provider-settings.dto.ts';
+import { GithubSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/github.provider-settings.dto';
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { GithubSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/github.provider-settings.dto.ts'; | |
import { GithubSettingsDto } from '@gitroom/nestjs-libraries/dtos/posts/providers-settings/github.provider-settings.dto'; |
import { FC, useCallback } from 'react'; | ||
import { withProvider } from '@gitroom/frontend/components/launches/providers/high.order.provider'; | ||
import { useSettings } from '@gitroom/frontend/components/launches/helpers/use.values'; | ||
import { useIntegration } from '@gitroom/frontend/components/launches/helpers/use.integration'; | ||
import { useCustomProviderFunction } from '@gitroom/frontend/components/launches/helpers/use.custom.provider.function'; | ||
|
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.
🛠️ Refactor suggestion
Missing import for useState hook
You'll need to import the useState hook for the suggested error handling implementation.
-import { FC, useCallback } from 'react';
+import { FC, useCallback, useState } from 'react';
import { withProvider } from '@gitroom/frontend/components/launches/providers/high.order.provider';
import { useSettings } from '@gitroom/frontend/components/launches/helpers/use.values';
import { useIntegration } from '@gitroom/frontend/components/launches/helpers/use.integration';
import { useCustomProviderFunction } from '@gitroom/frontend/components/launches/helpers/use.custom.provider.function';
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { FC, useCallback } from 'react'; | |
import { withProvider } from '@gitroom/frontend/components/launches/providers/high.order.provider'; | |
import { useSettings } from '@gitroom/frontend/components/launches/helpers/use.values'; | |
import { useIntegration } from '@gitroom/frontend/components/launches/helpers/use.integration'; | |
import { useCustomProviderFunction } from '@gitroom/frontend/components/launches/helpers/use.custom.provider.function'; | |
import { FC, useCallback, useState } from 'react'; | |
import { withProvider } from '@gitroom/frontend/components/launches/providers/high.order.provider'; | |
import { useSettings } from '@gitroom/frontend/components/launches/helpers/use.values'; | |
import { useIntegration } from '@gitroom/frontend/components/launches/helpers/use.integration'; | |
import { useCustomProviderFunction } from '@gitroom/frontend/components/launches/helpers/use.custom.provider.function'; |
const getRepos = useCallback(async () => { | ||
try { | ||
const repos = await customFunc.get('repositories', {}); | ||
console.log('Fetched Repositories:', repos); | ||
} catch (error) { | ||
console.error('Error fetching repositories:', error); | ||
} | ||
}, [customFunc]); |
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.
🛠️ Refactor suggestion
Add error handling UI feedback for repository fetch
The getRepos
function logs errors to the console but doesn't provide any user feedback. Consider adding a state variable to track loading state and display error messages to the user.
const GitHubSettings: FC = () => {
const form = useSettings();
const { date } = useIntegration();
const customFunc = useCustomProviderFunction();
+ const [loading, setLoading] = useState(false);
+ const [error, setError] = useState<string | null>(null);
const getRepos = useCallback(async () => {
+ setLoading(true);
+ setError(null);
try {
const repos = await customFunc.get('repositories', {});
console.log('Fetched Repositories:', repos);
+ // Handle the repos data - e.g., set to state
} catch (error) {
console.error('Error fetching repositories:', error);
+ setError('Failed to fetch repositories. Please try again.');
} finally {
+ setLoading(false);
}
}, [customFunc]);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
const getRepos = useCallback(async () => { | |
try { | |
const repos = await customFunc.get('repositories', {}); | |
console.log('Fetched Repositories:', repos); | |
} catch (error) { | |
console.error('Error fetching repositories:', error); | |
} | |
}, [customFunc]); | |
const GitHubSettings: FC = () => { | |
const form = useSettings(); | |
const { date } = useIntegration(); | |
const customFunc = useCustomProviderFunction(); | |
const [loading, setLoading] = useState(false); | |
const [error, setError] = useState<string | null>(null); | |
const getRepos = useCallback(async () => { | |
setLoading(true); | |
setError(null); | |
try { | |
const repos = await customFunc.get('repositories', {}); | |
console.log('Fetched Repositories:', repos); | |
// Handle the repos data - e.g., set to state | |
} catch (error) { | |
console.error('Error fetching repositories:', error); | |
setError('Failed to fetch repositories. Please try again.'); | |
} finally { | |
setLoading(false); | |
} | |
}, [customFunc]); | |
// The component's return statement and additional code would follow here. | |
}; |
async refreshToken(refresh_token: string): Promise<AuthTokenDetails> { | ||
return { | ||
refreshToken: '', | ||
expiresIn: 0, | ||
accessToken: '', | ||
id: '', | ||
name: '', | ||
picture: '', | ||
username: '', | ||
}; | ||
} |
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.
Incomplete token refresh implementation.
The refreshToken
method returns placeholder values and doesn't implement actual token refresh functionality, which might lead to authentication issues when tokens expire.
Implement proper token refresh logic instead of returning empty values. For GitHub, you may need to use the refresh flow documented in their API.
async fetchDiscussions(accessToken: string, repo: string) { | ||
const { data } = await ( | ||
await this.fetch(`https://api.github.com/repos/${repo}/discussions`, { | ||
headers: { Authorization: `Bearer ${accessToken}` }, | ||
}) | ||
).json(); | ||
return data; | ||
} |
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.
🛠️ Refactor suggestion
Missing error handling in fetchDiscussions method.
The method doesn't handle potential API errors or invalid responses.
Add proper error handling:
async fetchDiscussions(accessToken: string, repo: string) {
- const { data } = await (
- await this.fetch(`https://api.github.com/repos/${repo}/discussions`, {
- headers: { Authorization: `Bearer ${accessToken}` },
- })
- ).json();
- return data;
+ try {
+ const response = await this.fetch(`https://api.github.com/repos/${repo}/discussions`, {
+ headers: { Authorization: `Bearer ${accessToken}` },
+ });
+
+ if (!response.ok) {
+ throw new Error(`Failed to fetch discussions: ${response.statusText}`);
+ }
+
+ const { data } = await response.json();
+ return data || [];
+ } catch (error) {
+ console.error('Error fetching GitHub discussions:', error);
+ throw error;
+ }
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
async fetchDiscussions(accessToken: string, repo: string) { | |
const { data } = await ( | |
await this.fetch(`https://api.github.com/repos/${repo}/discussions`, { | |
headers: { Authorization: `Bearer ${accessToken}` }, | |
}) | |
).json(); | |
return data; | |
} | |
async fetchDiscussions(accessToken: string, repo: string) { | |
try { | |
const response = await this.fetch(`https://api.github.com/repos/${repo}/discussions`, { | |
headers: { Authorization: `Bearer ${accessToken}` }, | |
}); | |
if (!response.ok) { | |
throw new Error(`Failed to fetch discussions: ${response.statusText}`); | |
} | |
const { data } = await response.json(); | |
return data || []; | |
} catch (error) { | |
console.error('Error fetching GitHub discussions:', error); | |
throw error; | |
} | |
} |
async analytics(accessToken: string, repo: string): Promise<AnalyticsData[]> { | ||
const { data } = await ( | ||
await this.fetch( | ||
`https://api.github.com/repos/${repo}/stats/contributors`, | ||
{ | ||
headers: { Authorization: `Bearer ${accessToken}` }, | ||
} | ||
) | ||
).json(); | ||
|
||
return data.map((contributor: any) => ({ | ||
label: contributor.author.login, | ||
percentageChange: 0, | ||
data: contributor.weeks.map((week: any) => ({ | ||
total: week.c, | ||
date: dayjs.unix(week.w).format('YYYY-MM-DD'), | ||
})), | ||
})); | ||
} |
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.
🛠️ Refactor suggestion
Hard-coded percentageChange and missing error handling in analytics method.
The method sets percentageChange to 0 without calculation and lacks error handling.
Improve the implementation:
async analytics(accessToken: string, repo: string): Promise<AnalyticsData[]> {
- const { data } = await (
- await this.fetch(
- `https://api.github.com/repos/${repo}/stats/contributors`,
- {
- headers: { Authorization: `Bearer ${accessToken}` },
- }
- )
- ).json();
+ try {
+ const response = await this.fetch(
+ `https://api.github.com/repos/${repo}/stats/contributors`,
+ {
+ headers: { Authorization: `Bearer ${accessToken}` },
+ }
+ );
+
+ if (!response.ok) {
+ throw new Error(`Failed to fetch analytics: ${response.statusText}`);
+ }
+
+ const { data } = await response.json();
+ if (!data || !Array.isArray(data)) {
+ return [];
+ }
- return data.map((contributor: any) => ({
- label: contributor.author.login,
- percentageChange: 0,
- data: contributor.weeks.map((week: any) => ({
- total: week.c,
- date: dayjs.unix(week.w).format('YYYY-MM-DD'),
- })),
- }));
+ return data.map((contributor: any) => {
+ // Calculate percentage change based on recent weeks data
+ const weeks = contributor.weeks || [];
+ const recentWeeks = weeks.slice(-5);
+ let percentageChange = 0;
+
+ if (recentWeeks.length >= 2) {
+ const currentTotal = recentWeeks[recentWeeks.length - 1].c || 0;
+ const previousTotal = recentWeeks[recentWeeks.length - 2].c || 0;
+
+ if (previousTotal > 0) {
+ percentageChange = ((currentTotal - previousTotal) / previousTotal) * 100;
+ }
+ }
+
+ return {
+ label: contributor.author?.login || 'Unknown',
+ percentageChange,
+ data: (contributor.weeks || []).map((week: any) => ({
+ total: week.c || 0,
+ date: dayjs.unix(week.w).format('YYYY-MM-DD'),
+ })),
+ };
+ });
+ } catch (error) {
+ console.error('Error fetching GitHub analytics:', error);
+ throw error;
+ }
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
async analytics(accessToken: string, repo: string): Promise<AnalyticsData[]> { | |
const { data } = await ( | |
await this.fetch( | |
`https://api.github.com/repos/${repo}/stats/contributors`, | |
{ | |
headers: { Authorization: `Bearer ${accessToken}` }, | |
} | |
) | |
).json(); | |
return data.map((contributor: any) => ({ | |
label: contributor.author.login, | |
percentageChange: 0, | |
data: contributor.weeks.map((week: any) => ({ | |
total: week.c, | |
date: dayjs.unix(week.w).format('YYYY-MM-DD'), | |
})), | |
})); | |
} | |
async analytics(accessToken: string, repo: string): Promise<AnalyticsData[]> { | |
try { | |
const response = await this.fetch( | |
`https://api.github.com/repos/${repo}/stats/contributors`, | |
{ | |
headers: { Authorization: `Bearer ${accessToken}` }, | |
} | |
); | |
if (!response.ok) { | |
throw new Error(`Failed to fetch analytics: ${response.statusText}`); | |
} | |
const { data } = await response.json(); | |
if (!data || !Array.isArray(data)) { | |
return []; | |
} | |
return data.map((contributor: any) => { | |
// Calculate percentage change based on recent weeks data | |
const weeks = contributor.weeks || []; | |
const recentWeeks = weeks.slice(-5); | |
let percentageChange = 0; | |
if (recentWeeks.length >= 2) { | |
const currentTotal = recentWeeks[recentWeeks.length - 1].c || 0; | |
const previousTotal = recentWeeks[recentWeeks.length - 2].c || 0; | |
if (previousTotal > 0) { | |
percentageChange = ((currentTotal - previousTotal) / previousTotal) * 100; | |
} | |
} | |
return { | |
label: contributor.author?.login || 'Unknown', | |
percentageChange, | |
data: (contributor.weeks || []).map((week: any) => ({ | |
total: week.c || 0, | |
date: dayjs.unix(week.w).format('YYYY-MM-DD'), | |
})), | |
}; | |
}); | |
} catch (error) { | |
console.error('Error fetching GitHub analytics:', error); | |
throw error; | |
} | |
} |
async authenticate(params: { code: string; codeVerifier: string }) { | ||
const getAccessToken = await ( | ||
await this.fetch(`https://github.com/login/oauth/access_token`, { | ||
method: 'POST', | ||
headers: { | ||
'Content-Type': 'application/json', | ||
'Accept': 'application/json' | ||
}, | ||
body: JSON.stringify({ | ||
client_id: process.env.GITHUB_CLIENT_ID, | ||
client_secret: process.env.GITHUB_CLIENT_SECRET, | ||
code: params.code, | ||
}), | ||
}) | ||
).json(); | ||
|
||
const { access_token } = getAccessToken; | ||
|
||
const user = await ( | ||
await this.fetch('https://api.github.com/user', { | ||
headers: { Authorization: `Bearer ${access_token}` }, | ||
}) | ||
).json(); | ||
|
||
return { | ||
id: user.id, | ||
name: user.name, | ||
accessToken: access_token, | ||
refreshToken: access_token, | ||
expiresIn: 3600, | ||
picture: user.avatar_url, | ||
username: user.login, | ||
}; | ||
} |
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.
🛠️ Refactor suggestion
Add error handling and ensure token management is correct.
The authenticate
method lacks error handling for API calls and uses the same value for both access and refresh tokens.
Consider adding proper error handling and token management:
async authenticate(params: { code: string; codeVerifier: string }) {
- const getAccessToken = await (
- await this.fetch(`https://github.com/login/oauth/access_token`, {
- method: 'POST',
- headers: {
- 'Content-Type': 'application/json',
- 'Accept': 'application/json'
- },
- body: JSON.stringify({
- client_id: process.env.GITHUB_CLIENT_ID,
- client_secret: process.env.GITHUB_CLIENT_SECRET,
- code: params.code,
- }),
- })
- ).json();
+ try {
+ const response = await this.fetch(`https://github.com/login/oauth/access_token`, {
+ method: 'POST',
+ headers: {
+ 'Content-Type': 'application/json',
+ 'Accept': 'application/json'
+ },
+ body: JSON.stringify({
+ client_id: process.env.GITHUB_CLIENT_ID,
+ client_secret: process.env.GITHUB_CLIENT_SECRET,
+ code: params.code,
+ }),
+ });
+
+ if (!response.ok) {
+ throw new Error(`GitHub authentication failed: ${response.statusText}`);
+ }
+
+ const getAccessToken = await response.json();
const { access_token } = getAccessToken;
+ if (!access_token) {
+ throw new Error('Failed to obtain access token from GitHub');
+ }
- const user = await (
- await this.fetch('https://api.github.com/user', {
- headers: { Authorization: `Bearer ${access_token}` },
- })
- ).json();
+ const userResponse = await this.fetch('https://api.github.com/user', {
+ headers: { Authorization: `Bearer ${access_token}` },
+ });
+
+ if (!userResponse.ok) {
+ throw new Error(`Failed to fetch user data: ${userResponse.statusText}`);
+ }
+
+ const user = await userResponse.json();
return {
id: user.id,
name: user.name,
accessToken: access_token,
- refreshToken: access_token,
+ refreshToken: '', // GitHub doesn't provide refresh tokens for OAuth Apps
expiresIn: 3600,
picture: user.avatar_url,
username: user.login,
};
+ } catch (error) {
+ console.error('GitHub authentication error:', error);
+ throw error;
+ }
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
async authenticate(params: { code: string; codeVerifier: string }) { | |
const getAccessToken = await ( | |
await this.fetch(`https://github.com/login/oauth/access_token`, { | |
method: 'POST', | |
headers: { | |
'Content-Type': 'application/json', | |
'Accept': 'application/json' | |
}, | |
body: JSON.stringify({ | |
client_id: process.env.GITHUB_CLIENT_ID, | |
client_secret: process.env.GITHUB_CLIENT_SECRET, | |
code: params.code, | |
}), | |
}) | |
).json(); | |
const { access_token } = getAccessToken; | |
const user = await ( | |
await this.fetch('https://api.github.com/user', { | |
headers: { Authorization: `Bearer ${access_token}` }, | |
}) | |
).json(); | |
return { | |
id: user.id, | |
name: user.name, | |
accessToken: access_token, | |
refreshToken: access_token, | |
expiresIn: 3600, | |
picture: user.avatar_url, | |
username: user.login, | |
}; | |
} | |
async authenticate(params: { code: string; codeVerifier: string }) { | |
try { | |
const response = await this.fetch(`https://github.com/login/oauth/access_token`, { | |
method: 'POST', | |
headers: { | |
'Content-Type': 'application/json', | |
'Accept': 'application/json' | |
}, | |
body: JSON.stringify({ | |
client_id: process.env.GITHUB_CLIENT_ID, | |
client_secret: process.env.GITHUB_CLIENT_SECRET, | |
code: params.code, | |
}), | |
}); | |
if (!response.ok) { | |
throw new Error(`GitHub authentication failed: ${response.statusText}`); | |
} | |
const getAccessToken = await response.json(); | |
const { access_token } = getAccessToken; | |
if (!access_token) { | |
throw new Error('Failed to obtain access token from GitHub'); | |
} | |
const userResponse = await this.fetch('https://api.github.com/user', { | |
headers: { Authorization: `Bearer ${access_token}` }, | |
}); | |
if (!userResponse.ok) { | |
throw new Error(`Failed to fetch user data: ${userResponse.statusText}`); | |
} | |
const user = await userResponse.json(); | |
return { | |
id: user.id, | |
name: user.name, | |
accessToken: access_token, | |
refreshToken: '', // GitHub doesn't provide refresh tokens for OAuth Apps | |
expiresIn: 3600, | |
picture: user.avatar_url, | |
username: user.login, | |
}; | |
} catch (error) { | |
console.error('GitHub authentication error:', error); | |
throw error; | |
} | |
} |
async post( | ||
id: string, | ||
accessToken: string, | ||
postDetails: PostDetails<any>[], | ||
integration: Integration | ||
): Promise<PostResponse[]> { | ||
const [firstPost, ...comments] = postDetails; | ||
|
||
if (!firstPost) { | ||
throw new Error('No post details provided'); | ||
} | ||
|
||
let finalId = ''; | ||
let finalUrl = ''; | ||
|
||
// Step 1: Create a new discussion | ||
const discussionResponse = await this.postDiscussion( | ||
accessToken, | ||
integration.internalId, | ||
firstPost | ||
); | ||
|
||
finalId = discussionResponse.postId; | ||
finalUrl = discussionResponse.releaseURL; | ||
|
||
const postsArray: PostResponse[] = []; | ||
|
||
// Step 2: Add comments to the discussion | ||
for (const comment of comments) { | ||
const { data } = await ( | ||
await this.fetch( | ||
`https://api.github.com/repos/${integration.internalId}/discussions/${finalId}/comments`, | ||
{ | ||
method: 'POST', | ||
headers: { | ||
Authorization: `Bearer ${accessToken}`, | ||
'Content-Type': 'application/json', | ||
}, | ||
body: JSON.stringify({ | ||
body: comment.message, | ||
}), | ||
} | ||
) | ||
).json(); | ||
|
||
postsArray.push({ | ||
id: comment.id, | ||
postId: data.id, | ||
releaseURL: data.html_url, | ||
status: 'success', | ||
}); | ||
} | ||
|
||
return [ | ||
{ | ||
id: firstPost.id, | ||
postId: finalId, | ||
releaseURL: finalUrl, | ||
status: 'success', | ||
}, | ||
...postsArray, | ||
]; | ||
} |
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.
🛠️ Refactor suggestion
Improve error handling in the post method.
The post method lacks proper error handling for API calls to add comments.
Add error handling for comment posting:
async post(
id: string,
accessToken: string,
postDetails: PostDetails<any>[],
integration: Integration
): Promise<PostResponse[]> {
const [firstPost, ...comments] = postDetails;
if (!firstPost) {
throw new Error('No post details provided');
}
let finalId = '';
let finalUrl = '';
+ try {
// Step 1: Create a new discussion
const discussionResponse = await this.postDiscussion(
accessToken,
integration.internalId,
firstPost
);
+
+ if (discussionResponse.status === 'error') {
+ return [discussionResponse];
+ }
finalId = discussionResponse.postId;
finalUrl = discussionResponse.releaseURL;
const postsArray: PostResponse[] = [];
// Step 2: Add comments to the discussion
for (const comment of comments) {
+ try {
- const { data } = await (
- await this.fetch(
- `https://api.github.com/repos/${integration.internalId}/discussions/${finalId}/comments`,
- {
- method: 'POST',
- headers: {
- Authorization: `Bearer ${accessToken}`,
- 'Content-Type': 'application/json',
- },
- body: JSON.stringify({
- body: comment.message,
- }),
- }
- )
- ).json();
+ const response = await this.fetch(
+ `https://api.github.com/repos/${integration.internalId}/discussions/${finalId}/comments`,
+ {
+ method: 'POST',
+ headers: {
+ Authorization: `Bearer ${accessToken}`,
+ 'Content-Type': 'application/json',
+ },
+ body: JSON.stringify({
+ body: comment.message,
+ }),
+ }
+ );
+
+ if (!response.ok) {
+ throw new Error(`Failed to post comment: ${response.statusText}`);
+ }
+
+ const { data } = await response.json();
postsArray.push({
id: comment.id,
postId: data.id,
releaseURL: data.html_url,
status: 'success',
});
+ } catch (error) {
+ console.error('Error posting comment:', error);
+ postsArray.push({
+ id: comment.id,
+ postId: '',
+ releaseURL: '',
+ status: 'error',
+ error: error.message,
+ });
+ }
}
return [
{
id: firstPost.id,
postId: finalId,
releaseURL: finalUrl,
status: 'success',
},
...postsArray,
];
+ } catch (error) {
+ console.error('Error in GitHub post method:', error);
+ return [{
+ id: firstPost.id,
+ postId: '',
+ releaseURL: '',
+ status: 'error',
+ error: error.message,
+ }];
+ }
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
async post( | |
id: string, | |
accessToken: string, | |
postDetails: PostDetails<any>[], | |
integration: Integration | |
): Promise<PostResponse[]> { | |
const [firstPost, ...comments] = postDetails; | |
if (!firstPost) { | |
throw new Error('No post details provided'); | |
} | |
let finalId = ''; | |
let finalUrl = ''; | |
// Step 1: Create a new discussion | |
const discussionResponse = await this.postDiscussion( | |
accessToken, | |
integration.internalId, | |
firstPost | |
); | |
finalId = discussionResponse.postId; | |
finalUrl = discussionResponse.releaseURL; | |
const postsArray: PostResponse[] = []; | |
// Step 2: Add comments to the discussion | |
for (const comment of comments) { | |
const { data } = await ( | |
await this.fetch( | |
`https://api.github.com/repos/${integration.internalId}/discussions/${finalId}/comments`, | |
{ | |
method: 'POST', | |
headers: { | |
Authorization: `Bearer ${accessToken}`, | |
'Content-Type': 'application/json', | |
}, | |
body: JSON.stringify({ | |
body: comment.message, | |
}), | |
} | |
) | |
).json(); | |
postsArray.push({ | |
id: comment.id, | |
postId: data.id, | |
releaseURL: data.html_url, | |
status: 'success', | |
}); | |
} | |
return [ | |
{ | |
id: firstPost.id, | |
postId: finalId, | |
releaseURL: finalUrl, | |
status: 'success', | |
}, | |
...postsArray, | |
]; | |
} | |
async post( | |
id: string, | |
accessToken: string, | |
postDetails: PostDetails<any>[], | |
integration: Integration | |
): Promise<PostResponse[]> { | |
const [firstPost, ...comments] = postDetails; | |
if (!firstPost) { | |
throw new Error('No post details provided'); | |
} | |
let finalId = ''; | |
let finalUrl = ''; | |
try { | |
// Step 1: Create a new discussion | |
const discussionResponse = await this.postDiscussion( | |
accessToken, | |
integration.internalId, | |
firstPost | |
); | |
if (discussionResponse.status === 'error') { | |
return [discussionResponse]; | |
} | |
finalId = discussionResponse.postId; | |
finalUrl = discussionResponse.releaseURL; | |
const postsArray: PostResponse[] = []; | |
// Step 2: Add comments to the discussion | |
for (const comment of comments) { | |
try { | |
const response = await this.fetch( | |
`https://api.github.com/repos/${integration.internalId}/discussions/${finalId}/comments`, | |
{ | |
method: 'POST', | |
headers: { | |
Authorization: `Bearer ${accessToken}`, | |
'Content-Type': 'application/json', | |
}, | |
body: JSON.stringify({ | |
body: comment.message, | |
}), | |
} | |
); | |
if (!response.ok) { | |
throw new Error(`Failed to post comment: ${response.statusText}`); | |
} | |
const { data } = await response.json(); | |
postsArray.push({ | |
id: comment.id, | |
postId: data.id, | |
releaseURL: data.html_url, | |
status: 'success', | |
}); | |
} catch (error) { | |
console.error('Error posting comment:', error); | |
postsArray.push({ | |
id: comment.id, | |
postId: '', | |
releaseURL: '', | |
status: 'error', | |
error: error.message, | |
}); | |
} | |
} | |
return [ | |
{ | |
id: firstPost.id, | |
postId: finalId, | |
releaseURL: finalUrl, | |
status: 'success', | |
}, | |
...postsArray, | |
]; | |
} catch (error) { | |
console.error('Error in GitHub post method:', error); | |
return [{ | |
id: firstPost.id, | |
postId: '', | |
releaseURL: '', | |
status: 'error', | |
error: error.message, | |
}]; | |
} | |
} |
async postDiscussion( | ||
accessToken: string, | ||
repo: string, | ||
postDetails: PostDetails<any> | ||
): Promise<PostResponse> { | ||
const { message } = postDetails; | ||
const { data } = await ( | ||
await this.fetch(`https://api.github.com/repos/${repo}/discussions`, { | ||
method: 'POST', | ||
headers: { | ||
Authorization: `Bearer ${accessToken}`, | ||
'Content-Type': 'application/json', | ||
}, | ||
body: JSON.stringify({ | ||
//title, | ||
body: message, | ||
}), | ||
}) | ||
).json(); | ||
|
||
return { | ||
id: postDetails.id, | ||
postId: data.id, | ||
releaseURL: data.html_url, | ||
status: 'success', | ||
}; | ||
} |
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.
🛠️ Refactor suggestion
Missing title in postDiscussion and needs error handling.
The method has a commented out title parameter and lacks error handling.
Fix the implementation:
async postDiscussion(
accessToken: string,
repo: string,
postDetails: PostDetails<any>
): Promise<PostResponse> {
- const { message } = postDetails;
+ const { message, title } = postDetails;
+ if (!title) {
+ throw new Error('Title is required to create a GitHub discussion');
+ }
+
+ try {
- const { data } = await (
- await this.fetch(`https://api.github.com/repos/${repo}/discussions`, {
- method: 'POST',
- headers: {
- Authorization: `Bearer ${accessToken}`,
- 'Content-Type': 'application/json',
- },
- body: JSON.stringify({
- //title,
- body: message,
- }),
- })
- ).json();
+ const response = await this.fetch(`https://api.github.com/repos/${repo}/discussions`, {
+ method: 'POST',
+ headers: {
+ Authorization: `Bearer ${accessToken}`,
+ 'Content-Type': 'application/json',
+ },
+ body: JSON.stringify({
+ title,
+ body: message,
+ }),
+ });
+
+ if (!response.ok) {
+ throw new Error(`Failed to create discussion: ${response.statusText}`);
+ }
+
+ const { data } = await response.json();
return {
id: postDetails.id,
postId: data.id,
releaseURL: data.html_url,
status: 'success',
};
+ } catch (error) {
+ console.error('Error posting GitHub discussion:', error);
+ return {
+ id: postDetails.id,
+ postId: '',
+ releaseURL: '',
+ status: 'error',
+ error: error.message,
+ };
+ }
}
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
async postDiscussion( | |
accessToken: string, | |
repo: string, | |
postDetails: PostDetails<any> | |
): Promise<PostResponse> { | |
const { message } = postDetails; | |
const { data } = await ( | |
await this.fetch(`https://api.github.com/repos/${repo}/discussions`, { | |
method: 'POST', | |
headers: { | |
Authorization: `Bearer ${accessToken}`, | |
'Content-Type': 'application/json', | |
}, | |
body: JSON.stringify({ | |
//title, | |
body: message, | |
}), | |
}) | |
).json(); | |
return { | |
id: postDetails.id, | |
postId: data.id, | |
releaseURL: data.html_url, | |
status: 'success', | |
}; | |
} | |
async postDiscussion( | |
accessToken: string, | |
repo: string, | |
postDetails: PostDetails<any> | |
): Promise<PostResponse> { | |
- const { message } = postDetails; | |
+ const { message, title } = postDetails; | |
+ if (!title) { | |
+ throw new Error('Title is required to create a GitHub discussion'); | |
+ } | |
+ | |
- const { data } = await ( | |
- await this.fetch(`https://api.github.com/repos/${repo}/discussions`, { | |
- method: 'POST', | |
- headers: { | |
- Authorization: `Bearer ${accessToken}`, | |
- 'Content-Type': 'application/json', | |
- }, | |
- body: JSON.stringify({ | |
- //title, | |
- body: message, | |
- }), | |
- }) | |
- ).json(); | |
+ try { | |
+ const response = await this.fetch(`https://api.github.com/repos/${repo}/discussions`, { | |
+ method: 'POST', | |
+ headers: { | |
+ Authorization: `Bearer ${accessToken}`, | |
+ 'Content-Type': 'application/json', | |
+ }, | |
+ body: JSON.stringify({ | |
+ title, | |
+ body: message, | |
+ }), | |
+ }); | |
+ | |
+ if (!response.ok) { | |
+ throw new Error(`Failed to create discussion: ${response.statusText}`); | |
+ } | |
+ | |
+ const { data } = await response.json(); | |
return { | |
id: postDetails.id, | |
postId: data.id, | |
releaseURL: data.html_url, | |
status: 'success', | |
}; | |
+ } catch (error) { | |
+ console.error('Error posting GitHub discussion:', error); | |
+ return { | |
+ id: postDetails.id, | |
+ postId: '', | |
+ releaseURL: '', | |
+ status: 'error', | |
+ error: error.message, | |
+ }; | |
+ } | |
} |
import { useCustomProviderFunction } from '@gitroom/frontend/components/launches/helpers/use.custom.provider.function'; | ||
|
||
const GitHubPreview: FC = () => { | ||
const { value } = useIntegration(); |
Check warning
Code scanning / ESLint
Disallow unused variables Warning
|
||
const GitHubPreview: FC = () => { | ||
const { value } = useIntegration(); | ||
const settings = useSettings(); |
Check warning
Code scanning / ESLint
Disallow unused variables Warning
}; | ||
|
||
const GitHubSettings: FC = () => { | ||
const form = useSettings(); |
Check warning
Code scanning / ESLint
Disallow unused variables Warning
|
||
const GitHubSettings: FC = () => { | ||
const form = useSettings(); | ||
const { date } = useIntegration(); |
Check warning
Code scanning / ESLint
Disallow unused variables Warning
) || { component: null }; | ||
if ( | ||
!ProviderComponent || | ||
integrations.map((p) => p.id).indexOf(selectedProvider?.id!) === -1 |
Check warning
Code scanning / ESLint
Disallow non-null assertions using the `!` postfix operator Warning
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.
LGTM, but I'll still wait for @nevo-david before merging.
What kind of change does this PR introduce?
This PR introduces a new feature by adding support for GitHub integration in the application.This PR introduces a new feature by adding support for GitHub integration in the application.
Why was this change needed?
The GitHubProvider class was introduced to handle authentication, token refresh, and API interactions.
The ShowAllProviders component was updated to include GitHub as a provider.
The create.post.dto.ts file was updated to support GitHub-related settings in posts.
Please link to related issues when possible, and explain WHY you changed things, not WHAT you changed.
Other information:
Not discussed future plans to able to get repo and dicussion and time them
eg: Did you discuss this change with anybody before working on it (not required, but can be a good idea for bigger changes). Any plans for the future, etc?
Checklist:
Put a "X" in the boxes below to indicate you have followed the checklist;
Summary by CodeRabbit