-
Notifications
You must be signed in to change notification settings - Fork 0
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
Epik/hs 16 send email refractor handle submit function #183
Epik/hs 16 send email refractor handle submit function #183
Conversation
… of https://github.com/reisene/HulajDusza-serwis into epik/HS-16-send-email-refractor-handle-submit-function
… of https://github.com/reisene/HulajDusza-serwis into epik/HS-16-send-email-refractor-handle-submit-function
|
GitGuardian id | GitGuardian status | Secret | Commit | Filename | |
---|---|---|---|---|---|
13888663 | Triggered | Airtable API Key | 379f90d | public_html/php/send_email.php | View secret |
13888663 | Triggered | Airtable API Key | 852811f | public_html/php/send_email.php | View secret |
13888665 | Triggered | reCAPTCHA Key | 300c5a4 | public_html/php/send_email.php | View secret |
13888665 | Triggered | reCAPTCHA Key | 6f0998a | public_html/php/send_email.php | View secret |
13888665 | Triggered | reCAPTCHA Key | 08d82f6 | public_html/php/send_email.php | View secret |
13888663 | Triggered | Airtable API Key | 08d82f6 | public_html/php/send_email.php | View secret |
🛠 Guidelines to remediate hardcoded secrets
- Understand the implications of revoking this secret by investigating where it is used in your code.
- Replace and store your secrets safely. Learn here the best practices.
- Revoke and rotate these secrets.
- If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.
To avoid such incidents in the future consider
- following these best practices for managing and storing secrets including API keys and other credentials
- install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.
🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.
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.
Cześć @reisene - Przejrzałem twoje zmiany - oto kilka uwag:
Ogólne uwagi:
- Rozważ internacjonalizację komunikatów o błędach zamiast ich twardego kodowania w języku polskim, aby ułatwić przyszłą lokalizację
Oto, co sprawdziłem podczas przeglądu
- 🟡 Ogólne problemy: znaleziono 1 problem
- 🟢 Bezpieczeństwo: wszystko wygląda dobrze
- 🟢 Testowanie: wszystko wygląda dobrze
- 🟢 Złożoność: wszystko wygląda dobrze
- 🟢 Dokumentacja: wszystko wygląda dobrze
Sourcery jest darmowe dla open source - jeśli podobają Ci się nasze recenzje, rozważ ich udostępnienie ✨
Original comment in English
Hey @reisene - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider internationalizing error messages instead of hardcoding them in Polish to make future localization easier
Here's what I looked at during the review
- 🟡 General issues: 1 issue found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 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.
if (type === 'error') { | ||
Sentry.captureException(new Error(message)); | ||
} |
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.
sugestia: Ulepsz raportowanie błędów poprzez dodanie dodatkowego kontekstu
Dodaj więcej kontekstu w raporcie błędów Sentry, takich jak typ powiadomienia, znacznik czasu i wszelkie istotne informacje o stanie, aby ułatwić debugowanie.
if (type === 'error') { | |
Sentry.captureException(new Error(message)); | |
} | |
if (type === 'error') { | |
// Add context for error tracking | |
Sentry.setContext("notification", { | |
type: type, | |
timestamp: new Date().toISOString(), | |
message: message, | |
elementExists: { | |
notification: !!notification, | |
notificationMessage: !!notificationMessage | |
} | |
}); | |
const error = new Error('Notification Error'); | |
error.message = message; | |
Sentry.captureException(error); | |
} |
Original comment in English
suggestion: Enhance error reporting with additional context
Include more context in the Sentry error report such as the notification type, timestamp, and any relevant state information to help with debugging.
if (type === 'error') { | |
Sentry.captureException(new Error(message)); | |
} | |
if (type === 'error') { | |
// Add context for error tracking | |
Sentry.setContext("notification", { | |
type: type, | |
timestamp: new Date().toISOString(), | |
message: message, | |
elementExists: { | |
notification: !!notification, | |
notificationMessage: !!notificationMessage | |
} | |
}); | |
const error = new Error('Notification Error'); | |
error.message = message; | |
Sentry.captureException(error); | |
} |
}; | ||
|
||
export default displayNotification; | ||
function displayNotification(e,t){let i=document.getElementById("notification");var n=document.getElementById("notification-message");i&&n?(i.classList.add(t,"show"),n.textContent=e,i.setAttribute("aria-live","assertive"),"error"===t&&Sentry.captureException(new Error(e)),setTimeout(()=>{i.classList.remove("show","success","error")},5e3)):(console.error("Elementy notification lub notificationMessage nie istnieją"),Sentry.captureException(new Error("Elementy notification lub notificationMessage nie istnieją")))}export default displayNotification; |
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.
problem (jakość kodu): Użyj const
lub let
zamiast var
. (avoid-using-var
)
Wyjaśnienie
`const` jest preferowane, ponieważ zapewnia, że nie można ponownie przypisać referencji (co może prowadzić do błędnego i mylącego kodu). `let` może być używane, jeśli potrzebujesz ponownie przypisać referencje - jest preferowane w stosunku do `var`, ponieważ jest blokowe, a nie funkcjonalne.Original comment in English
issue (code-quality): Use const
or let
instead of var
. (avoid-using-var
)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
@@ -0,0 +1,2 @@ | |||
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(3<=n.length){let t="";for(let e=0;e<n.length;e++)0<e&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; |
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.
problem (jakość kodu): Użyj const
lub let
zamiast var
. (avoid-using-var
)
Wyjaśnienie
`const` jest preferowane, ponieważ zapewnia, że nie można ponownie przypisać referencji (co może prowadzić do błędnego i mylącego kodu). `let` może być używane, jeśli potrzebujesz ponownie przypisać referencje - jest preferowane w stosunku do `var`, ponieważ jest blokowe, a nie funkcjonalne.Original comment in English
issue (code-quality): Use const
or let
instead of var
. (avoid-using-var
)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
@@ -0,0 +1,2 @@ | |||
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(3<=n.length){let t="";for(let e=0;e<n.length;e++)0<e&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; |
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.
sugestia (jakość kodu): Przenieś zmienne z prawej strony na lewą stronę porównań (flip-comparison
)
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(3<=n.length){let t="";for(let e=0;e<n.length;e++)0<e&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; | |
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(n.length >= 3){let t="";for(let e=0;e<n.length;e++)0<e&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; |
Wyjaśnienie
Jest to konwencja kodowania, że podczas porównań do stałej liczbowej, liczba powinna być po prawej stronie, a zmienna po lewej. Utrzymanie spójności w ten sposób sprawia, że kod jest łatwiejszy do odczytania.Original comment in English
suggestion (code-quality): Moves variables from the right side to the left side of comparisons (flip-comparison
)
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(3<=n.length){let t="";for(let e=0;e<n.length;e++)0<e&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; | |
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(n.length >= 3){let t="";for(let e=0;e<n.length;e++)0<e&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; |
Explanation
It is a coding convention that when doing comparisons to a numeric constant, thenumber should be on the right and the variable on the left. Having things be
consistent in this way makes the code easier to read.
@@ -0,0 +1,2 @@ | |||
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(3<=n.length){let t="";for(let e=0;e<n.length;e++)0<e&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; |
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.
sugestia (jakość kodu): Przenieś zmienne z prawej strony na lewą stronę porównań (flip-comparison
)
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(3<=n.length){let t="";for(let e=0;e<n.length;e++)0<e&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; | |
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(3<=n.length){let t="";for(let e=0;e<n.length;e++)e > 0&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; |
Wyjaśnienie
Jest to konwencja kodowania, że podczas porównań do stałej liczbowej, liczba powinna być po prawej stronie, a zmienna po lewej. Utrzymanie spójności w ten sposób sprawia, że kod jest łatwiejszy do odczytania.Original comment in English
suggestion (code-quality): Moves variables from the right side to the left side of comparisons (flip-comparison
)
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(3<=n.length){let t="";for(let e=0;e<n.length;e++)0<e&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; | |
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(3<=n.length){let t="";for(let e=0;e<n.length;e++)e > 0&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; |
Explanation
It is a coding convention that when doing comparisons to a numeric constant, thenumber should be on the right and the variable on the left. Having things be
consistent in this way makes the code easier to read.
document.querySelector(".a2a_button_sms").style.display = "none"; | ||
} | ||
}); | ||
document.addEventListener("DOMContentLoaded",function(){var t={};t.templates=t.templates||{},t.onclick=!1,t.locale="pl",t.thanks={postShare:!0,ad:!0};let n=document.getElementById("share-buttons"),o=document.getElementById("toggle-share-buttons");o.addEventListener("click",function(){var t;"none"===n.style.display||""===n.style.display?(t=o.getBoundingClientRect(),n.style.left=t.right+0+"px",n.style.display="block"):n.style.display="none"}),document.addEventListener("click",function(t){var e=n.contains(t.target),t=o.contains(t.target);e||t||(n.style.display="none")}),"ontouchstart"in window||navigator.maxTouchPoints?/Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent)&&(document.querySelector(".a2a_button_sms").style.display="block"):document.querySelector(".a2a_button_sms").style.display="none"}); |
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.
problem (jakość kodu): Użyj const
lub let
zamiast var
. (avoid-using-var
)
Wyjaśnienie
`const` jest preferowane, ponieważ zapewnia, że nie można ponownie przypisać referencji (co może prowadzić do błędnego i mylącego kodu). `let` może być używane, jeśli potrzebujesz ponownie przypisać referencje - jest preferowane w stosunku do `var`, ponieważ jest blokowe, a nie funkcjonalne.Original comment in English
issue (code-quality): Use const
or let
instead of var
. (avoid-using-var
)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
@@ -69,0 +1,2 @@ | |||
document.addEventListener("DOMContentLoaded",function(){var t={};t.templates=t.templates||{},t.onclick=!1,t.locale="pl",t.thanks={postShare:!0,ad:!0};let n=document.getElementById("share-buttons"),o=document.getElementById("toggle-share-buttons");o.addEventListener("click",function(){var t;"none"===n.style.display||""===n.style.display?(t=o.getBoundingClientRect(),n.style.left=t.right+0+"px",n.style.display="block"):n.style.display="none"}),document.addEventListener("click",function(t){var e=n.contains(t.target),t=o.contains(t.target);e||t||(n.style.display="none")}),"ontouchstart"in window||navigator.maxTouchPoints?/Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent)&&(document.querySelector(".a2a_button_sms").style.display="block"):document.querySelector(".a2a_button_sms").style.display="none"}); |
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.
problem (jakość kodu): Użyj const
lub let
zamiast var
. (avoid-using-var
)
Wyjaśnienie
`const` jest preferowane, ponieważ zapewnia, że nie można ponownie przypisać referencji (co może prowadzić do błędnego i mylącego kodu). `let` może być używane, jeśli potrzebujesz ponownie przypisać referencje - jest preferowane w stosunku do `var`, ponieważ jest blokowe, a nie funkcjonalne.Original comment in English
issue (code-quality): Use const
or let
instead of var
. (avoid-using-var
)
Explanation
`const` is preferred as it ensures you cannot reassign references (which can lead to buggy and confusing code). `let` may be used if you need to reassign references - it's preferred to `var` because it is block- rather than function-scoped.From the Airbnb JavaScript Style Guide
function animateLoading() { | ||
formButton.classList.remove('submit') | ||
formButton.classList.add('loading'); | ||
formButton.querySelector('.fa-paper-plane').classList.add('hidden'); | ||
formButton.querySelector('.fa-spinner').classList.remove('hidden'); | ||
} |
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.
problem (jakość kodu): Unikaj deklaracji funkcji, preferując wyrażenia przypisania funkcji, wewnątrz bloków. (avoid-function-declarations-in-blocks
)
Wyjaśnienie
Deklaracje funkcji mogą być podnoszone w JavaScript, ale zachowanie jest niejednolite między przeglądarkami. Podnoszenie jest generalnie mylące i powinno być unikane. Zamiast używać deklaracji funkcji wewnątrz bloków, powinieneś używać wyrażeń funkcji, które tworzą funkcje w zasięgu.Original comment in English
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks
)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.function animateButton(type) { | ||
if (type === 'success' || type === 'error') { | ||
formButton.classList.add(type); | ||
|
||
// Resetowanie animacji | ||
formButton.style.animation = 'none'; // Resetuj animację | ||
formButton.offsetHeight; // Wymuś reflow | ||
formButton.style.animation = ''; // Przywróć animację | ||
|
||
if (type === 'success') { | ||
formButton.querySelector('.fa-check').classList.remove('hidden'); | ||
} else { | ||
formButton.querySelector('.fa-times').classList.remove('hidden'); | ||
} | ||
formButton.querySelector('.fa-spinner').classList.add('hidden'); | ||
} | ||
} |
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.
problem (jakość kodu): Unikaj deklaracji funkcji, preferując wyrażenia przypisania funkcji, wewnątrz bloków. (avoid-function-declarations-in-blocks
)
Wyjaśnienie
Deklaracje funkcji mogą być podnoszone w JavaScript, ale zachowanie jest niejednolite między przeglądarkami. Podnoszenie jest generalnie mylące i powinno być unikane. Zamiast używać deklaracji funkcji wewnątrz bloków, powinieneś używać wyrażeń funkcji, które tworzą funkcje w zasięgu.Original comment in English
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks
)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.function resetButton() { | ||
if (formButton) { | ||
formButton.classList.remove('loading', 'success', 'error'); | ||
formButton.classList.add('submit'); | ||
const faCheck = formButton.querySelector('.fa-check'); | ||
const faTimes = formButton.querySelector('.fa-times'); | ||
const faSpinner = formButton.querySelector('.fa-spinner'); | ||
if (faCheck) { | ||
faCheck.classList.add('hidden'); | ||
} | ||
if (faTimes) { | ||
faTimes.classList.add('hidden'); | ||
} | ||
if (faSpinner) { | ||
faSpinner.classList.add('hidden'); | ||
} | ||
const faPaperPlane = formButton.querySelector('.fa-paper-plane'); | ||
faPaperPlane.setAttribute('class', 'svg-inline--fa fa-paper-plane'); | ||
} else { | ||
formButton.querySelector('.fa-times').classList.remove('hidden'); | ||
} | ||
formButton.querySelector('.fa-spinner').classList.add('hidden'); | ||
} | ||
} | ||
|
||
/** | ||
* @description Resets a form button to its default state, removing any loading or | ||
* status indicators and restoring the initial appearance. It also hides associated | ||
* icon elements when applicable. | ||
*/ | ||
function resetButton() { | ||
if (formButton) { | ||
formButton.classList.remove('loading', 'success', 'error'); | ||
formButton.classList.add('submit'); | ||
const faCheck = formButton.querySelector('.fa-check'); | ||
const faTimes = formButton.querySelector('.fa-times'); | ||
if (faCheck) { | ||
faCheck.classList.add('hidden'); | ||
} | ||
if (faTimes) { | ||
faTimes.classList.add('hidden'); | ||
} | ||
const faPaperPlane = formButton.querySelector('.fa-paper-plane'); | ||
faPaperPlane.setAttribute('class', 'svg-inline--fa fa-paper-plane'); | ||
} else { | ||
Sentry.captureException(new Error('formButton is null'), { | ||
extra: { | ||
foo: 'bar', | ||
}, | ||
}); | ||
} | ||
Sentry.captureException(new Error('formButton is null'), { | ||
extra: { | ||
url: window.location.href, | ||
referrer: document.referrer, | ||
userAgent: navigator.userAgent, | ||
formName: 'kontakt', | ||
formAction: window.location.href, | ||
formMethod: 'POST', | ||
}, | ||
}); | ||
} | ||
} |
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.
problem (jakość kodu): Unikaj deklaracji funkcji, preferując wyrażenia przypisania funkcji, wewnątrz bloków. (avoid-function-declarations-in-blocks
)
Wyjaśnienie
Deklaracje funkcji mogą być podnoszone w JavaScript, ale zachowanie jest niejednolite między przeglądarkami. Podnoszenie jest generalnie mylące i powinno być unikane. Zamiast używać deklaracji funkcji wewnątrz bloków, powinieneś używać wyrażeń funkcji, które tworzą funkcje w zasięgu.Original comment in English
issue (code-quality): Avoid function declarations, favouring function assignment expressions, inside blocks. (avoid-function-declarations-in-blocks
)
Explanation
Function declarations may be hoisted in Javascript, but the behaviour is inconsistent between browsers. Hoisting is generally confusing and should be avoided. Rather than using function declarations inside blocks, you should use function expressions, which create functions in-scope.@@ -0,0 +1,3 @@ | |||
module.exports = { | |||
presets: [['@babel/preset-env', {targets: {node: 'current'}}]], |
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 Babel configuration targets the current Node.js version, which is practical for development but might not be suitable for production environments. If the Node.js version differs between development and production, this could lead to code that works in development but fails in production.
Recommendation: Consider specifying a specific Node.js version or range that matches the production environment to ensure consistency across different environments.
}); | ||
const content = `export default [\n${postPaths.map(path => ` '${path}'`).join(',\n')}\n];`; | ||
await fs.writeFile('src/js/modules/post-paths.js', content, 'utf8'); |
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 use of await
inside a forEach
loop is incorrect because forEach
does not handle promises, which can lead to unhandled promises or race conditions. Consider using a for...of
loop or Promise.all
with map
to handle asynchronous file operations correctly.
Recommended Change:
for (const file of files) {
if (file.endsWith('.html') && file !== 'template.html') {
postPaths.push(`/posts/${file}`);
}
}
const content = `export default [
${postPaths.map(path => ` '${path}'`).join(',
')}
];`;
await fs.writeFile('src/js/modules/post-paths.js', content, 'utf8');
@@ -75,7 +83,7 @@ | |||
], gulp.series('html')); | |||
gulp.watch(paths.css, gulp.series('css')); // Monitoruje zmiany w plikach CSS | |||
gulp.watch(paths.js, gulp.series('js')); | |||
gulp.watch(paths.posts, '*.html', gulp.series('watchPosts')); | |||
gulp.watch(paths.posts, '*.html', gulp.series('watchPosts') ); |
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 syntax used in gulp.watch
for monitoring the posts
directory might be incorrect. The method gulp.watch
expects two arguments: the path to watch and the tasks to run. The current implementation might not correctly set up the watcher.
Recommended Change:
gulp.watch([path.join(paths.posts, '*.html')], gulp.series('watchPosts'));
@@ -4,6 +4,7 @@ const { nodeProfilingIntegration } = require("@sentry/profiling-node"); | |||
|
|||
Sentry.init({ | |||
environment: "development", | |||
release: "dev", | |||
dsn: "https://63e2bb53c6fbdeb59c0b5430398260e9@o4507719861075968.ingest.de.sentry.io/4507719884144720", |
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.
Hardcoding the Sentry DSN (Data Source Name) in the source code is a security risk, as it exposes sensitive information that could potentially be accessed by unauthorized parties. Recommendation: Move the DSN to an environment variable or a secure configuration file that is not included in the version control system.
instruments.js
Outdated
profilesSampleRate: 1.0, | ||
}); | ||
}); |
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.
Setting both tracesSampleRate
and profilesSampleRate
to 1.0 means that 100% of transactions and profiles are captured. This can significantly impact the performance and resource usage of your application, especially in a production environment. Recommendation: Consider lowering these rates or implementing a dynamic sampling strategy that adjusts based on the environment and current load.
import displayNotification from"./modules/notification.js";import phoneFormatter from"./modules/phone-format.js";import initButtonAnimation from"./modules/button-animation.js";let form=document.getElementById("my-form"),init=initButtonAnimation();function generateCsfrToken(){fetch("/php/generate-token.php").then(e=>e.text()).then(e=>{localStorage.setItem("csrf_token",e),document.getElementById("csrf-token").value=e}).catch(e=>{var t={message:"Błąd pobierania tokena",url:"/php/generate-token.php",method:"GET"};Sentry.captureException(new Error(t.message),{extra:t}),console.error("Błąd pobierania tokena:",e)})}async function handleSubmit(e,t){if(e.preventDefault(),form){var e={email:document.getElementById("email").value,phone:document.getElementById("phone").value.replace(/\D/g,""),name:document.getElementById("name").value,message:document.getElementById("message").value},r=form.querySelector('button[type="submit"]');r&&(r.disabled=!0);try{await validateFormData(r,e)}catch(e){return void console.error(e)}var o=""+Date.now()+Math.floor(1e6*Math.random()).toString(36),a=document.getElementById("csrf-token").value,n=new FormData,t=(n.append("name",e.name),n.append("email",e.email),n.append("phone",e.phone),n.append("message",e.message),n.append("g-recaptcha-response",t),n.append("uniqueID",o),n.append("csrf_token",a),new File([JSON.stringify(e)],"form_data.txt",{type:"text/plain"}));await sendDataToServer(r,n,t)}else console.error("Form not found."),Sentry.captureException(error,{extra:{url:window.location.href,referrer:document.referrer,userAgent:navigator.userAgent,formName:"kontakt",formAction:window.location.href,formMethod:"POST"}})}async function validateFormData(t,e){try{var r=await(await fetch("../php/validate-data.php",{method:"POST",headers:{"Content-Type":"application/json"},body:JSON.stringify(e)})).json();if(r.error)throw displayNotification(r.error,"error"),setTimeout(()=>{init.resetButton()},200),new Error(r.error);return!0}catch(e){throw t.disabled=!1,e}}async function sendDataToServer(e,t,r){try{var o=await fetch("../php/send_email.php",{method:"POST",body:t});if(!o.ok)throw new Error(`Błąd ${o.status}: `+o.statusText);var a=await o.json();a.success?(displayNotification(a.message,"success"),setTimeout(()=>{init.animateButton("success")},0),setTimeout(()=>{form.reset()},5e3)):(displayNotification(a.message,"error"),setTimeout(()=>{init.animateButton("error")},0))}catch(e){console.error("Form submission error:",e.message,e.stack),handleError(r,e)}finally{e.disabled=!1,setTimeout(()=>{init.resetButton()},5e3)}}function handleError(e,t){Sentry.captureException(t,{attachments:[{filename:"form_data.txt",data:e,contentType:"text/plain"}],tags:{"form-name":"kontakt"},extra:{url:window.location.href,referrer:document.referrer,userAgent:navigator.userAgent,formData:e,formMethod:"POST",formAction:"../php/send_email.php",formError:t.message,formErrorStack:t.stack,formErrorName:t.name,formErrorMessage:t.message}}),t.message.includes("Błąd")?displayNotification("Wystąpił błąd. Proszę spróbować ponownie.","error"):displayNotification("Wystąpił nieoczekiwany błąd. Proszę skontaktować się z administratorem.","error")}document.addEventListener("DOMContentLoaded",()=>{document.getElementById("phone").addEventListener("input",phoneFormatter),generateCsfrToken(),phoneFormatter()}),form.addEventListener("submit",r=>{r.preventDefault(),fetch("../config.json").then(e=>e.text()).then(e=>{let t=JSON.parse(e);grecaptcha.ready(()=>{grecaptcha.execute(t.recaptchaSiteKey,{action:"submit"}).then(e=>{e?handleSubmit(r,e):displayNotification("ReCAPTCHA verification failed. Please try again.","error")}).catch(e=>{console.error("ReCAPTCHA error:",e),displayNotification("ReCAPTCHA verification failed. Please try again.","error")})})}).catch(e=>{console.error("Error loading config:",e)})}); |
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 code on line 1 is overly complex and hard to maintain due to multiple imports and variable declarations on a single line. It is recommended to split this line into multiple lines, each handling a single concern. This will improve readability and make the code easier to maintain. For example:
import displayNotification from "./modules/notification.js";
import phoneFormatter from "./modules/phone-format.js";
import initButtonAnimation from "./modules/button-animation.js";
let form = document.getElementById("my-form");
let init = initButtonAnimation();
Additionally, consider using const
instead of let
for variables that do not need to be reassigned to avoid accidental reassignments.
@@ -138,0 +1,2 @@ | |||
import displayNotification from"./modules/notification.js";import phoneFormatter from"./modules/phone-format.js";import initButtonAnimation from"./modules/button-animation.js";let form=document.getElementById("my-form"),init=initButtonAnimation();function generateCsfrToken(){fetch("/php/generate-token.php").then(e=>e.text()).then(e=>{localStorage.setItem("csrf_token",e),document.getElementById("csrf-token").value=e}).catch(e=>{var t={message:"Błąd pobierania tokena",url:"/php/generate-token.php",method:"GET"};Sentry.captureException(new Error(t.message),{extra:t}),console.error("Błąd pobierania tokena:",e)})}async function handleSubmit(e,t){if(e.preventDefault(),form){var e={email:document.getElementById("email").value,phone:document.getElementById("phone").value.replace(/\D/g,""),name:document.getElementById("name").value,message:document.getElementById("message").value},r=form.querySelector('button[type="submit"]');r&&(r.disabled=!0);try{await validateFormData(r,e)}catch(e){return void console.error(e)}var o=""+Date.now()+Math.floor(1e6*Math.random()).toString(36),a=document.getElementById("csrf-token").value,n=new FormData,t=(n.append("name",e.name),n.append("email",e.email),n.append("phone",e.phone),n.append("message",e.message),n.append("g-recaptcha-response",t),n.append("uniqueID",o),n.append("csrf_token",a),new File([JSON.stringify(e)],"form_data.txt",{type:"text/plain"}));await sendDataToServer(r,n,t)}else console.error("Form not found."),Sentry.captureException(error,{extra:{url:window.location.href,referrer:document.referrer,userAgent:navigator.userAgent,formName:"kontakt",formAction:window.location.href,formMethod:"POST"}})}async function validateFormData(t,e){try{var r=await(await fetch("../php/validate-data.php",{method:"POST",headers:{"Content-Type":"application/json"},body:JSON.stringify(e)})).json();if(r.error)throw displayNotification(r.error,"error"),setTimeout(()=>{init.resetButton()},200),new Error(r.error);return!0}catch(e){throw t.disabled=!1,e}}async function sendDataToServer(e,t,r){try{var o=await fetch("../php/send_email.php",{method:"POST",body:t});if(!o.ok)throw new Error(`Błąd ${o.status}: `+o.statusText);var a=await o.json();a.success?(displayNotification(a.message,"success"),setTimeout(()=>{init.animateButton("success")},0),setTimeout(()=>{form.reset()},5e3)):(displayNotification(a.message,"error"),setTimeout(()=>{init.animateButton("error")},0))}catch(e){console.error("Form submission error:",e.message,e.stack),handleError(r,e)}finally{e.disabled=!1,setTimeout(()=>{init.resetButton()},5e3)}}function handleError(e,t){Sentry.captureException(t,{attachments:[{filename:"form_data.txt",data:e,contentType:"text/plain"}],tags:{"form-name":"kontakt"},extra:{url:window.location.href,referrer:document.referrer,userAgent:navigator.userAgent,formData:e,formMethod:"POST",formAction:"../php/send_email.php",formError:t.message,formErrorStack:t.stack,formErrorName:t.name,formErrorMessage:t.message}}),t.message.includes("Błąd")?displayNotification("Wystąpił błąd. Proszę spróbować ponownie.","error"):displayNotification("Wystąpił nieoczekiwany błąd. Proszę skontaktować się z administratorem.","error")}document.addEventListener("DOMContentLoaded",()=>{document.getElementById("phone").addEventListener("input",phoneFormatter),generateCsfrToken(),phoneFormatter()}),form.addEventListener("submit",r=>{r.preventDefault(),fetch("../config.json").then(e=>e.text()).then(e=>{let t=JSON.parse(e);grecaptcha.ready(()=>{grecaptcha.execute(t.recaptchaSiteKey,{action:"submit"}).then(e=>{e?handleSubmit(r,e):displayNotification("ReCAPTCHA verification failed. Please try again.","error")}).catch(e=>{console.error("ReCAPTCHA error:",e),displayNotification("ReCAPTCHA verification failed. Please try again.","error")})})}).catch(e=>{console.error("Error loading config:",e)})}); |
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.
Storing CSRF tokens in localStorage
can pose a security risk if the environment is susceptible to cross-site scripting (XSS) attacks. It's safer to use HttpOnly
cookies for storing such sensitive information, which JavaScript cannot access, thus providing better protection against XSS. Modify the generateCsfrToken
function to handle CSRF tokens more securely.
let CACHE_NAME="hulajdusza-cache-v1",urlsToCache=["/","/index.html","/about.html","/services.html","/contact.html","/content/cookies.html","/content/pricing.html","/content/privacy.html","/css/style.css","/css/index.css","/css/services.css","/css/about.css","/css/contact.css","/css/pricing.css","/css/privacy.css","/js/script.js","/js/send_email.js","/js/phone_format.js","/js/sharing-buttons.js","/js/counters.js","/icons/favicon-32x32.png","/icons/apple-icon-57x57.png","/img/Logo/HulajDusza_logo.png"],cacheExpirationTime=30;self.addEventListener("install",e=>{e.waitUntil(caches.open(CACHE_NAME).then(e=>e.addAll(urlsToCache)))}),self.addEventListener("fetch",c=>{c.respondWith((async()=>{var e,t,s=await caches.open(CACHE_NAME),a=await s.match(c.request);return a?(e=a.headers.get("date"),t=new Date,(e=new Date(e)).setDate(e.getDate()+cacheExpirationTime),e<t?(e=await fetch(c.request),await s.put(c.request,e.clone()),e):a):(t=await fetch(c.request),await s.put(c.request,t.clone()),t)})())}),self.addEventListener("activate",e=>{e.waitUntil(caches.keys().then(e=>Promise.all(e.map(e=>{if(e!==CACHE_NAME)return caches.delete(e)}))))}),self.addEventListener("push",e=>{e.data?console.log("Received push data:",e.data.text()):console.log("Received empty push data")}),self.addEventListener("notificationclick",e=>{e.notification.close(),e.waitUntil(clients.matchAll().then(e=>{if(e.openWindow)return e.openWindow("/")}))}); |
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 cacheExpirationTime
is set to 30 without specifying the unit of time (e.g., days, hours, minutes). This can lead to confusion or incorrect handling of cache expiration. It is recommended to explicitly define the time unit to ensure clarity and correct functionality.
Suggested Change:
let cacheExpirationTime = 30; // days
Alternatively, you can adjust the unit based on the application's needs (e.g., minutes, hours).
document.querySelector(".a2a_button_sms").style.display = "none"; | ||
} | ||
}); | ||
document.addEventListener("DOMContentLoaded",function(){var t={};t.templates=t.templates||{},t.onclick=!1,t.locale="pl",t.thanks={postShare:!0,ad:!0};let n=document.getElementById("share-buttons"),o=document.getElementById("toggle-share-buttons");o.addEventListener("click",function(){var t;"none"===n.style.display||""===n.style.display?(t=o.getBoundingClientRect(),n.style.left=t.right+0+"px",n.style.display="block"):n.style.display="none"}),document.addEventListener("click",function(t){var e=n.contains(t.target),t=o.contains(t.target);e||t||(n.style.display="none")}),"ontouchstart"in window||navigator.maxTouchPoints?/Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent)&&(document.querySelector(".a2a_button_sms").style.display="block"):document.querySelector(".a2a_button_sms").style.display="none"}); |
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 variable t
is defined globally within the event listener, which can lead to potential conflicts or unintended behavior if other scripts or parts of the application use the same global variable name. Recommendation: Consider encapsulating the variable within a function or using a more unique name to avoid conflicts.
@@ -69,0 +1,2 @@ | |||
document.addEventListener("DOMContentLoaded",function(){var t={};t.templates=t.templates||{},t.onclick=!1,t.locale="pl",t.thanks={postShare:!0,ad:!0};let n=document.getElementById("share-buttons"),o=document.getElementById("toggle-share-buttons");o.addEventListener("click",function(){var t;"none"===n.style.display||""===n.style.display?(t=o.getBoundingClientRect(),n.style.left=t.right+0+"px",n.style.display="block"):n.style.display="none"}),document.addEventListener("click",function(t){var e=n.contains(t.target),t=o.contains(t.target);e||t||(n.style.display="none")}),"ontouchstart"in window||navigator.maxTouchPoints?/Android|webOS|iPhone|iPad|iPod|BlackBerry|IEMobile|Opera Mini/i.test(navigator.userAgent)&&(document.querySelector(".a2a_button_sms").style.display="block"):document.querySelector(".a2a_button_sms").style.display="none"}); |
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 locale is hardcoded to 'pl', which limits the script's reusability in different locales without modifications. Recommendation: Consider making the locale a configurable parameter or detecting it dynamically to enhance the flexibility and usability of the script across different regions.
}); | ||
const content = `export default [\n${postPaths.map(path => ` '${path}'`).join(',\n')}\n];`; | ||
await fs.writeFile('src/js/modules/post-paths.js', content, 'utf8'); |
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.
Potential Performance Issue with File Writing
The watchPosts
task writes to a file every time a post HTML file changes. This could lead to performance issues if the number of post files is large or if changes occur frequently. To optimize, consider debouncing the file write operation or checking if the content has actually changed before writing to the disk.
Suggested Solution:
Implement a debouncing mechanism or a content comparison check before writing to the file to reduce unnecessary disk I/O operations.
@@ -75,7 +83,7 @@ | |||
], gulp.series('html')); | |||
gulp.watch(paths.css, gulp.series('css')); // Monitoruje zmiany w plikach CSS | |||
gulp.watch(paths.js, gulp.series('js')); | |||
gulp.watch(paths.posts, '*.html', gulp.series('watchPosts')); | |||
gulp.watch(paths.posts, '*.html', gulp.series('watchPosts') ); |
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.
Incorrect Watcher Setup for watchPosts
Task
The gulp.watch
setup for watchPosts
seems incorrect. The method signature expects paths and options as the first argument, but here it seems like the intention was to watch HTML files within the paths.posts
directory. This could lead to the task not being triggered as expected.
Suggested Solution:
Correct the watcher setup to properly monitor HTML files within the paths.posts
directory. It should be structured as follows:
gulp.watch([path.join(paths.posts, '*.html')], gulp.series('watchPosts'));
@@ -4,6 +4,7 @@ const { nodeProfilingIntegration } = require("@sentry/profiling-node"); | |||
|
|||
Sentry.init({ | |||
environment: "development", | |||
release: "dev", | |||
dsn: "https://63e2bb53c6fbdeb59c0b5430398260e9@o4507719861075968.ingest.de.sentry.io/4507719884144720", |
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.
Exposing the Sentry DSN (Data Source Name) directly in the source code is a security risk, especially if the codebase is stored in a public repository or shared. Recommendation: Move the DSN to an environment variable or a secure configuration file that is not included in the version control system.
instruments.js
Outdated
profilesSampleRate: 1.0, | ||
}); | ||
}); |
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.
Setting both tracesSampleRate
and profilesSampleRate
to 1.0 means capturing 100% of traces and profiles, which can significantly impact performance, especially under high load or in production environments. Recommendation: Consider lowering these rates or implementing dynamic sampling rates based on the environment or system load.
@@ -316,6 +316,7 @@ <h3>Wyślij wiadomość</h3> | |||
</div> | |||
<label for="message">Wiadomość:<span class="required">*</span></label> | |||
<textarea id="message" name="message" rows="5" placeholder="Jak możemy pomóc?"></textarea> | |||
<input type="hidden" id="csrf-token" name="csrf_token" value=""/> |
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 CSRF token input field is present but does not contain a value (value=""
). This is a security risk as it makes the form vulnerable to CSRF attacks. It is essential to ensure that a valid token is generated and included in the value attribute when the page is rendered. This can typically be handled server-side by generating a unique token for each session or form instance.
Recommendation:
Ensure the server-side code generates a CSRF token and populates the value
attribute of this input field dynamically when the form is loaded.
@@ -179,0 +1,2 @@ | |||
import Menu from"./modules/menu.js";import initStickyHeader from"./modules/sticky-header.js";class App{constructor(){this.init()}init(){this.initAOS(),this.initMenu(),initStickyHeader(),this.initScrollToTop(),this.initIntersectionObserver(),this.initServiceWorker(),this.initTawkTo(),this.initPageTitle()}initAOS(){AOS.init({duration:1200})}initMenu(){(new Menu).init()}initScrollToTop(){let e=$("#scrollToTopBtn");$(".smooth-scroll").click(e=>{e.preventDefault();e=$(e.target).attr("href");$("html, body").animate({scrollToTopBtn:$(e).offset().top},800)}),$(window).scroll(()=>{20<$(window).scrollTop()?e.addClass("show"):e.removeClass("show")}),e.click(()=>{$("html, body").animate({scrollTop:0},"smooth")})}initIntersectionObserver(){var e=$(".fade-in");let t=new IntersectionObserver((e,t)=>{e.forEach(e=>{e.isIntersecting&&($(e.target).addClass("visible"),t.unobserve(e.target))})},{threshold:.25});e.each(e=>{t.observe(e)})}initServiceWorker(){"serviceWorker"in navigator&&window.addEventListener("load",this.registerServiceWorker)}registerServiceWorker(){navigator.serviceWorker.register("/js/service-worker.js").catch(e=>Sentry.captureException(e,{extra:{foo:"bar"}}))}initTawkTo(){var e,t;new Date;e=document.createElement("script"),t=document.getElementsByTagName("script")[0],e.async=!0,e.src="https://embed.tawk.to/66ae13d81601a2195ba058ee/1i4bvr1p7",e.charset="UTF-8",e.setAttribute("crossorigin","*"),t.parentNode.insertBefore(e,t)}initPageTitle(){var e=document.title;document.title=e+" - HulajDusza serwis hulajnóg elektrycznych"}}$(document).ready(()=>{new App}); | |||
//# sourceMappingURL=script.js.map |
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 source map URL is exposed in the production code, which can lead to security risks by revealing the code structure and possibly sensitive information in the source files.
Recommended Solution: Ensure that source maps are only available in the development environment. You can configure your build tools to exclude source maps from production builds or serve them only to authenticated users in a development setting.
import displayNotification from"./modules/notification.js";import phoneFormatter from"./modules/phone-format.js";import initButtonAnimation from"./modules/button-animation.js";let form=document.getElementById("my-form"),init=initButtonAnimation();function generateCsfrToken(){fetch("/php/generate-token.php").then(e=>e.text()).then(e=>{localStorage.setItem("csrf_token",e),document.getElementById("csrf-token").value=e}).catch(e=>{var t={message:"Błąd pobierania tokena",url:"/php/generate-token.php",method:"GET"};Sentry.captureException(new Error(t.message),{extra:t}),console.error("Błąd pobierania tokena:",e)})}async function handleSubmit(e,t){if(e.preventDefault(),form){var e={email:document.getElementById("email").value,phone:document.getElementById("phone").value.replace(/\D/g,""),name:document.getElementById("name").value,message:document.getElementById("message").value},r=form.querySelector('button[type="submit"]');r&&(r.disabled=!0);try{await validateFormData(r,e)}catch(e){return void console.error(e)}var o=""+Date.now()+Math.floor(1e6*Math.random()).toString(36),a=document.getElementById("csrf-token").value,n=new FormData,t=(n.append("name",e.name),n.append("email",e.email),n.append("phone",e.phone),n.append("message",e.message),n.append("g-recaptcha-response",t),n.append("uniqueID",o),n.append("csrf_token",a),new File([JSON.stringify(e)],"form_data.txt",{type:"text/plain"}));await sendDataToServer(r,n,t)}else console.error("Form not found."),Sentry.captureException(error,{extra:{url:window.location.href,referrer:document.referrer,userAgent:navigator.userAgent,formName:"kontakt",formAction:window.location.href,formMethod:"POST"}})}async function validateFormData(t,e){try{var r=await(await fetch("../php/validate-data.php",{method:"POST",headers:{"Content-Type":"application/json"},body:JSON.stringify(e)})).json();if(r.error)throw displayNotification(r.error,"error"),setTimeout(()=>{init.resetButton()},200),new Error(r.error);return!0}catch(e){throw t.disabled=!1,e}}async function sendDataToServer(e,t,r){try{var o=await fetch("../php/send_email.php",{method:"POST",body:t});if(!o.ok)throw new Error(`Błąd ${o.status}: `+o.statusText);var a=await o.json();a.success?(displayNotification(a.message,"success"),setTimeout(()=>{init.animateButton("success")},0),setTimeout(()=>{form.reset()},5e3)):(displayNotification(a.message,"error"),setTimeout(()=>{init.animateButton("error")},0))}catch(e){console.error("Form submission error:",e.message,e.stack),handleError(r,e)}finally{e.disabled=!1,setTimeout(()=>{init.resetButton()},5e3)}}function handleError(e,t){Sentry.captureException(t,{attachments:[{filename:"form_data.txt",data:e,contentType:"text/plain"}],tags:{"form-name":"kontakt"},extra:{url:window.location.href,referrer:document.referrer,userAgent:navigator.userAgent,formData:e,formMethod:"POST",formAction:"../php/send_email.php",formError:t.message,formErrorStack:t.stack,formErrorName:t.name,formErrorMessage:t.message}}),t.message.includes("Błąd")?displayNotification("Wystąpił błąd. Proszę spróbować ponownie.","error"):displayNotification("Wystąpił nieoczekiwany błąd. Proszę skontaktować się z administratorem.","error")}document.addEventListener("DOMContentLoaded",()=>{document.getElementById("phone").addEventListener("input",phoneFormatter),generateCsfrToken(),phoneFormatter()}),form.addEventListener("submit",r=>{r.preventDefault(),fetch("../config.json").then(e=>e.text()).then(e=>{let t=JSON.parse(e);grecaptcha.ready(()=>{grecaptcha.execute(t.recaptchaSiteKey,{action:"submit"}).then(e=>{e?handleSubmit(r,e):displayNotification("ReCAPTCHA verification failed. Please try again.","error")}).catch(e=>{console.error("ReCAPTCHA error:",e),displayNotification("ReCAPTCHA verification failed. Please try again.","error")})})}).catch(e=>{console.error("Error loading config:",e)})}); |
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 code on line 1 is overly complex and difficult to read because it combines imports, variable declarations, and function definitions all in one line. This can make the code hard to maintain and debug. Consider breaking this line into multiple lines, separating imports, variable declarations, and function definitions to enhance readability and maintainability.
Recommended Change:
import displayNotification from "./modules/notification.js";
import phoneFormatter from "./modules/phone-format.js";
import initButtonAnimation from "./modules/button-animation.js";
let form = document.getElementById("my-form");
let init = initButtonAnimation();
function generateCsfrToken() { ... }
@@ -138,0 +1,2 @@ | |||
import displayNotification from"./modules/notification.js";import phoneFormatter from"./modules/phone-format.js";import initButtonAnimation from"./modules/button-animation.js";let form=document.getElementById("my-form"),init=initButtonAnimation();function generateCsfrToken(){fetch("/php/generate-token.php").then(e=>e.text()).then(e=>{localStorage.setItem("csrf_token",e),document.getElementById("csrf-token").value=e}).catch(e=>{var t={message:"Błąd pobierania tokena",url:"/php/generate-token.php",method:"GET"};Sentry.captureException(new Error(t.message),{extra:t}),console.error("Błąd pobierania tokena:",e)})}async function handleSubmit(e,t){if(e.preventDefault(),form){var e={email:document.getElementById("email").value,phone:document.getElementById("phone").value.replace(/\D/g,""),name:document.getElementById("name").value,message:document.getElementById("message").value},r=form.querySelector('button[type="submit"]');r&&(r.disabled=!0);try{await validateFormData(r,e)}catch(e){return void console.error(e)}var o=""+Date.now()+Math.floor(1e6*Math.random()).toString(36),a=document.getElementById("csrf-token").value,n=new FormData,t=(n.append("name",e.name),n.append("email",e.email),n.append("phone",e.phone),n.append("message",e.message),n.append("g-recaptcha-response",t),n.append("uniqueID",o),n.append("csrf_token",a),new File([JSON.stringify(e)],"form_data.txt",{type:"text/plain"}));await sendDataToServer(r,n,t)}else console.error("Form not found."),Sentry.captureException(error,{extra:{url:window.location.href,referrer:document.referrer,userAgent:navigator.userAgent,formName:"kontakt",formAction:window.location.href,formMethod:"POST"}})}async function validateFormData(t,e){try{var r=await(await fetch("../php/validate-data.php",{method:"POST",headers:{"Content-Type":"application/json"},body:JSON.stringify(e)})).json();if(r.error)throw displayNotification(r.error,"error"),setTimeout(()=>{init.resetButton()},200),new Error(r.error);return!0}catch(e){throw t.disabled=!1,e}}async function sendDataToServer(e,t,r){try{var o=await fetch("../php/send_email.php",{method:"POST",body:t});if(!o.ok)throw new Error(`Błąd ${o.status}: `+o.statusText);var a=await o.json();a.success?(displayNotification(a.message,"success"),setTimeout(()=>{init.animateButton("success")},0),setTimeout(()=>{form.reset()},5e3)):(displayNotification(a.message,"error"),setTimeout(()=>{init.animateButton("error")},0))}catch(e){console.error("Form submission error:",e.message,e.stack),handleError(r,e)}finally{e.disabled=!1,setTimeout(()=>{init.resetButton()},5e3)}}function handleError(e,t){Sentry.captureException(t,{attachments:[{filename:"form_data.txt",data:e,contentType:"text/plain"}],tags:{"form-name":"kontakt"},extra:{url:window.location.href,referrer:document.referrer,userAgent:navigator.userAgent,formData:e,formMethod:"POST",formAction:"../php/send_email.php",formError:t.message,formErrorStack:t.stack,formErrorName:t.name,formErrorMessage:t.message}}),t.message.includes("Błąd")?displayNotification("Wystąpił błąd. Proszę spróbować ponownie.","error"):displayNotification("Wystąpił nieoczekiwany błąd. Proszę skontaktować się z administratorem.","error")}document.addEventListener("DOMContentLoaded",()=>{document.getElementById("phone").addEventListener("input",phoneFormatter),generateCsfrToken(),phoneFormatter()}),form.addEventListener("submit",r=>{r.preventDefault(),fetch("../config.json").then(e=>e.text()).then(e=>{let t=JSON.parse(e);grecaptcha.ready(()=>{grecaptcha.execute(t.recaptchaSiteKey,{action:"submit"}).then(e=>{e?handleSubmit(r,e):displayNotification("ReCAPTCHA verification failed. Please try again.","error")}).catch(e=>{console.error("ReCAPTCHA error:",e),displayNotification("ReCAPTCHA verification failed. Please try again.","error")})})}).catch(e=>{console.error("Error loading config:",e)})}); |
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 use of let
for the form
variable suggests that it might be reassigned later in the code, which can lead to bugs if not handled carefully. If the form
variable is not intended to be reassigned, using const
would be more appropriate as it prevents reassignment and makes the code's intent clearer.
Recommended Change:
const form = document.getElementById("my-form");
let CACHE_NAME="hulajdusza-cache-v1",urlsToCache=["/","/index.html","/about.html","/services.html","/contact.html","/content/cookies.html","/content/pricing.html","/content/privacy.html","/css/style.css","/css/index.css","/css/services.css","/css/about.css","/css/contact.css","/css/pricing.css","/css/privacy.css","/js/script.js","/js/send_email.js","/js/phone_format.js","/js/sharing-buttons.js","/js/counters.js","/icons/favicon-32x32.png","/icons/apple-icon-57x57.png","/img/Logo/HulajDusza_logo.png"],cacheExpirationTime=30;self.addEventListener("install",e=>{e.waitUntil(caches.open(CACHE_NAME).then(e=>e.addAll(urlsToCache)))}),self.addEventListener("fetch",c=>{c.respondWith((async()=>{var e,t,s=await caches.open(CACHE_NAME),a=await s.match(c.request);return a?(e=a.headers.get("date"),t=new Date,(e=new Date(e)).setDate(e.getDate()+cacheExpirationTime),e<t?(e=await fetch(c.request),await s.put(c.request,e.clone()),e):a):(t=await fetch(c.request),await s.put(c.request,t.clone()),t)})())}),self.addEventListener("activate",e=>{e.waitUntil(caches.keys().then(e=>Promise.all(e.map(e=>{if(e!==CACHE_NAME)return caches.delete(e)}))))}),self.addEventListener("push",e=>{e.data?console.log("Received push data:",e.data.text()):console.log("Received empty push data")}),self.addEventListener("notificationclick",e=>{e.notification.close(),e.waitUntil(clients.matchAll().then(e=>{if(e.openWindow)return e.openWindow("/")}))}); |
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 single line of code that handles multiple functionalities (caching, event listeners) is overly complex and hard to maintain. Consider refactoring this into multiple lines or separate functions to improve readability and maintainability. For example, you could separate the event listener callbacks into named functions:
function handleInstall(event) {
event.waitUntil(caches.open(CACHE_NAME).then(cache => cache.addAll(urlsToCache)));
}
self.addEventListener('install', handleInstall);
This approach not only makes the code cleaner but also easier to manage and debug.
@@ -112,0 +1,2 @@ | |||
let CACHE_NAME="hulajdusza-cache-v1",urlsToCache=["/","/index.html","/about.html","/services.html","/contact.html","/content/cookies.html","/content/pricing.html","/content/privacy.html","/css/style.css","/css/index.css","/css/services.css","/css/about.css","/css/contact.css","/css/pricing.css","/css/privacy.css","/js/script.js","/js/send_email.js","/js/phone_format.js","/js/sharing-buttons.js","/js/counters.js","/icons/favicon-32x32.png","/icons/apple-icon-57x57.png","/img/Logo/HulajDusza_logo.png"],cacheExpirationTime=30;self.addEventListener("install",e=>{e.waitUntil(caches.open(CACHE_NAME).then(e=>e.addAll(urlsToCache)))}),self.addEventListener("fetch",c=>{c.respondWith((async()=>{var e,t,s=await caches.open(CACHE_NAME),a=await s.match(c.request);return a?(e=a.headers.get("date"),t=new Date,(e=new Date(e)).setDate(e.getDate()+cacheExpirationTime),e<t?(e=await fetch(c.request),await s.put(c.request,e.clone()),e):a):(t=await fetch(c.request),await s.put(c.request,t.clone()),t)})())}),self.addEventListener("activate",e=>{e.waitUntil(caches.keys().then(e=>Promise.all(e.map(e=>{if(e!==CACHE_NAME)return caches.delete(e)}))))}),self.addEventListener("push",e=>{e.data?console.log("Received push data:",e.data.text()):console.log("Received empty push data")}),self.addEventListener("notificationclick",e=>{e.notification.close(),e.waitUntil(clients.matchAll().then(e=>{if(e.openWindow)return e.openWindow("/")}))}); |
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.
There is a lack of error handling for network requests and cache operations within the service worker. This can lead to unhandled promise rejections and potential failures in caching strategies. Implementing try-catch blocks or handling promise rejections can significantly improve the robustness of your service worker. For instance, you can modify the fetch event handling to include error handling:
self.addEventListener('fetch', event => {
event.respondWith(
caches.open(CACHE_NAME).then(cache => {
return cache.match(event.request).then(response => {
if (response) {
// Return cached response
return response;
}
throw Error('No match in cache, fetching from network...');
}).catch(error => {
// Handle errors during cache match or fetch
console.error('Fetch handler error:', error);
return fetch(event.request);
});
})
);
});
This modification ensures that any errors during the cache match or network fetch are caught and handled appropriately.
Dependency ReviewThe following issues were found:
License Issuespackage.json
OpenSSF ScorecardScorecard details
Scanned Files
|
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.
Code review by ChatGPT
This pull request failed to merge: PR cannot be automatically rebased, please rebase manually to continue. After you have resolved the problem, you should remove the Additional debug info: A merge conflict occurred while queueing this pull request. Potential conflicting pull requests: Please resolve manually and requeue. If resolving manually does not work, you may need to wait for previous PRs in the queue to merge. |
const gulp = require('gulp'); | ||
const fileInclude = require('gulp-file-include'); | ||
const replace = require('gulp-replace'); | ||
const uglify = require('gulp-uglify'); |
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' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
@@ -1,8 +1,10 @@ | |||
require ('./instruments.js'); | |||
|
|||
const Sentry = require('@sentry/node'); |
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' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
@@ -0,0 +1 @@ | |||
import '@testing-library/jest-dom/extend-expect'; |
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.
'import' is only available in ES6 (use 'esversion: 6').
}; | ||
|
||
export default displayNotification; | ||
function displayNotification(e,t){let i=document.getElementById("notification");var n=document.getElementById("notification-message");i&&n?(i.classList.add(t,"show"),n.textContent=e,i.setAttribute("aria-live","assertive"),"error"===t&&Sentry.captureException(new Error(e)),setTimeout(()=>{i.classList.remove("show","success","error")},5e3)):(console.error("Elementy notification lub notificationMessage nie istnieją"),Sentry.captureException(new Error("Elementy notification lub notificationMessage nie istnieją")))}export default displayNotification; |
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.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
'export' is only available in ES6 (use 'esversion: 6').
'let' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
@@ -0,0 +1,2 @@ | |||
function phoneFormatter(){let e=document.getElementById("phone");e.addEventListener("input",()=>{var n=e.value.replace(/\D/g,"");if(3<=n.length){let t="";for(let e=0;e<n.length;e++)0<e&&e%3==0&&(t+=" "),t+=n[e];e.value=t}})}export default phoneFormatter; |
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.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
'export' is only available in ES6 (use 'esversion: 6').
'let' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
* and applies a 'loading' class to change the button's style, effectively animating | ||
* it into a loading state. | ||
*/ | ||
function animateLoading() { |
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.
Missing semicolon.
formButton.classList.add(type); | ||
if (type === 'success') { | ||
formButton.querySelector('.fa-check').classList.remove('hidden'); | ||
function initButtonAnimation() { |
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' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
}; | ||
|
||
export default displayNotification; | ||
export default displayNotification; |
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.
'export' is only available in ES6 (use 'esversion: 6').
} | ||
|
||
setTimeout(() => { |
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.
'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
|
||
} | ||
|
||
export default phoneFormatter; |
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.
'export' is only available in ES6 (use 'esversion: 6').
|
This pull request failed to merge: PR cannot be automatically rebased, please rebase manually to continue. After you have resolved the problem, you should remove the Additional debug info: A merge conflict occurred while queueing this pull request. Potential conflicting pull requests: Please resolve manually and requeue. If resolving manually does not work, you may need to wait for previous PRs in the queue to merge. |
@mergify rebase |
❌ Base branch update has failedGit reported the following error:
|
@mergify queue |
🟠 Waiting for conditions to match
|
This pull request failed to merge: PR cannot be automatically rebased, please rebase manually to continue. After you have resolved the problem, you should remove the Additional debug info: A merge conflict occurred while queueing this pull request. Potential conflicting pull requests: Please resolve manually and requeue. If resolving manually does not work, you may need to wait for previous PRs in the queue to merge. |
Podsumowanie przez Sourcery
Refaktoryzacja funkcji wysyłania e-maili w celu uwzględnienia generowania i walidacji tokenów CSRF, poprawiając bezpieczeństwo. Ulepszenie obsługi formularzy poprzez lepszą walidację i zarządzanie błędami. Wprowadzenie formatowania numerów telefonów i generowania meta tagów Open Graph. Optymalizacja modułów JavaScript poprzez minifikację i restrukturyzację kodu. Aktualizacja skryptów budowania i konfiguracja Jest do raportowania pokrycia testami.
Nowe funkcje:
Ulepszenia:
Budowanie:
CI:
Dokumentacja:
Testy:
Original summary in English
Summary by Sourcery
Refactor the email sending functionality to include CSRF token generation and validation, improving security. Enhance form handling with better validation and error management. Introduce phone number formatting and Open Graph meta tag generation. Optimize JavaScript modules by minifying and restructuring code. Update build scripts and configure Jest for test coverage reporting.
New Features:
Enhancements:
Build:
CI:
Documentation:
Tests: