-
Notifications
You must be signed in to change notification settings - Fork 43
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
Mava customer support #1734
base: dev
Are you sure you want to change the base?
Mava customer support #1734
Conversation
WalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
✅ Deploy Preview for kleros-v2-testnet ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-neo ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for kleros-v2-university ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Code Climate has analyzed commit de304f3 and detected 1 issue on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
Quality Gate failedFailed conditions |
✅ Deploy Preview for kleros-v2-testnet-devtools ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (1)
web/src/context/AtlasProvider.tsx (1)
138-142
: Improve testability by abstracting Mava integration.Direct usage of
window.Mava
makes the code harder to test and maintain.Consider creating a service layer:
// services/mava.ts export interface MavaService { initialize(): void; identifyUser(email: string, walletAddress: string): void; } export class MavaServiceImpl implements MavaService { private initialized = false; initialize(): void { if (!window.Mava || this.initialized) return; try { window.Mava.initialize(); this.initialized = true; } catch (error) { console.error('Failed to initialize Mava:', error); } } identifyUser(email: string, walletAddress: string): void { if (!window.Mava || !this.initialized) return; try { window.Mava.identify({ emailAddress: email, customAttributes: [{ label: "Wallet Address", value: walletAddress }] }); } catch (error) { console.error('Failed to identify user in Mava:', error); } } }This allows for:
- Better testing through dependency injection
- Centralized error handling
- Single responsibility principle
- Easy mocking in tests
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
web/src/context/AtlasProvider.tsx
(1 hunks)web/src/index.html
(1 hunks)
🧰 Additional context used
🪛 Gitleaks
web/src/index.html
19-19: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (3)
web/src/index.html (2)
10-10
: LGTM! Good cleanup.
Removing the keywords meta tag is appropriate as it's no longer relevant for SEO purposes.
13-20
: Verify Mava widget configuration
The script configuration looks standard, but please verify:
- Is "v2" the latest stable version of the widget?
- Are there any recommended CSP (Content Security Policy) headers we should add for mava.app?
- Does the widget require any additional configuration for error tracking or analytics?
Let's check the current CSP configuration:
🧰 Tools
🪛 Gitleaks
19-19: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
web/src/context/AtlasProvider.tsx (1)
138-142
: Consider privacy implications of sharing PII with Mava.
The code shares user email and wallet address with a third-party service without explicit user consent.
Let's check if there's a privacy policy or consent mechanism in place:
Consider:
- Adding a user consent mechanism before initializing Mava
- Documenting data sharing in the privacy policy
- Implementing a data deletion mechanism for GDPR compliance
widget-version="v2" | ||
id="MavaWebChat" | ||
enable-sdk="true" | ||
data-token="1fea31aa0b93836faca36269f324468e08cc26f0298f8d8e6c5b089d0d58eb1c" |
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.
Security concern: Exposed API token
The Mava token is currently exposed in the HTML source, making it visible to anyone who views the page source. If this token provides access to sensitive operations or customer data, it should be handled more securely.
Consider:
- Using environment variables and server-side injection of the token
- Implementing token rotation mechanisms
- Adding appropriate CORS and domain restrictions in the Mava dashboard
Would you like assistance in implementing a more secure token handling solution?
🧰 Tools
🪛 Gitleaks
19-19: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
if (user?.email && window.Mava) { | ||
window.Mava.initialize(); | ||
window.Mava.identify({ emailAddress: user.email }); | ||
window.Mava.identify({ customAttributes: [{ label: "Wallet Address", value: address }] }); | ||
} |
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.
Add type safety for window.Mava integration.
The code assumes window.Mava
is globally available without proper TypeScript declarations. This could lead to runtime errors.
Add a type declaration file (e.g., mava.d.ts
):
interface MavaIdentifyOptions {
emailAddress?: string;
customAttributes?: Array<{
label: string;
value: string;
}>;
}
interface MavaSDK {
initialize(): void;
identify(options: MavaIdentifyOptions): void;
}
declare global {
interface Window {
Mava?: MavaSDK;
}
}
Add error handling for Mava initialization.
The Mava integration lacks error handling which could cause silent failures.
Consider wrapping the Mava calls in a try-catch:
- if (user?.email && window.Mava) {
- window.Mava.initialize();
- window.Mava.identify({ emailAddress: user.email });
- window.Mava.identify({ customAttributes: [{ label: "Wallet Address", value: address }] });
- }
+ if (user?.email && window.Mava) {
+ try {
+ window.Mava.initialize();
+ window.Mava.identify({
+ emailAddress: user.email,
+ customAttributes: [{ label: "Wallet Address", value: address }]
+ });
+ } catch (error) {
+ console.error('Failed to initialize Mava:', error);
+ }
+ }
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (user?.email && window.Mava) { | |
window.Mava.initialize(); | |
window.Mava.identify({ emailAddress: user.email }); | |
window.Mava.identify({ customAttributes: [{ label: "Wallet Address", value: address }] }); | |
} | |
if (user?.email && window.Mava) { | |
try { | |
window.Mava.initialize(); | |
window.Mava.identify({ | |
emailAddress: user.email, | |
customAttributes: [{ label: "Wallet Address", value: address }] | |
}); | |
} catch (error) { | |
console.error('Failed to initialize Mava:', error); | |
} | |
} |
🛠️ Refactor suggestion
Optimize Mava initialization.
Currently, Mava is initialized on every user fetch, which might be inefficient.
Consider moving the initialization to a more appropriate lifecycle:
+ // Track if Mava has been initialized
+ const [isMavaInitialized, setIsMavaInitialized] = useState(false);
+
+ // Initialize Mava once when the component mounts
+ useEffect(() => {
+ if (window.Mava && !isMavaInitialized) {
+ try {
+ window.Mava.initialize();
+ setIsMavaInitialized(true);
+ } catch (error) {
+ console.error('Failed to initialize Mava:', error);
+ }
+ }
+ }, [isMavaInitialized]);
const {
data: user,
isLoading: isFetchingUser,
refetch: refetchUser,
} = useQuery({
queryKey: [`UserSettings`],
enabled: isVerified && !isUndefined(address),
queryFn: async () => {
try {
if (!isVerified || isUndefined(address)) return undefined;
const user = await fetchUser(atlasGqlClient);
- if (user?.email && window.Mava) {
- window.Mava.initialize();
+ if (user?.email && window.Mava && isMavaInitialized) {
window.Mava.identify({
emailAddress: user.email,
customAttributes: [{ label: "Wallet Address", value: address }]
});
}
return user;
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
if (user?.email && window.Mava) { | |
window.Mava.initialize(); | |
window.Mava.identify({ emailAddress: user.email }); | |
window.Mava.identify({ customAttributes: [{ label: "Wallet Address", value: address }] }); | |
} | |
// Track if Mava has been initialized | |
const [isMavaInitialized, setIsMavaInitialized] = useState(false); | |
// Initialize Mava once when the component mounts | |
useEffect(() => { | |
if (window.Mava && !isMavaInitialized) { | |
try { | |
window.Mava.initialize(); | |
setIsMavaInitialized(true); | |
} catch (error) { | |
console.error('Failed to initialize Mava:', error); | |
} | |
} | |
}, [isMavaInitialized]); | |
const { | |
data: user, | |
isLoading: isFetchingUser, | |
refetch: refetchUser, | |
} = useQuery({ | |
queryKey: [`UserSettings`], | |
enabled: isVerified && !isUndefined(address), | |
queryFn: async () => { | |
try { | |
if (!isVerified || isUndefined(address)) return undefined; | |
const user = await fetchUser(atlasGqlClient); | |
if (user?.email && window.Mava && isMavaInitialized) { | |
window.Mava.identify({ | |
emailAddress: user.email, | |
customAttributes: [{ label: "Wallet Address", value: address }] | |
}); | |
} | |
return user; |
PR-Codex overview
This PR introduces user identification and integration with the
Mava
widget in theAtlasProvider
, along with minor adjustments in theindex.html
file to include theMava
script.Detailed summary
web/src/context/AtlasProvider.tsx
:Mava
if the user is verified and has an email.web/src/index.html
:<script>
tag to include theMava
widget.<meta>
tag for keywords to a self-closing format.Summary by CodeRabbit
New Features
Bug Fixes
Chores
<meta>
tag for keywords in the HTML document.