-
Notifications
You must be signed in to change notification settings - Fork 37
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
20 creating popup js code #374
Changes from all commits
680566f
9297310
bd9f6ea
2d3dd6e
bed915f
fa70b3f
f7e02a5
5859fd5
f46f927
912ab8b
c224559
73b86fe
aab2c35
42040ed
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,52 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const showPopup = (popupData) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!popupData || popupData.length === 0) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
console.warn('No popup data available'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
popupData.forEach(popup => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Create popup container | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const popupContainer = document.createElement('div'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Object.assign(popupContainer.style, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
position: 'fixed', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
bottom: '20px', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
right: '20px', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
backgroundColor: popup.headerBg || '#FFFFFF', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
padding: '20px', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
border: '1px solid #000', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
zIndex: 1000, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Create header | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const header = document.createElement('h2'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
header.textContent = popup.headerText; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
header.style.color = popup.headerTextColor || '#000'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
popupContainer.appendChild(header); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Create content | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const content = document.createElement('div'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
content.innerHTML = popup.contentHtml; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Object.assign(content.style, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
color: popup.fontColor || '#000', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
fontSize: popup.font || '14px', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
popupContainer.appendChild(content); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Create action button | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const actionButton = document.createElement('button'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
actionButton.textContent = popup.actionButtonText || 'Close'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Object.assign(actionButton.style, { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
backgroundColor: popup.actionButtonColor || '#CCC', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
actionButton.addEventListener('click', () => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
document.body.removeChild(popupContainer); // Remove popup when button is clicked | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
popupContainer.appendChild(actionButton); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Append the popup to the document body | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
document.body.appendChild(popupContainer); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
export default showPopup; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+1
to
+51
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mom's spaghetti alert! 🍝 Security and accessibility concerns!
Here's a safer implementation with accessibility support: + import DOMPurify from 'dompurify';
const showPopup = (popupData) => {
if (!popupData || popupData.length === 0) {
console.warn('No popup data available');
return;
}
popupData.forEach(popup => {
const popupContainer = document.createElement('div');
+ popupContainer.setAttribute('role', 'dialog');
+ popupContainer.setAttribute('aria-modal', 'true');
+ popupContainer.setAttribute('aria-labelledby', 'popup-header');
Object.assign(popupContainer.style, {
position: 'fixed',
bottom: '20px',
right: '20px',
backgroundColor: popup.headerBg || '#FFFFFF',
padding: '20px',
border: '1px solid #000',
zIndex: 1000,
});
const header = document.createElement('h2');
+ header.id = 'popup-header';
header.textContent = popup.headerText;
header.style.color = popup.headerTextColor || '#000';
popupContainer.appendChild(header);
const content = document.createElement('div');
- content.innerHTML = popup.contentHtml;
+ content.innerHTML = DOMPurify.sanitize(popup.contentHtml);
Object.assign(content.style, {
color: popup.fontColor || '#000',
fontSize: popup.font || '14px',
});
popupContainer.appendChild(content);
const actionButton = document.createElement('button');
actionButton.textContent = popup.actionButtonText || 'Close';
+ actionButton.setAttribute('aria-label', 'Close popup');
Object.assign(actionButton.style, {
backgroundColor: popup.actionButtonColor || '#CCC',
});
- actionButton.addEventListener('click', () => {
+ const closePopup = () => {
document.body.removeChild(popupContainer);
- });
+ };
+ actionButton.addEventListener('click', closePopup);
+ // Add keyboard support
+ popupContainer.addEventListener('keydown', (e) => {
+ if (e.key === 'Escape') closePopup();
+ });
popupContainer.appendChild(actionButton);
document.body.appendChild(popupContainer);
});
}; Consider creating a shared component library to avoid code duplication between backend/public/scripts and jsAgent. This will help maintain consistency and reduce maintenance overhead. 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,30 @@ | ||||||||||||||||||||||||||||||||||||
<!-- Client-side HTML/JS Snippet to be integrated into their website --> | ||||||||||||||||||||||||||||||||||||
<script> | ||||||||||||||||||||||||||||||||||||
(function() { | ||||||||||||||||||||||||||||||||||||
const apiId = 'YOUR_CLIENT_API_ID'; | ||||||||||||||||||||||||||||||||||||
const apiUrl = `https://onboarding-demo.bluewavelabs.ca/api/onboard`; | ||||||||||||||||||||||||||||||||||||
Comment on lines
+4
to
+5
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Palms are sweaty: Don't hardcode those URLs! 🍝 The API URL and client ID should be configurable parameters. - const apiId = 'YOUR_CLIENT_API_ID';
- const apiUrl = `https://onboarding-demo.bluewavelabs.ca/api/onboard`;
+ const apiId = window.ONBOARDING_API_ID;
+ const apiUrl = window.ONBOARDING_API_URL || 'https://onboarding-demo.bluewavelabs.ca/api/onboard'; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||
fetch(apiUrl, { | ||||||||||||||||||||||||||||||||||||
method: 'POST', | ||||||||||||||||||||||||||||||||||||
headers: { | ||||||||||||||||||||||||||||||||||||
'Content-Type': 'application/json', | ||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||
body: JSON.stringify({ userId: apiId }) | ||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||
.then(response => response.json()) | ||||||||||||||||||||||||||||||||||||
.then(data => { | ||||||||||||||||||||||||||||||||||||
const script = document.createElement('script'); | ||||||||||||||||||||||||||||||||||||
script.src = `https://onboarding-demo.bluewavelabs.ca/api/scripts/popupRenderer.js?apiId=${apiId}`; | ||||||||||||||||||||||||||||||||||||
script.type = 'module'; | ||||||||||||||||||||||||||||||||||||
script.onload = () => { | ||||||||||||||||||||||||||||||||||||
import(`https://onboarding-demo.bluewavelabs.ca/api/scripts/popupRenderer.js?apiId=${apiId}`) | ||||||||||||||||||||||||||||||||||||
.then(module => { | ||||||||||||||||||||||||||||||||||||
module.default(data.popupData); | ||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||
document.head.appendChild(script); | ||||||||||||||||||||||||||||||||||||
Comment on lines
+16
to
+25
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. He's nervous: You're loading the script twice! 🍝 The script is being loaded twice - once via createElement and once via import. This is inefficient and could cause race conditions. - const script = document.createElement('script');
- script.src = `https://onboarding-demo.bluewavelabs.ca/api/scripts/popupRenderer.js?apiId=${apiId}`;
- script.type = 'module';
- script.onload = () => {
- import(`https://onboarding-demo.bluewavelabs.ca/api/scripts/popupRenderer.js?apiId=${apiId}`)
- .then(module => {
- module.default(data.popupData);
- });
- };
- document.head.appendChild(script);
+ import(`https://onboarding-demo.bluewavelabs.ca/api/scripts/popupRenderer.js?apiId=${apiId}`)
+ .then(module => {
+ module.default(data.popupData);
+ })
+ .catch(error => {
+ console.warn('Failed to load popup renderer');
+ }); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
}) | ||||||||||||||||||||||||||||||||||||
.catch(error => console.error('Error fetching onboarding data:', error)); | ||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. But on the surface he looks calm and ready: Secure that error handling! 🍝 Don't expose sensitive error information to the console. - .catch(error => console.error('Error fetching onboarding data:', error));
+ .catch(() => console.warn('Failed to initialize onboarding')); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||
})(); | ||||||||||||||||||||||||||||||||||||
</script> | ||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,12 @@ | ||||||||||||
const bannerService = require("../service/banner.service.js"); | ||||||||||||
const getBannerData = async (req, res) => { | ||||||||||||
try { | ||||||||||||
const { userId } = req.body; | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yo dawg, we need to validate that userId before we wreck ourselves! 🔍 Add input validation to ensure userId is present and in the correct format before proceeding. - const { userId } = req.body;
+ const { userId } = req.body;
+ if (!userId || typeof userId !== 'string') {
+ return res.status(400).json({ error: 'Invalid or missing userId' });
+ } 📝 Committable suggestion
Suggested change
|
||||||||||||
const bannerData = await bannerService.getBannerData(userId); | ||||||||||||
res.status(200).json(bannerData); | ||||||||||||
} catch (error) { | ||||||||||||
res.status(500).json({ error: error.message }); | ||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Careful with them error messages, they're spicier than mom's spaghetti! 🍝 The current error handling might expose sensitive information. Consider using a standardized error response format. - res.status(500).json({ error: error.message });
+ res.status(500).json({ error: 'An error occurred while fetching banner data' });
+ console.error('Banner data error:', error); 📝 Committable suggestion
Suggested change
|
||||||||||||
} | ||||||||||||
}; | ||||||||||||
|
||||||||||||
module.exports = { getBannerData }; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
module.exports = { | ||
// bannerData: require('./bannerData.controller'), | ||
popupData: require('./popupData.controller'), | ||
// tourData: require('./tourData.controller'), | ||
// hintData: require('./hintData.controller') | ||
}; | ||
|
Original file line number | Diff line number | Diff line change | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,13 @@ | ||||||||||||
const popupService = require('../../service/popup.service'); | ||||||||||||
|
||||||||||||
const getPopupData = async (req, res) => { | ||||||||||||
try { | ||||||||||||
const { userId } = req.body; | ||||||||||||
const popupData = await popupService.getPopups(userId); | ||||||||||||
res.status(200).json(popupData); | ||||||||||||
} catch (error) { | ||||||||||||
res.status(500).json({ error: error.message }); | ||||||||||||
} | ||||||||||||
Comment on lines
+9
to
+10
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Let's keep our errors under wraps Returning detailed error messages to the client can reveal sensitive information. Send a generic error message instead and log the error internally. Apply this diff: - res.status(500).json({ error: error.message });
+ console.error(error);
+ res.status(500).json({ error: 'Internal server error' }); 📝 Committable suggestion
Suggested change
|
||||||||||||
}; | ||||||||||||
|
||||||||||||
module.exports = { getPopupData }; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,13 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
const tourService = require('../service/tour.service'); | ||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yo! The import path needs fixing, dawg! 🍝 The service import path doesn't match the project structure. From the Here's the fix: -const tourService = require('../service/tour.service');
+const tourService = require('../../service/tour.service'); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
const getTourData = async (req, res) => { | ||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||
const { userId } = req.body; | ||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Knees weak, arms heavy: Missing input validation! 🍝 There's no validation for the Add validation before processing: + if (!userId) {
+ return res.status(400).json({
+ status: 'error',
+ message: 'userId is required'
+ });
+ }
+ if (typeof userId !== 'string' && typeof userId !== 'number') {
+ return res.status(400).json({
+ status: 'error',
+ message: 'Invalid userId format'
+ });
+ } 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
const tourData = await tourService.getTourData(userId); | ||||||||||||||||||||||||||||||||||||||||||||||
res.status(200).json(tourData); | ||||||||||||||||||||||||||||||||||||||||||||||
} catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||
res.status(500).json({ error: error.message }); | ||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+3
to
+11
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mom's spaghetti moment: GET requests shouldn't use request body! 🍝 Using Here's a better implementation: -const getTourData = async (req, res) => {
+const getTourData = async (req, res) => {
try {
- const { userId } = req.body;
+ const { userId } = req.query;
const tourData = await tourService.getTourData(userId);
res.status(200).json(tourData);
} catch (error) {
- res.status(500).json({ error: error.message });
+ res.status(500).json({
+ status: 'error',
+ message: 'Failed to fetch tour data',
+ error: error.message
+ });
}
}; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||
module.exports = { getTourData }; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,35 @@ | ||||||||||||||||||||||||||||||||
const db = require('../models'); | ||||||||||||||||||||||||||||||||
const { v4: uuidv4 } = require('uuid'); | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// Middleware to validate API ID | ||||||||||||||||||||||||||||||||
const validateApiId = async (req, res, next) => { | ||||||||||||||||||||||||||||||||
const { apiId } = req.query; // Assume API ID is sent in query params | ||||||||||||||||||||||||||||||||
if (!apiId) { | ||||||||||||||||||||||||||||||||
return res.status(400).json({ error: "API ID is required." }); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
Comment on lines
+6
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mom's spaghetti: Sanitize that apiId input! 🍝 The apiId from query params needs sanitization to prevent SQL injection and other attacks. - const { apiId } = req.query;
+ const apiId = String(req.query.apiId).trim().slice(0, 100);
if (!apiId) {
- return res.status(400).json({ error: "API ID is required." });
+ return res.status(400).json({ error: "Invalid request" });
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||
const user = await db.User.findOne({ where: { apiId } }); // API ID must be in User model | ||||||||||||||||||||||||||||||||
if (!user) { | ||||||||||||||||||||||||||||||||
return res.status(403).json({ error: "Invalid API ID." }); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
req.user = user; // Attach the user to the request for future use | ||||||||||||||||||||||||||||||||
next(); | ||||||||||||||||||||||||||||||||
} catch (error) { | ||||||||||||||||||||||||||||||||
return res.status(500).json({ error: "API ID validation failed." }); | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
Comment on lines
+11
to
+21
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Knees weak, arms heavy: Add rate limiting! 🍝 The API validation endpoint needs rate limiting to prevent brute force attacks. Add rate limiting middleware before this function: const rateLimit = require('express-rate-limit');
const apiLimiter = rateLimit({
windowMs: 15 * 60 * 1000, // 15 minutes
max: 100 // limit each IP to 100 requests per windowMs
}); |
||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
// Middleware to generate client ID for each session | ||||||||||||||||||||||||||||||||
const generateClientId = (req, res, next) => { | ||||||||||||||||||||||||||||||||
if (!req.session.clientId) { | ||||||||||||||||||||||||||||||||
req.session.clientId = uuidv4(); // Generate new client ID and store in session | ||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||
next(); | ||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||
Comment on lines
+25
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's vomit on his sweater already: Check session middleware! 🍝 The generateClientId middleware assumes session middleware is configured but doesn't verify its existence. Add session middleware check: const generateClientId = (req, res, next) => {
+ if (!req.session) {
+ return res.status(500).json({ error: "Server configuration error" });
+ }
if (!req.session.clientId) {
req.session.clientId = uuidv4();
}
next();
}; 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||
module.exports = { | ||||||||||||||||||||||||||||||||
validateApiId, | ||||||||||||||||||||||||||||||||
generateClientId | ||||||||||||||||||||||||||||||||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
const express = require("express"); | ||
const { validateApiId, generateClientId } = require("../middleware/onboard.middleware"); | ||
const onboardControllers = require("../controllers/onboard"); | ||
|
||
const router = express.Router(); | ||
|
||
router.use(validateApiId); | ||
router.use(generateClientId); | ||
|
||
// router.get("/banner", onboardControllers.bannerData.getBannerData); | ||
router.get("/popup", onboardControllers.popupData.getPopupData); | ||
// router.get("/tour", onboardControllers.tourData.getTourData); | ||
// router.get("/hint", onboardControllers.hintData.getHintData); | ||
|
||
module.exports = router; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -55,6 +55,24 @@ class PopupService { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async getPopupByApiAndClientId(apiId, clientId) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return await Popup.findAll({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
include: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
model: db.User, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
as: "creator", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
where: { apiId } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
where: { clientId }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
order: [['createdAt', 'DESC']], | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} catch (error) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
throw new Error("Error retrieving popups for the given API and Client ID"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+58
to
+74
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Yo dawg, we need to optimize this query performance! The where condition on the included model might make your palms sweaty with poor performance on large datasets. Consider these improvements:
async getPopupByApiAndClientId(apiId, clientId) {
try {
return await Popup.findAll({
include: [
{
model: db.User,
- as: "creator",
- where: { apiId }
+ as: "creator"
},
],
- where: { clientId },
+ where: {
+ clientId,
+ '$creator.apiId$': apiId
+ },
order: [['createdAt', 'DESC']],
});
} catch (error) {
- throw new Error("Error retrieving popups for the given API and Client ID");
+ throw new Error(`Failed to retrieve popups: ${error.message}`);
}
} 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
async getPopupByUrl(url) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return await Popup.findAll({ where: { url } }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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.
Yo dawg, that pg version ain't right! 🍝
The specified version
^8.13.1
doesn't exist in the npm registry. The latest version in the 8.x series is 8.11.3.Apply this fix:
Also consider removing the caret (^) to prevent unexpected minor version updates that could break your spaghetti code.
📝 Committable suggestion