-
Notifications
You must be signed in to change notification settings - Fork 1
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 multiple Apollo clients issue #319
Conversation
Reviewer's Guide by SourceryThis pull request addresses an issue with multiple Apollo clients by updating the Apollo client configuration in the ApolloConnection component and modifying the getMemberById function in the MemberDataApiImpl class. The changes improve the handling of authentication and optimize the database query for retrieving member data. Sequence diagram for Apollo client link setupsequenceDiagram
actor User
participant ApolloConnection
participant ApolloClient
participant ApolloLink
participant HttpLink
participant BatchHttpLink
participant setContext
User->>ApolloConnection: Initialize
ApolloConnection->>setContext: Create withToken context
ApolloConnection->>ApolloLink: Split link based on clientName
ApolloLink->>HttpLink: Use countryLink if clientName is 'country'
ApolloLink->>BatchHttpLink: Use httpLink otherwise
ApolloConnection->>ApolloClient: Set link
ApolloClient->>ApolloConnection: Client ready
ApolloConnection->>User: Provide ApolloProvider with client
Updated class diagram for MemberDataApiImplclassDiagram
class MemberDataApiImpl {
- findOneById(memberId: string): Promise<MemberData>
+ findById(memberId: string): Promise<MemberData>
+ getMemberById(memberId: string): Promise<MemberData>
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey @mgupta83 - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider removing the console.log statement in the ApolloConnection component before merging to production.
Here's what I looked at during the review
- 🟡 General issues: 2 issues found
- 🟢 Security: all looks good
- 🟡 Review instructions: 1 issue found
- 🟢 Testing: all looks good
- 🟡 Complexity: 1 issue found
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This pull request fixes an issue with multiple Apollo clients. The changes include updating the
getMemberById
function in theMemberDataApiImpl
class to use thefindById
method instead offindOneById
. Additionally, theApolloConnection
component now properly sets the Apollo client link based on the client name and includes the necessary dependencies.Summary by Sourcery
Fix the multiple Apollo clients issue by properly setting the Apollo client link based on the client name and updating the
getMemberById
function to usefindById
for better data handling.Bug Fixes:
Enhancements:
getMemberById
function in theMemberDataApiImpl
class to use thefindById
method instead offindOneById
for improved data retrieval.