Skip to content
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

Fix contributor website import script #8404

Merged
merged 1 commit into from
Nov 8, 2024
Merged

Conversation

FelixMalfait
Copy link
Member

We had duplicate entries in the database for issueLabels/ prLabels

@FelixMalfait FelixMalfait merged commit b2004fe into main Nov 8, 2024
18 checks passed
@FelixMalfait FelixMalfait deleted the fix-website-scripts branch November 8, 2024 09:57
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Summary

Implemented database conflict handling for issue and PR labels to prevent duplicate entries in the Twenty website's GitHub data sync process.

  • Added onConflictDoNothing option in /packages/twenty-website/src/database/database.ts to handle duplicate entries gracefully
  • Fixed potential null reference errors in /packages/twenty-website/src/github/contributors/get-latest-update.tsx with fallback date
  • Corrected field name from 'pullRequestId' to 'issueId' in /packages/twenty-website/src/github/contributors/save-issues-to-db.tsx
  • Added type safety concern: insertMany function uses any types in database.ts
  • Fixed typo in sync logging message from 'Synching' to 'Syncing'

6 file(s) reviewed, 3 comment(s)
Edit PR Review Bot Settings | Greptile

Comment on lines +13 to +14
? new Date(latestPR[0].updatedAt)
: new Date('2023-01-01');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

style: Consider making '2023-01-01' a configurable constant at the module level since it's used multiple times and represents a business rule

@@ -13,7 +13,7 @@ export const executePartialSync = async () => {
return new Error('No GitHub token provided');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: returning new Error() doesn't handle the error - should throw or propagate it

@@ -14,7 +14,7 @@ export const fetchAndSaveGithubData = async () => {
return new Error('No GitHub token provided');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

logic: returning Error object directly won't work as expected - should throw or return Promise.reject()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant