feat: Portal setup, theming and initial page scaffolding #76
Merged
Conversation
Contributor
There was a problem hiding this comment.
Copilot reviewed 39 out of 54 changed files in this pull request and generated 2 suggestions.
Files not reviewed (15)
- .env.example: Language not supported
- build/dev/portal/Dockerfile: Language not supported
- internal/portal/.gitignore: Language not supported
- internal/portal/index.html: Language not supported
- internal/portal/package.json: Language not supported
- internal/portal/src/app.scss: Language not supported
- internal/portal/src/common/Badge/Badge.scss: Language not supported
- internal/portal/src/common/Button/Button.scss: Language not supported
- internal/portal/src/common/Checkbox/Checkbox.scss: Language not supported
- internal/portal/src/common/CopyButton/CopyButton.scss: Language not supported
- build/dev/compose.yml: Evaluated as low risk
- docs/contributing/portal.md: Evaluated as low risk
- internal/config/config.go: Evaluated as low risk
- internal/portal/embed.go: Evaluated as low risk
- internal/portal/src/Sentry.tsx: Evaluated as low risk
Comments skipped due to low confidence (4)
internal/portal/src/common/CopyButton/CopyButton.tsx:41
- [nitpick] The class name 'unstyled-button' is ambiguous. It should be renamed to something more specific like 'copy-button'.
className="unstyled-button"
internal/portal/src/common/Button/Button.tsx:25
- The className concatenation might result in an extra space if className is not provided. Consider using a template literal with conditional checks.
} ${className}`;
internal/portal/src/app.tsx:139
- The error message 'Invalid token' should be capitalized for consistency.
console.error("Invalid token");
internal/portal/src/app.tsx:183
- There is no check to ensure CONFIGS.FORCE_THEME is a valid theme value. This could lead to unexpected behavior. Consider adding a check to ensure CONFIGS.FORCE_THEME is either 'dark' or 'light'.
const queryTheme = CONFIGS.FORCE_THEME || searchParams.get("theme");
| <Link | ||
| to={to} | ||
| className={className} | ||
| {...(disabled && { onClick: (e) => e.preventDefault() })} |
There was a problem hiding this comment.
The onClick prop is not being passed to the Link component when disabled is false. Ensure the onClick prop is passed correctly.
Suggested change
| {...(disabled && { onClick: (e) => e.preventDefault() })} | |
| {...(disabled ? { onClick: (e) => e.preventDefault() } : { onClick })} |
| }, []); | ||
|
|
||
| if (!token) { | ||
| window.location.replace(CONFIGS.REFERER_URL); |
There was a problem hiding this comment.
If the token is not found, the function calls window.location.replace(CONFIGS.REFERER_URL) and then returns undefined. This could lead to unexpected behavior if the component tries to use the token after this point. Consider adding a return statement after the redirect.
Suggested change
| window.location.replace(CONFIGS.REFERER_URL); | |
| return null; |
There was a problem hiding this comment.
❌ Changes requested. Reviewed everything up to 447ea57 in 1 minute and 49 seconds
More details
- Looked at
3686lines of code in50files - Skipped
4files when reviewing. - Skipped posting
4drafted comments based on config settings.
1. internal/services/api/jwt.go:23
- Draft comment:
The JWT expiration time has been increased to 24 hours. Ensure this aligns with your security requirements, as it increases the window for potential misuse if a token is compromised. - Reason this comment was not posted:
Confidence changes required:50%
Injwt.go, the expiration time for JWT tokens has been changed from 1 hour to 24 hours. This change should be carefully considered as it increases the window of opportunity for token misuse if a token is compromised. It's important to ensure that this change aligns with the security requirements of the application.
2. internal/services/api/jwt_test.go:52
- Draft comment:
The JWT expiration time in tests has been updated to 24 hours. Ensure this change is consistent with your security policies. - Reason this comment was not posted:
Confidence changes required:50%
Injwt_test.go, the expiration time for JWT tokens has been updated to 24 hours to match the change injwt.go. This is consistent with the code change, but the same security considerations apply as mentioned in thejwt.gocomment.
3. internal/services/api/jwt_test.go:70
- Draft comment:
The JWT expiration time in tests has been updated to 24 hours. Ensure this change is consistent with your security policies. - Reason this comment was not posted:
Confidence changes required:50%
Injwt_test.go, the expiration time for JWT tokens has been updated to 24 hours to match the change injwt.go. This is consistent with the code change, but the same security considerations apply as mentioned in thejwt.gocomment.
4. internal/services/api/jwt_test.go:88
- Draft comment:
The JWT expiration time in tests has been updated to 24 hours. Ensure this change is consistent with your security policies. - Reason this comment was not posted:
Confidence changes required:50%
Injwt_test.go, the expiration time for JWT tokens has been updated to 24 hours to match the change injwt.go. This is consistent with the code change, but the same security considerations apply as mentioned in thejwt.gocomment.
Workflow ID: wflow_2gCSHNcO1Usvo5T1
Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
| if (mode === "production") { | ||
| plugins.push( | ||
| sentryVitePlugin({ | ||
| authToken: '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', |
There was a problem hiding this comment.
Avoid hardcoding sensitive information like the Sentry auth token. Use environment variables to manage such sensitive data.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Introduces portal setup with theming, initial page scaffolding, and updates to configuration, routing, and UI components.
.env.exampleandconfig.go.router.goandtenant_handlers.go.jwt.goandjwt_test.go.SearchInput,Table,Toast,Tooltip, andTopicPickercomponents with corresponding styles.Destination,DestinationSettings,Events, andDestinationListcomponents for portal UI.main.tsxbased onCONFIGS.global.scssfor consistent theming.vite.config.tsto include Sentry plugin for production builds..gitignore.This description was created by
for 447ea57. It will automatically update as commits are pushed.