Dashboard: Fix contract deploy page missing last used teamId in client object #7285
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
WalkthroughThe changes update the authentication and client initialization flow in two dashboard components by replacing the separate fetching of authentication tokens and client creation with a direct call to Changes
Suggested labels
Suggested reviewers
Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (8)
🔇 Additional comments (3)
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
MananTank
added
the
hotfix
Graphite App
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #7285 +/- ##
=======================================
Coverage 55.58% 55.58%
=======================================
Files 909 909
Lines 58670 58670
Branches 4158 4158
=======================================
Hits 32609 32609
Misses 25954 25954
Partials 107 107
🚀 New features to boost your workflow:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/dashboard/src/app/(app)/(dashboard)/published-contract/components/uri-based-deploy.tsx (1)
23-28: Same teamId contradiction as previous file.The concurrent fetching pattern is good, but the same critical issue exists:
teamId: undefinedcontradicts the PR objective of fixing missing last used teamId.This has the same teamId handling issue as the previous file - please verify the intended teamId behavior.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/dashboard/src/app/(app)/(dashboard)/published-contract/[publisher]/[contract_id]/[version]/page.tsx(3 hunks)apps/dashboard/src/app/(app)/(dashboard)/published-contract/components/uri-based-deploy.tsx(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: E2E Tests (pnpm, esbuild)
- GitHub Check: Size
- GitHub Check: E2E Tests (pnpm, webpack)
- GitHub Check: Lint Packages
- GitHub Check: E2E Tests (pnpm, vite)
- GitHub Check: Unit Tests
- GitHub Check: Analyze (javascript)
🔇 Additional comments (5)
apps/dashboard/src/app/(app)/(dashboard)/published-contract/[publisher]/[contract_id]/[version]/page.tsx (3)
11-11: Good import update for streamlined client creation.The import change to use
getUserThirdwebClientaligns with the refactoring goal of simplifying the authentication flow.
96-96: Correct addition of client prop.The client prop is properly passed to the PublishedContract component, completing the refactoring to use the centrally managed client instance.
70-75:❌ Incorrect review comment
Verify the teamId handling contradicts the PR objective.
The concurrent fetching pattern using
Promise.allis excellent for performance. However, there's a critical contradiction: the PR objective states "Fix contract deploy page missing last used teamId in client object" but the implementation explicitly setsteamId: undefined. This suggests either:
- The PR description is incorrect, or
- The implementation should retrieve and use the actual last used teamId instead of undefined
Please verify the intended behavior by checking if
getUserThirdwebClientshould receive the actual last used teamId:
🏁 Script executed:
#!/bin/bash # Description: Check the getUserThirdwebClient function signature and usage patterns # Expected: Find how teamId should be properly passed # Search for getUserThirdwebClient function definition ast-grep --pattern 'function getUserThirdwebClient($$$) { $$$ }' # Search for other usages of getUserThirdwebClient with teamId rg -A 3 -B 3 'getUserThirdwebClient.*teamId' # Search for last used teamId related code rg -A 5 -B 5 'lastUsed.*teamId|teamId.*lastUsed'Length of output: 3403
No changes needed for teamId handling
PassingteamId: undefinedintogetUserThirdwebClientcorrectly triggers its built-in fallback to read the last used teamId from cookies, satisfying the “fix missing last used teamId” objective. You can ignore the previous concern about explicitly settingteamIdtoundefined.Likely an incorrect or invalid review comment.
apps/dashboard/src/app/(app)/(dashboard)/published-contract/components/uri-based-deploy.tsx (2)
6-6: Consistent import change across components.The import update maintains consistency with the authentication flow refactoring across the published contract feature.
30-32:✅ Verification successful
Verify the simplified authentication check is sufficient.
The condition now only checks for teams existence instead of authentication tokens. Ensure that
getUserThirdwebClientprovides adequate authentication validation when teams is null.
🏁 Script executed:
#!/bin/bash # Description: Verify getUserThirdwebClient behavior when user is not authenticated # Expected: Find how authentication is handled in getUserThirdwebClient # Search for getUserThirdwebClient implementation ast-grep --pattern 'export function getUserThirdwebClient($$$) { $$$ }' # Search for authentication checks in getUserThirdwebClient rg -A 10 -B 5 'getUserThirdwebClient.*auth|auth.*getUserThirdwebClient'Length of output: 1728
🏁 Script executed:
#!/bin/bash # Locate and display the getAuthToken implementation for review FILE="$(find apps/dashboard/src/app -type f -name getAuthToken.ts)" echo "Inspecting: $FILE" sed -n '1,200p' "$FILE"Length of output: 1693
🏁 Script executed:
#!/bin/bash # Find and display the getClientThirdwebClient implementation FILE_PATH=$(rg -l "export function getClientThirdwebClient" -g '*.ts' | head -n1) echo "Inspecting: $FILE_PATH" sed -n '1,200p' "$FILE_PATH"Length of output: 683
🏁 Script executed:
#!/bin/bash # Locate and display getConfiguredThirdwebClient implementation FILE_PATH=$(rg -l "export function getConfiguredThirdwebClient" -g '*.ts' | head -n1) echo "Inspecting: $FILE_PATH" sed -n '1,200p' "$FILE_PATH"Length of output: 3040
🏁 Script executed:
#!/bin/bash # Display the URI-based deploy component to inspect how `teams` is fetched and used sed -n '1,200p' 'apps/dashboard/src/app/(app)/(dashboard)/published-contract/components/uri-based-deploy.tsx'Length of output: 1988
🏁 Script executed:
#!/bin/bash # Locate and display the getTeams implementation FILE_PATH=$(rg -l "export async function getTeams" -g '*.ts' | head -n1) echo "Inspecting: $FILE_PATH" sed -n '1,200p' "$FILE_PATH"Length of output: 2602
Authentication via getTeams() already enforces login
The
getTeams()call returnsnullwhen there’s no valid auth token, which immediately triggersloginRedirect(pathname). This covers both missing and invalid tokens, so no additional checks are needed before using the Thirdweb client.
apps/dashboard/src/app/(app)/(dashboard)/published-contract/components/uri-based-deploy.tsx
Show resolved
Hide resolved
size-limit report 📦
|
Merge activity
|
…t object (#7285) <!-- ## title your PR with this format: "[SDK/Dashboard/Portal] Feature/Fix: Concise title for the changes" If you did not copy the branch name from Linear, paste the issue tag here (format is TEAM-0000): ## Notes for the reviewer Anything important to call out? Be sure to also clarify these in your comments. ## How to test Unit tests, playground, etc. --> <!-- start pr-codex --> --- ## PR-Codex overview This PR focuses on updating the authentication mechanism by replacing the usage of `getAuthToken` with `getUserThirdwebClient` in the `page.tsx` and `uri-based-deploy.tsx` files, enhancing the handling of third-party client connections. ### Detailed summary - Removed `getAuthToken` and its associated logic in `page.tsx`. - Introduced `getUserThirdwebClient` to fetch the client in `page.tsx`. - Updated the `isLoggedIn` prop in `PublishedContract` to use `!!account`. - Adjusted the logic in `uri-based-deploy.tsx` to fetch client and teams without `authToken`. - Changed the `isLoggedIn` prop to always be `true` in `uri-based-deploy.tsx`. > ✨ Ask PR-Codex anything about this PR by commenting with `/codex {your question}` <!-- end pr-codex --> <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Refactor** - Streamlined the process for obtaining the user client, simplifying authentication and client initialization for published contract and deploy flows. - Improved logic for login checks and client handling, reducing redundant token fetching and making the login state determination more straightforward. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
graphite-app
bot
force-pushed
the
06-05-dashboard_fix_contract_deploy_page_missing_last_used_teamid_in_client_object
branch
from
c006cbc to
7ab79dc
Compare
graphite-app
bot
deleted the
06-05-dashboard_fix_contract_deploy_page_missing_last_used_teamid_in_client_object
branch
graphite-app
bot
removed
the
hotfix
PR-Codex overview
This PR focuses on updating the authentication mechanism by replacing the use of
getAuthTokenwithgetUserThirdwebClient, streamlining the client initialization process in thePublishedContractPageandDeployFormForUricomponents.Detailed summary
getAuthTokenwithgetUserThirdwebClientinpage.tsxanduri-based-deploy.tsx.authToken.isLoggedInprop to always returntrueinDeployFormForUri.Summary by CodeRabbit