-
Notifications
You must be signed in to change notification settings - Fork 0
[PROD RELEASE V6] #386
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
[PROD RELEASE V6] #386
Conversation
| jobs: | ||
| trivy-scan: | ||
| name: Use Trivy | ||
| runs-on: ubuntu-24.04 |
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.
[maintainability]
Consider using a stable version of the runner, such as ubuntu-latest, instead of ubuntu-24.04 to ensure compatibility and reduce maintenance overhead when new Ubuntu versions are released.
| uses: actions/checkout@v4 | ||
|
|
||
| - name: Run Trivy scanner in repo mode | ||
| uses: aquasecurity/trivy-action@0.33.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.
[security]
The Trivy action version 0.33.1 is specified. Ensure this is the latest stable version to benefit from the latest features and security patches. Consider using a version range or the latest tag if appropriate.
| github-pat: ${{ secrets.GITHUB_TOKEN }} | ||
|
|
||
| - name: Upload Trivy scan results to GitHub Security tab | ||
| uses: github/codeql-action/upload-sarif@v3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[security]
The upload-sarif action version v3 is specified. Ensure this is the latest stable version to benefit from the latest features and security patches. Consider using a version range or the latest tag if appropriate.
| export const TCACADEMY_HOST: string = `https://academy.${TC_DOMAIN}`; | ||
| export const SELF_SERVICE_HOST: string = `https://work.${TC_DOMAIN}`; | ||
| export const TC_API_V5_HOST: string = `https://api.${TC_DOMAIN}/v5`; | ||
| export const TC_API_HOST: string = `https://api.${TC_DOMAIN}/v6`; |
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.
[❗❗ correctness]
Changing the API version from /v5 to /v6 could have significant implications on the functionality and compatibility of the application. Ensure that all endpoints and data structures are compatible with the new version, and thoroughly test the integration to prevent any runtime errors or unexpected behavior.
|
|
||
| export async function sendSupportRequest(request: ContactSupportRequest): Promise<any> { | ||
| const url: string = `${TC_API_V5_HOST}/challenges/support-requests` | ||
| const url: string = `${TC_API_HOST}/challenges/support-requests` |
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.
[❗❗ correctness]
The change from TC_API_V5_HOST to TC_API_HOST suggests a potential update in API versioning or endpoint configuration. Ensure that TC_API_HOST is correctly configured and that this change aligns with the intended API version and endpoint structure. This could impact the correctness of the API calls if not properly updated.
| import { TC_API_HOST } from 'lib/config'; | ||
| import type { ContactSupportRequest } from './contact-support-request.model' | ||
|
|
||
| export async function sendSupportRequest(request: ContactSupportRequest): Promise<any> { |
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.
[maintainability]
The function sendSupportRequest returns a Promise<any>. Consider specifying a more precise return type instead of any to improve type safety and maintainability. This will help catch potential errors at compile time and make the code easier to understand.
|
|
||
|
|
||
| let resolve: (value: AuthUser) => void; | ||
| localCache[userHandle] = new Promise((r) => {resolve = r}); |
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.
[❗❗ correctness]
The promise initialization with let resolve: (value: AuthUser) => void; and new Promise((r) => {resolve = r}); can lead to potential issues if resolve is not called, as it will leave the promise pending indefinitely. Consider using a more robust pattern to ensure the promise is always resolved or rejected.
|
|
||
| const requestUrl: string = `${TC_API_V5_HOST}/members/${userHandle}`; | ||
| const requestUrl: string = `${TC_API_HOST}/members/${userHandle}`; | ||
| const request = fetch(requestUrl, {headers: {...getRequestAuthHeaders()}}); |
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.
[❗❗ correctness]
The fetch call does not handle HTTP errors. If the response status is not 200, the json() method will still be called, which could lead to unexpected behavior. Consider checking response.ok and handling errors appropriately.
No description provided.