feat: changing permission verify strategy #1374
Conversation
| this.mainWindow.webContents.setWindowOpenHandler((handler) => { | ||
| shell.openExternal(handler.url); | ||
| return { action: "deny" }; | ||
| }); |
There was a problem hiding this comment.
this is for the featurebase widget link
it opens a new window when clicking the "see all changelogs" button
…launcher/hydra into feature/check-directory-permission
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
This PR implements a more robust folder write permission verification system and enhances achievement notifications across the application.
- Added test file write/delete verification in
/src/main/events/hardware/check-folder-write-permission.tsto reliably check directory permissions - Improved achievement notifications by adding sound feedback and renderer process event handling in
/src/main/services/notifications/index.ts - Added periodic update checks every 60 minutes in
/src/renderer/src/components/header/auto-update-sub-header.tsx - Standardized error handling in
/src/main/services/hydra-api.tswith better typing and error messages - Centralized z-index management through theme variables in
/src/renderer/src/theme.css.ts
💡 (1/5) You can manually trigger the bot by mentioning @greptileai in a comment!
34 file(s) reviewed, 28 comment(s)
Edit PR Review Bot Settings | Greptile
| const checkFolderWritePermission = async ( | ||
| _event: Electron.IpcMainInvokeEvent, | ||
| path: string | ||
| ) => | ||
| new Promise((resolve) => { | ||
| fs.access(path, fs.constants.W_OK, (err) => { | ||
| resolve(!err); | ||
| }); | ||
| }); | ||
| testPath: string | ||
| ) => { |
There was a problem hiding this comment.
style: function is marked async but contains no await operations - should either use async fs operations or remove async keyword
| }); | ||
| testPath: string | ||
| ) => { | ||
| const testFilePath = path.join(testPath, ".hydra-write-test"); |
There was a problem hiding this comment.
style: consider using a more unique filename with timestamp/uuid to avoid potential conflicts
| fs.writeFileSync(testFilePath, ""); | ||
| fs.rmSync(testFilePath); |
There was a problem hiding this comment.
style: using synchronous fs operations can block the main thread - consider using async writeFile/rm
| } catch (err) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
style: error is silently swallowed - should at least log it for debugging purposes
| @@ -45,6 +45,7 @@ Sentry.init({ | |||
| tracesSampleRate: 1.0, | |||
| replaysSessionSampleRate: 0.1, | |||
| replaysOnErrorSampleRate: 1.0, | |||
| release: await window.electron.getVersion(), | |||
There was a problem hiding this comment.
logic: Using await at module level requires top-level await support. Consider moving Sentry initialization into an async function that runs after the app loads to ensure compatibility across all environments.
| @@ -78,6 +78,7 @@ export function useUserDetails() { | |||
| ...response, | |||
| username: userDetails?.username || "", | |||
| subscription: userDetails?.subscription || null, | |||
| featurebaseJwt: userDetails?.featurebaseJwt || "", | |||
| }); | |||
| }, | |||
| [updateUserDetails, userDetails?.username, userDetails?.subscription] | |||
There was a problem hiding this comment.
logic: dependency array is missing userDetails.featurebaseJwt but includes it in the spread object
| ) | ||
| } | ||
| /> | ||
| </div> | ||
|
|
||
| <div className={styles.gameOptionHeader}> | ||
| <h2>{t("downloads_secion_title")}</h2> |
There was a problem hiding this comment.
syntax: typo in translation key 'downloads_secion_title' should be 'downloads_section_title'
| $toast-z-index: 5; | ||
| $bottom-panel-z-index: 3; | ||
| $title-bar-z-index: 4; | ||
| $title-bar-z-index: 1900000001; | ||
| $backdrop-z-index: 4; | ||
| $modal-z-index: 5; |
There was a problem hiding this comment.
style: z-index values are inconsistent across components (5, 3, 1900000001, 4, 5). Consider using standardized increments like 100, 200, 300 for better maintainability
| @@ -267,6 +267,7 @@ export interface UserDetails { | |||
| backgroundImageUrl: string | null; | |||
| profileVisibility: ProfileVisibility; | |||
| bio: string; | |||
| featurebaseJwt: string; | |||
There was a problem hiding this comment.
logic: featurebaseJwt is marked as required but should likely be optional since not all users may have this token
|
When submitting this pull request, I confirm the following (please check the boxes):
Fill in the PR content: