-
Notifications
You must be signed in to change notification settings - Fork 23
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
Update Repo queries to use GQL #2943
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅ ✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #2943 +/- ##
==========================================
+ Coverage 98.35% 98.36% +0.01%
==========================================
Files 888 891 +3
Lines 13162 13266 +104
Branches 3481 3475 -6
==========================================
+ Hits 12945 13049 +104
Misses 213 213
Partials 4 4
... and 26 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found. @@ Coverage Diff @@
## main #2943 +/- ##
==========================================
+ Coverage 98.35% 98.36% +0.01%
==========================================
Files 888 891 +3
Lines 13162 13266 +104
Branches 3435 3545 +110
==========================================
+ Hits 12945 13049 +104
Misses 213 213
Partials 4 4
... and 26 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
✅ All tests successful. No failed tests found @@ Coverage Diff @@
## main #2943 +/- ##
==========================================
+ Coverage 98.35% 98.36% +0.01%
==========================================
Files 888 891 +3
Lines 13162 13266 +104
Branches 3499 3540 +41
==========================================
+ Hits 12945 13049 +104
Misses 213 213
Partials 4 4
... and 26 files with indirect coverage changes
Continue to review full report in Codecov by Sentry.
|
Test Failures Detected: Due to failing tests, we cannot provide coverage reports at this time. ❌ Failed Test Results:Completed 4764 tests with View the full list of failed testsSecretString
|
Bundle ReportChanges will increase total bundle size by 18.88kB ⬆️
|
✅ Deploy preview for gazebo ready!Previews expire after 1 month automatically.
|
Bundle ReportChanges will increase total bundle size by 18.88kB ⬆️
|
@ai-review-prompt-prod |
Currently I only review Open Pull Requests that are not in the draft state. Please try again when your PR is ready to be reviewed. |
@ai-review-prompt-prod |
@ai-review-prompt-prod one more try. (sorry for the PR spam) |
@ai-review-prompt-prod 🙏 |
@ai-review-prompt-prod trying new approach. Ignore this. |
console.log('ASFS') | ||
mutate() | ||
if (isLoading) { | ||
// https://cathalmacdonnacha.com/mocking-error-empty-and-loading-states-with-msw | ||
return res(ctx.status(200), ctx.data(mockResponse), ctx.delay(100)) | ||
} | ||
) | ||
if (failedMutation) { | ||
console.log('SAKF') | ||
return res(ctx.status(200), ctx.data(mockErrorResponse)) |
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.
The console.log statements should be removed as they are not necessary for the test and can clutter the output.
console.log('ASFS') | |
mutate() | |
if (isLoading) { | |
// https://cathalmacdonnacha.com/mocking-error-empty-and-loading-states-with-msw | |
return res(ctx.status(200), ctx.data(mockResponse), ctx.delay(100)) | |
} | |
) | |
if (failedMutation) { | |
console.log('SAKF') | |
return res(ctx.status(200), ctx.data(mockErrorResponse)) | |
deleted console.log('ASFS' | |
deleted console.log('SAKF' | |
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.
Well, we got a suggestion. Wasn't very good though 😬
// https://cathalmacdonnacha.com/mocking-error-empty-and-loading-states-with-msw | ||
return res(ctx.status(200), ctx.data(mockResponse), ctx.delay(100)) |
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.
yooo this is cool
} | ||
|
||
function RepoUploadToken({ uploadToken }) { | ||
const [showModal, setShowModal] = useState(false) | ||
const { regenerateToken, isLoading } = useRegenerateToken() | ||
const { mutate, isLoading } = useRegenerateToken() |
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.
const { mutate, isLoading } = useRegenerateToken() | |
const { mutate, isLoading } = useRegenerateRepoUploadToken() |
And remove function on line 16.
}) | ||
}, | ||
onSuccess: ({ data }) => { | ||
const error = data?.encodeSecretString?.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.
Do you think it's worth validating these responses with zod schemas?
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.
I could see either way - curious your thoughts though. Not sure what we do for other mutations.
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.
looks good, couple comments. Gonna test preview deploy now
disappearAfter: TOAST_DURATION, | ||
}) | ||
} | ||
} |
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.
I was expecting a success toast here. Any reason why not have it?
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.
The repo token text updates in place, so there is visual indication that it succeeded
@ai-review-prompt-prod |
I've reviewed this Pull Request and have no helpful feedback. |
Tested all 3 on staging
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. In 2022 this entity acquired Codecov and as result Sentry is going to need some rights from me in order to utilize my contributions in this PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.