-
Notifications
You must be signed in to change notification settings - Fork 727
Github archive #2691
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
Github archive #2691
Changes from 69 commits
0412c35
5705571
040b95c
d96883f
9c7b4b0
7200b42
164240a
c9fbc20
a5d30d4
41d3353
a5b8aae
2d59559
2377e64
d2371e7
62d222d
edb5c80
aedbb35
84ce248
dffbfac
bdfd13a
a74fad9
f18dd04
b11fc25
ddafb39
45eae0f
189c093
c0b1616
3b1276e
8ed8aac
7ec4541
6639a40
6541967
c1a7b09
66d482a
408c155
2d4e896
bf609e8
4b36d45
c5272f7
9c55d68
416218b
b380ae4
6c00277
a26afcf
fde1f73
f920447
c3c45dd
9acea9d
a8d814c
244da57
3889452
18c305a
cceceda
0851772
ce902ae
323ab88
06a7850
bcf88d1
acbcc05
bc722de
ddbdc1f
d1bb5b6
ce32d7d
7e84fcd
da1ef1b
7e878b2
f03246f
6b5e8c9
fbc9961
6e81949
1e333c1
48b4bdb
ee0b036
e343209
2eb7db7
999b266
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import Permissions from '@/security/permissions' | ||
| import GithubIntegrationService from '@/services/githubIntegrationService' | ||
| import PermissionChecker from '@/services/user/permissionChecker' | ||
|
|
||
| export default async (req, res) => { | ||
| new PermissionChecker(req).validateHas(Permissions.values.integrationEdit) | ||
|
|
||
| const payload = await GithubIntegrationService.getOrgRepos(req.params.org) | ||
| await req.responseHandler.success(req, res, payload) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import Permissions from '@/security/permissions' | ||
| import GithubIntegrationService from '@/services/githubIntegrationService' | ||
| import PermissionChecker from '@/services/user/permissionChecker' | ||
|
|
||
| export default async (req, res) => { | ||
| new PermissionChecker(req).validateHas(Permissions.values.integrationEdit) | ||
|
|
||
| const payload = await GithubIntegrationService.findOrgs(req.query.query) | ||
| await req.responseHandler.success(req, res, payload) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| import Permissions from '@/security/permissions' | ||
| import GithubIntegrationService from '@/services/githubIntegrationService' | ||
| import PermissionChecker from '@/services/user/permissionChecker' | ||
|
|
||
| export default async (req, res) => { | ||
| new PermissionChecker(req).validateHas(Permissions.values.integrationEdit) | ||
|
|
||
| const payload = await new GithubIntegrationService(req).findGithubRepos(req.query.query) | ||
| await req.responseHandler.success(req, res, payload) | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,56 +1,144 @@ | ||
| /* eslint-disable no-continue */ | ||
| import cronGenerator from 'cron-time-generator' | ||
|
|
||
| import { timeout } from '@crowd/common' | ||
| import { getServiceChildLogger } from '@crowd/logging' | ||
|
|
||
| import SequelizeRepository from '../../database/repositories/sequelizeRepository' | ||
| import GithubIntegrationService from '../../services/githubIntegrationService' | ||
| import IntegrationService from '../../services/integrationService' | ||
| import { CrowdJob } from '../../types/jobTypes' | ||
|
|
||
| const log = getServiceChildLogger('refreshGithubRepoSettings') | ||
|
|
||
| const job: CrowdJob = { | ||
| name: 'Refresh Github repo settings', | ||
| // every day | ||
| cronTime: cronGenerator.every(1).days(), | ||
| onTrigger: async () => { | ||
| log.info('Updating Github repo settings.') | ||
| const dbOptions = await SequelizeRepository.getDefaultIRepositoryOptions() | ||
| interface Integration { | ||
| id: string | ||
| tenantId: string | ||
| segmentId: string | ||
| integrationIdentifier: string | ||
| settings: { | ||
| orgs: Array<{ | ||
| name: string | ||
| logo: string | ||
| url: string | ||
| fullSync: boolean | ||
| updatedAt: string | ||
| repos: Array<{ | ||
| name: string | ||
| url: string | ||
| updatedAt: string | ||
| }> | ||
| }> | ||
| } | ||
| } | ||
|
|
||
| interface Integration { | ||
| id: string | ||
| tenantId: string | ||
| integrationIdentifier: string | ||
| } | ||
| export const refreshGithubRepoSettings = async () => { | ||
| log.info('Updating Github repo settings.') | ||
| const dbOptions = await SequelizeRepository.getDefaultIRepositoryOptions() | ||
|
|
||
| const githubIntegrations = await dbOptions.database.sequelize.query( | ||
| `SELECT id, "tenantId", "integrationIdentifier" FROM integrations | ||
| const githubIntegrations = await dbOptions.database.sequelize.query( | ||
| `SELECT id, "tenantId", settings, "segmentId" FROM integrations | ||
| WHERE platform = 'github' AND "deletedAt" IS NULL | ||
| AND "createdAt" < NOW() - INTERVAL '1 day'`, | ||
| ) | ||
| ) | ||
|
Comment on lines
+39
to
+43
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. Fix SQL injection vulnerability The raw SQL query is vulnerable to SQL injection. Use parameterized queries instead. - const githubIntegrations = await dbOptions.database.sequelize.query(
- `SELECT id, "tenantId", settings, "segmentId" FROM integrations
- WHERE platform = 'github' AND "deletedAt" IS NULL
- AND "createdAt" < NOW() - INTERVAL '1 day'`,
- )
+ const githubIntegrations = await dbOptions.database.sequelize.query(
+ `SELECT id, "tenantId", settings, "segmentId" FROM integrations
+ WHERE platform = :platform AND "deletedAt" IS NULL
+ AND "createdAt" < NOW() - INTERVAL '1 day'`,
+ {
+ replacements: { platform: 'github' },
+ type: QueryTypes.SELECT
+ }
+ )
|
||
|
|
||
| for (const integration of githubIntegrations[0] as Integration[]) { | ||
|
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. 🛠️ Refactor suggestion Improve type safety The type assertion - for (const integration of githubIntegrations[0] as Integration[]) {
+ const integrations = githubIntegrations[0];
+ if (!Array.isArray(integrations)) {
+ throw new Error('Expected array of integrations');
+ }
+ for (const integration of integrations as Integration[]) {
+ if (!isValidIntegration(integration)) {
+ log.warn(`Skipping invalid integration: ${integration?.id}`);
+ continue;
+ }Add this type guard function: function isValidIntegration(obj: any): obj is Integration {
return (
obj &&
typeof obj.id === 'string' &&
typeof obj.tenantId === 'string' &&
typeof obj.segmentId === 'string' &&
typeof obj.integrationIdentifier === 'string' &&
obj.settings &&
Array.isArray(obj.settings.orgs)
);
} |
||
| log.info(`Updating repo settings for Github integration: ${integration.id}`) | ||
|
|
||
| try { | ||
| const options = await SequelizeRepository.getDefaultIRepositoryOptions() | ||
| options.currentTenant = { id: integration.tenantId } | ||
| options.currentSegments = [ | ||
| // @ts-ignore | ||
|
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. 🛠️ Refactor suggestion Remove @ts-ignore and fix the type Instead of suppressing the TypeScript error, properly type the segments array. - // @ts-ignore
- {
+ {
+ id: integration.segmentId,
+ } as const
|
||
| { | ||
| id: integration.segmentId, | ||
| }, | ||
| ] | ||
|
|
||
| const integrationService = new IntegrationService(options) | ||
|
|
||
| if (!integration.settings.orgs) { | ||
| log.info(`No orgs found for Github integration: ${integration.id}`) | ||
| continue | ||
| } | ||
|
|
||
| // Get all orgs with fullSync enabled | ||
| const fullSyncOrgs = integration.settings.orgs.filter((org) => org.fullSync) | ||
| const currentRepos = new Set( | ||
| integration.settings.orgs.flatMap((org) => org.repos.map((r) => r.url)), | ||
| ) | ||
| const newRepoMappings: Record<string, string> = {} | ||
|
|
||
| for (const integration of githubIntegrations[0] as Integration[]) { | ||
| log.info(`Updating repo settings for Github integration: ${integration.id}`) | ||
| // Fetch new repos for each org | ||
| for (const org of fullSyncOrgs) { | ||
| log.info(`Fetching repos for org ${org.name} with fullSync enabled`) | ||
| const githubRepos = await GithubIntegrationService.getOrgRepos(org.name) | ||
|
|
||
| try { | ||
| const options = await SequelizeRepository.getDefaultIRepositoryOptions() | ||
| options.currentTenant = { id: integration.tenantId } | ||
| // Find new repos that aren't in current settings | ||
| const newRepos = githubRepos.filter((repo) => !currentRepos.has(repo.url)) | ||
|
|
||
| const integrationService = new IntegrationService(options) | ||
| // newly discovered repos will be mapped to default segment of the integration | ||
| await integrationService.updateGithubIntegrationSettings(integration.integrationIdentifier) | ||
| // Map new repos to the integration's segment | ||
| newRepos.forEach((repo) => { | ||
| newRepoMappings[repo.url] = integration.segmentId | ||
| }) | ||
|
|
||
| log.info(`Successfully updated repo settings for Github integration: ${integration.id}`) | ||
| } catch (err) { | ||
| log.error( | ||
| `Error updating repo settings for Github integration ${integration.id}: ${err.message}`, | ||
| // Update org's repos list directly in the integration settings | ||
| const orgIndex = integration.settings.orgs.findIndex((o) => o.name === org.name) | ||
| if (orgIndex !== -1) { | ||
| integration.settings.orgs[orgIndex].repos = [ | ||
| ...integration.settings.orgs[orgIndex].repos, | ||
| ...newRepos.map((repo) => ({ | ||
| name: repo.name, | ||
| url: repo.url, | ||
| updatedAt: new Date().toISOString(), | ||
| })), | ||
| ] | ||
| integration.settings.orgs[orgIndex].updatedAt = new Date().toISOString() | ||
| } | ||
| } | ||
|
|
||
| if (Object.keys(newRepoMappings).length > 0) { | ||
| log.info(`Found ${Object.keys(newRepoMappings).length} new repos to add`) | ||
|
|
||
| // Update integration with modified settings object | ||
| await integrationService.update(integration.id, { | ||
| settings: integration.settings, | ||
| status: 'in-progress', | ||
| }) | ||
|
|
||
| // Map new repos | ||
| await integrationService.mapGithubRepos( | ||
| integration.id, | ||
| newRepoMappings, | ||
| true, | ||
| // this will fire onboarding only for new repos | ||
| true, | ||
| ) | ||
| } finally { | ||
| await timeout(1000) | ||
|
|
||
| log.info(`Successfully mapped ${Object.keys(newRepoMappings).length} new repos`) | ||
| } | ||
|
|
||
| log.info(`Successfully updated repo settings for Github integration: ${integration.id}`) | ||
| } catch (err) { | ||
| log.error( | ||
| `Error updating repo settings for Github integration ${integration.id}: ${err.message}`, | ||
| ) | ||
| // that's probably a rate limit error, let's sleep for a minute | ||
| await timeout(60000) | ||
| } finally { | ||
| await timeout(10000) | ||
| } | ||
| } | ||
|
|
||
| log.info('Finished updating Github repo settings.') | ||
| log.info('Finished updating Github repo settings.') | ||
| } | ||
|
|
||
| const job: CrowdJob = { | ||
| name: 'Refresh Github repo settings', | ||
| // every day | ||
| cronTime: cronGenerator.every(1).days(), | ||
| onTrigger: async () => { | ||
| await refreshGithubRepoSettings() | ||
| }, | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import { getServiceChildLogger } from '@crowd/logging' | ||
|
|
||
| import { refreshGithubRepoSettings } from '../jobs/refreshGithubRepoSettings' | ||
|
|
||
| const logger = getServiceChildLogger('refreshGithubRepoSettings') | ||
|
|
||
| setImmediate(async () => { | ||
| try { | ||
| const startTime = Date.now() | ||
| logger.info('Starting refresh of Github repo settings') | ||
|
|
||
| await refreshGithubRepoSettings() | ||
|
|
||
| const duration = Date.now() - startTime | ||
| logger.info(`Completed refresh of Github repo settings in ${duration}ms`) | ||
|
|
||
| process.exit(0) | ||
| } catch (error) { | ||
| logger.error(`Error refreshing Github repo settings: ${error.message}`) | ||
| process.exit(1) | ||
| } | ||
| }) |
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 explicit error handling.
While the code uses a response handler, it should explicitly handle potential errors from
mapGithubReposto ensure proper error responses.export default async (req, res) => { new PermissionChecker(req).validateHas(Permissions.values.tenantEdit) - const payload = await new IntegrationService(req).mapGithubRepos(req.params.id, req.body.mapping, true, req.body?.isUpdateTransaction ?? false) - await req.responseHandler.success(req, res, payload) + try { + const payload = await new IntegrationService(req).mapGithubRepos( + req.params.id, + req.body.mapping, + true, + req.body?.isUpdateTransaction ?? false + ) + await req.responseHandler.success(req, res, payload) + } catch (error) { + await req.responseHandler.error(req, res, error) + } }