-
Notifications
You must be signed in to change notification settings - Fork 56
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
feat(themes): 📝 Blog Interaction #429
base: master
Are you sure you want to change the base?
Conversation
/autoupdate |
|
/autoupdate |
|
/auotupdate |
updateLikedBlogs(blogId, add) { | ||
const likedBlogs = JSON.parse(localStorage.getItem('liked_blogs')) || []; | ||
const updatedBlogs = add ? [...likedBlogs, blogId] : likedBlogs.filter(id => id !== blogId); | ||
localStorage.setItem('liked_blogs', JSON.stringify(updatedBlogs)); |
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.
❌ Codacy found a critical Error Prone issue: The 'localStorage' is still an experimental feature The configured version range is '>=16.0.0'.
The issue described by ESLint indicates that the localStorage
feature is considered experimental in the configured version range (>=16.0.0). This means that relying on localStorage
might not be safe or stable, and it could lead to unexpected behavior or compatibility issues in certain environments.
To address this, one approach is to use a feature detection pattern to ensure that localStorage
is available and functioning before attempting to use it. If localStorage
is not available, you can provide a fallback mechanism.
Here’s the single-line code suggestion to fix the issue:
localStorage.setItem('liked_blogs', JSON.stringify(updatedBlogs)); | |
if (typeof localStorage !== 'undefined') localStorage.setItem('liked_blogs', JSON.stringify(updatedBlogs)); |
This modification ensures that the code only attempts to use localStorage
if it is defined, thus avoiding potential issues in environments where localStorage
is not available or is considered experimental.
This comment was generated by an experimental AI tool.
|
||
updateLikedBlogs(blogId, add) { | ||
const likedBlogs = JSON.parse(localStorage.getItem('liked_blogs')) || []; | ||
const updatedBlogs = add ? [...likedBlogs, blogId] : likedBlogs.filter(id => id !== blogId); |
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.
Codacy has a fix for the issue: Expected parentheses around arrow function argument.
const updatedBlogs = add ? [...likedBlogs, blogId] : likedBlogs.filter(id => id !== blogId); | |
const updatedBlogs = add ? [...likedBlogs, blogId] : likedBlogs.filter((id) => id !== blogId); |
const endpoint = this.isLiked ? `blogs/${blogId}/unlike` : `blogs/${blogId}/like`; | ||
try { | ||
await salla.api.request(endpoint, '', this.isLiked ? 'delete' : 'put'); | ||
likeBtn.innerHTML = originalContent; |
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 issue flagged by the ESLint linter is that the code is directly setting innerHTML
with originalContent
, which can lead to Cross-Site Scripting (XSS) vulnerabilities if originalContent
contains any user-provided data. When you use innerHTML
, any HTML tags within the string will be parsed and rendered, which can be exploited if the content is not properly sanitized.
To fix this issue, you should use a method that does not interpret the string as HTML, such as textContent
, which will treat the string as plain text and not parse it as HTML.
Here is the single line change to fix the issue:
likeBtn.innerHTML = originalContent; | |
likeBtn.textContent = originalContent; |
Note: This suggestion assumes that originalContent
does not need to contain actual HTML. If originalContent
must include HTML, you would need to ensure it's properly sanitized before setting it with innerHTML
.
This comment was generated by an experimental AI tool.
const endpoint = this.isLiked ? `blogs/${blogId}/unlike` : `blogs/${blogId}/like`; | ||
try { | ||
await salla.api.request(endpoint, '', this.isLiked ? 'delete' : 'put'); | ||
likeBtn.innerHTML = originalContent; |
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.
❌ Codacy found a critical Security issue: User controlled data in a likeBtn.innerHTML
is an anti-pattern that can lead to XSS vulnerabilities
The issue identified by the Semgrep linter is that setting likeBtn.innerHTML = originalContent;
directly can introduce a Cross-Site Scripting (XSS) vulnerability. This is because originalContent
may contain user-controlled data, which, if not properly sanitized, can lead to malicious scripts being executed in the browser.
To fix this issue, we should avoid directly setting innerHTML
with potentially unsafe content. Instead, we can use textContent
to ensure that any HTML tags are treated as plain text, not as executable code. However, in this specific case, since originalContent
is meant to be HTML, a more appropriate solution would be to ensure that the content is sanitized before setting it as innerHTML
.
Given the constraint of a single line change, and assuming originalContent
is safe to use (e.g., it has been sanitized elsewhere in the code), we can use a safer method to set the inner HTML content. Here’s the code suggestion:
likeBtn.innerHTML = originalContent; | |
likeBtn.innerHTML = DOMPurify.sanitize(originalContent); |
In this suggestion, DOMPurify
is a library that sanitizes HTML to prevent XSS attacks. This ensures that any user-controlled data is properly sanitized before being inserted into the DOM. Make sure to include the DOMPurify library in your project to use this solution.
This comment was generated by an experimental AI tool.
initToggleLike() { | ||
const likeBtn = document.querySelector('#blog-like'); | ||
|
||
if (!likeBtn || !salla.url.is_page('blog.single')) { |
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.
❌ Codacy found a critical Error Prone issue: You have a misspelled word: salla on Identifier
The issue identified by ESLint is that the identifier salla
is potentially misspelled or undefined. This could be a typo or a missing import/definition for the salla
object. To fix this issue, you need to ensure that the salla
object is correctly defined or imported in your code.
If salla
is supposed to be an imported module, you should add the appropriate import statement at the top of your file.
Here is the code suggestion to fix the issue:
if (!likeBtn || !salla.url.is_page('blog.single')) { | |
import salla from './salla'; |
This comment was generated by an experimental AI tool.
} | ||
|
||
const blogId = likeBtn.dataset.blogId; | ||
const likedBlogs = JSON.parse(localStorage.getItem('liked_blogs')) || []; |
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.
❌ Codacy found a critical Error Prone issue: The 'localStorage' is still an experimental feature The configured version range is '>=16.0.0'.
The issue described by ESLint is related to the usage of localStorage
, which is considered experimental and may not be fully supported or stable in the specified version range (>=16.0.0). To address this issue, we can use a feature detection technique to check if localStorage
is available before attempting to use it. This ensures that our code does not break in environments where localStorage
is not supported.
Here is the code suggestion to fix the issue:
const likedBlogs = JSON.parse(localStorage.getItem('liked_blogs')) || []; | |
const likedBlogs = (typeof localStorage !== 'undefined' && JSON.parse(localStorage.getItem('liked_blogs'))) || []; |
This change ensures that we only attempt to use localStorage
if it is defined, thus avoiding potential runtime errors in environments where localStorage
is not available.
This comment was generated by an experimental AI tool.
|
||
likeBtn.addEventListener('click', async (event) => { | ||
event.preventDefault(); | ||
if (salla.config.isGuest()) { |
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.
❌ Codacy found a critical Error Prone issue: You have a misspelled word: salla on Identifier
The issue reported by ESLint is that the identifier salla
might be misspelled or not defined, which can lead to runtime errors if salla
is not a valid object or if it's not imported correctly. This kind of error is common when there's a typo or when the object is not defined in the current scope.
To fix the issue, you should ensure that the salla
object is correctly defined or imported in the scope where it's being used. Assuming salla
is a global object or imported correctly and the misspelling is the issue, you should correct the spelling of salla
to the correct identifier.
Here's the corrected line assuming the correct identifier is sallaApp
:
if (salla.config.isGuest()) { | |
if (sallaApp.config.isGuest()) { |
Ensure that sallaApp
is the correct and defined identifier in your code. If salla
is indeed the correct identifier and ESLint is mistakenly flagging it, you might need to configure ESLint to recognize salla
as a global variable.
This comment was generated by an experimental AI tool.
const endpoint = this.isLiked ? `blogs/${blogId}/unlike` : `blogs/${blogId}/like`; | ||
try { | ||
await salla.api.request(endpoint, '', this.isLiked ? 'delete' : 'put'); | ||
likeBtn.innerHTML = originalContent; |
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 issue flagged by ESLint is related to the assignment of potentially unsafe content to innerHTML
, which can expose the application to Cross-Site Scripting (XSS) attacks. When you directly set innerHTML
, any script included in the content can be executed, which poses a security risk.
To fix this issue, you should avoid directly assigning HTML strings to innerHTML
. Instead, you can use a method that safely assigns content, such as textContent
for plain text or using DOM manipulation methods to construct the HTML elements.
In this specific case, if originalContent
is plain text, you should use textContent
. If originalContent
contains HTML, you should sanitize it before assignment. Here is a single line change assuming originalContent
is plain text:
likeBtn.innerHTML = originalContent; | |
likeBtn.textContent = originalContent; |
If originalContent
contains HTML and you need to preserve it, you would need to sanitize it first:
likeBtn.innerHTML = originalContent; | |
likeBtn.innerHTML = DOMPurify.sanitize(originalContent); |
Note that DOMPurify
is a widely-used library for sanitizing HTML to prevent XSS attacks. Make sure to include and configure this library in your project if you choose to use it.
This comment was generated by an experimental AI tool.
return salla.notify.error(salla.lang.get('common.messages.must_login')); | ||
} | ||
|
||
const originalContent = likeBtn.innerHTML; |
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 issue identified by ESLint is that storing raw HTML in a variable (originalContent
) can be risky, as it may lead to cross-site scripting (XSS) vulnerabilities. An attacker could inject malicious HTML or JavaScript into the innerHTML
of likeBtn
, which would then be stored in originalContent
and potentially reinserted into the DOM later.
To mitigate this risk, we should avoid storing and reusing raw HTML. Instead, we can use a safer method to temporarily store the state of the button, such as by using text content or a data attribute.
Here's a single-line code suggestion to fix the issue by storing the text content of the likeBtn
:
const originalContent = likeBtn.innerHTML; | |
const originalContent = likeBtn.textContent; |
This change ensures that only the text content of the button is stored, which reduces the risk of XSS attacks.
This comment was generated by an experimental AI tool.
return; | ||
} | ||
|
||
const blogId = likeBtn.dataset.blogId; |
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.
❌ Codacy found a critical Error Prone issue: You have a misspelled word: dataset on Identifier
The issue reported by ESLint suggests that there is a misspelling in the word "dataset" on the identifier likeBtn.dataset.blogId
. However, in this context, dataset
is a valid JavaScript property used to access data attributes of an HTML element. The ESLint error might be a false positive, but to address it, we can use a more explicit way to access the data attribute using the getAttribute
method, which might also be more readable for some developers.
Here's the code suggestion to fix the issue:
const blogId = likeBtn.dataset.blogId; | |
const blogId = likeBtn.getAttribute('data-blog-id'); |
This change retrieves the value of the data-blog-id
attribute explicitly, which should resolve the ESLint error.
This comment was generated by an experimental AI tool.
@@ -0,0 +1,88 @@ | |||
import BasePage from './base-page'; |
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.
❌ Codacy found a critical Error Prone issue: Import path mush be a path alias
The issue identified by ESLint is that the import path should use a path alias instead of a relative path. Path aliases help in making import statements more readable and maintainable, especially in larger projects. They also help to avoid issues with deeply nested directories.
To fix this issue, you need to configure your project to use path aliases. This is typically done in your jsconfig.json
or tsconfig.json
file. Assuming you have already set up an alias for the base directory (e.g., @base
), you can then update the import statement to use this alias.
Here is the single line code suggestion to fix the issue:
import BasePage from './base-page'; | |
import BasePage from '@base/base-page'; |
Make sure that the alias @base
is correctly configured in your project's configuration file. For example, in a jsconfig.json
or tsconfig.json
file, it might look something like this:
{
"compilerOptions": {
"baseUrl": "./",
"paths": {
"@base/*": ["src/base/*"]
}
}
}
This configuration tells the compiler that any import path starting with @base
should be resolved relative to the src/base
directory. Adjust the paths as necessary to fit your project's structure.
This comment was generated by an experimental AI tool.
|
||
anime({ | ||
targets: countSpan, | ||
innerHTML: isLiked ? currentCount + 1 : currentCount - 1, |
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 issue identified by ESLint is that using innerHTML
with unencoded input can lead to security vulnerabilities, specifically Cross-Site Scripting (XSS) attacks. By directly setting the innerHTML
property with a number (even though it seems harmless), it opens up the potential for malicious input to be injected, especially if the source of currentCount
can be manipulated.
To fix this issue, we should use textContent
instead of innerHTML
. textContent
is safer as it sets the text content of the node and does not interpret it as HTML.
Here is the single line change to fix the issue:
innerHTML: isLiked ? currentCount + 1 : currentCount - 1, | |
targets: countSpan, textContent: isLiked ? currentCount + 1 : currentCount - 1, |
This comment was generated by an experimental AI tool.
const endpoint = this.isLiked ? `blogs/${blogId}/unlike` : `blogs/${blogId}/like`; | ||
try { | ||
await salla.api.request(endpoint, '', this.isLiked ? 'delete' : 'put'); | ||
likeBtn.innerHTML = originalContent; |
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.
❌ Codacy found a critical Security issue: User controlled data in methods like innerHTML
, outerHTML
or document.write
is an anti-pattern that can lead to XSS vulnerabilities
The issue identified by the Semgrep linter is that using innerHTML
with user-controlled data can lead to Cross-Site Scripting (XSS) vulnerabilities. XSS attacks occur when an attacker is able to inject malicious scripts into web pages viewed by other users. By directly setting innerHTML
, there's a risk that an attacker could inject malicious HTML or JavaScript.
To fix this issue, we should avoid using innerHTML
to set content that could be influenced by user input. Instead, we can use safer alternatives like textContent
or manipulating the DOM elements directly.
Here's the single line change to fix the issue:
likeBtn.innerHTML = originalContent; | |
likeBtn.textContent = originalContent; |
However, please note that textContent
will not render HTML tags as HTML but as plain text. If originalContent
contains HTML that needs to be rendered, you should ensure that originalContent
is sanitized properly before assigning it to innerHTML
. Here we assume originalContent
is plain text for demonstration purposes.
This comment was generated by an experimental AI tool.
|
||
const endpoint = this.isLiked ? `blogs/${blogId}/unlike` : `blogs/${blogId}/like`; | ||
try { | ||
await salla.api.request(endpoint, '', this.isLiked ? 'delete' : 'put'); |
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.
❌ Codacy found a critical Error Prone issue: You have a misspelled word: salla on Identifier
The issue described by ESLint suggests that there might be a misspelling in the identifier salla
. This could mean that salla
is not defined or misspelled, leading to potential runtime errors. To fix this issue, you need to ensure that the identifier salla
is correctly spelled and defined in the scope where it is being used.
Assuming that salla
should be correctly spelled and is a valid object in the scope, we should double-check the spelling. If the spelling is correct, then the issue might be a false positive from ESLint, and we can ignore it. However, if there is indeed a typo, we need to correct it.
Let's assume the correct identifier should be sallaApp
instead of salla
. Here is the single line change to fix the issue:
await salla.api.request(endpoint, '', this.isLiked ? 'delete' : 'put'); | |
await sallaApp.api.request(endpoint, '', this.isLiked ? 'delete' : 'put'); |
Ensure that sallaApp
is correctly defined and available in the current scope. If salla
is indeed correct, then no change is needed, and you might need to adjust your ESLint configuration.
This comment was generated by an experimental AI tool.
likeBtn.addEventListener('click', async (event) => { | ||
event.preventDefault(); | ||
if (salla.config.isGuest()) { | ||
return salla.notify.error(salla.lang.get('common.messages.must_login')); |
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.
❌ Codacy found a critical Error Prone issue: You have a misspelled word: lang on Identifier
The issue identified by ESLint is that there is a misspelled word in the identifier salla.lang.get
. The linter suggests that lang
might be a misspelling. This could be a typo, and the correct property might be something like salla.language.get
.
To fix this issue, you should correct the misspelled identifier. Assuming the correct property is language
instead of lang
, the single line change would be:
return salla.notify.error(salla.lang.get('common.messages.must_login')); | |
return salla.notify.error(salla.language.get('common.messages.must_login')); |
This comment was generated by an experimental AI tool.
What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)
What is the current behaviour? (You can also link to an open issue here)
What is the new behaviour? (You can also link to the ticket here)
Does this PR introduce a breaking change?
Screenshots (If appropriate)