Conversation
Generate prisma client
[DO NOT MERGE] Challenge Winners Processing -> dev
Tweaks for isAssigned flag on tasks to check resourcesg
Allow copilot access to private tasks
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
|
||
| for (const batch of batches) { | ||
| const idParams = batch.map((id) => BigInt(id)); | ||
| const rows = await memberClient.$queryRaw` |
There was a problem hiding this comment.
[correctness]
Using BigInt for user IDs might cause issues if the IDs are not guaranteed to be numeric or if they exceed JavaScript's safe integer range. Consider validating the IDs before converting them to BigInt.
| const getReadline = () => { | ||
| if (!rl) { | ||
| if (!process.stdin.isTTY) { | ||
| throw new Error("Interactive input required but stdin is not a TTY."); |
There was a problem hiding this comment.
[design]
The script requires interactive input but does not handle non-TTY environments gracefully. Consider adding a non-interactive mode or a fallback mechanism for environments where TTY is not available.
| } | ||
|
|
||
| main().catch((error) => { | ||
| console.error(error); |
There was a problem hiding this comment.
[correctness]
Using process.exit(1) will terminate the Node.js process immediately, which might prevent asynchronous operations from completing. Consider using process.exitCode = 1 instead to allow pending operations to finish.
| @@ -0,0 +1 @@ | |||
| export * from "./index" No newline at end of file | |||
There was a problem hiding this comment.
[💡 style]
Consider adding a newline at the end of the file. While this is not a functional issue, it is a common convention that can prevent unnecessary diffs in version control systems when lines are added in the future.
| /* !!! This is code generated by Prisma. Do not edit directly. !!! | ||
| /* eslint-disable */ | ||
| // biome-ignore-all lint: generated file | ||
| module.exports = { ...require('.') } No newline at end of file |
There was a problem hiding this comment.
[maintainability]
The use of require('.') could potentially lead to issues if the index file exports a large number of modules or if there are circular dependencies. Consider explicitly requiring only the necessary modules to improve maintainability and reduce potential side effects.
| @@ -0,0 +1 @@ | |||
| export * from "./index" No newline at end of file | |||
There was a problem hiding this comment.
[💡 style]
Consider adding a newline at the end of the file. While this is not a correctness issue, it is a convention that can help avoid unnecessary diffs in version control systems when lines are added to the file.
| /* !!! This is code generated by Prisma. Do not edit directly. !!! | ||
| /* eslint-disable */ | ||
| // biome-ignore-all lint: generated file | ||
| module.exports = { ...require('#main-entry-point') } No newline at end of file |
There was a problem hiding this comment.
[❗❗ security]
Using require with a dynamic path like '#main-entry-point' can be risky if the path is not well-controlled. Ensure that #main-entry-point is a safe and validated path to prevent potential security issues.
| @@ -0,0 +1 @@ | |||
| export * from "./default" No newline at end of file | |||
There was a problem hiding this comment.
[💡 style]
Consider adding a newline at the end of the file. While this is not a correctness issue, it is a common convention that can help avoid unnecessary diffs in version control systems when lines are appended.
| ); | ||
| const role = _.toLower(_.get(member, "role", "")); | ||
| return PROJECT_TASK_VIEW_ACCESS_ROLES.has(role); | ||
| } catch (err) { |
There was a problem hiding this comment.
[correctness]
Consider adding a check to ensure that project is not null or undefined before accessing its properties. This will prevent potential runtime errors if projectHelper.getProject returns a null or undefined value.
| return PROJECT_TASK_VIEW_ACCESS_ROLES.has(role); | ||
| } catch (err) { | ||
| const status = _.get(err, "httpStatus") || _.get(err, "response.status"); | ||
| if (status === HttpStatus.FORBIDDEN || status === HttpStatus.NOT_FOUND) { |
There was a problem hiding this comment.
[💡 design]
The error handling logic here assumes that any error other than FORBIDDEN or NOT_FOUND should be logged and return false. Consider whether other types of errors should be handled differently, such as retrying the request or escalating the error.
| const isCopilotResource = _.some(memberResources, (resource) => { | ||
| const roleId = _.toString(resource.roleId); | ||
| const roleName = _.toLower(_.get(resource, "roleName", "")); | ||
| return copilotRoleIds.includes(roleId) || roleName === "copilot"; |
There was a problem hiding this comment.
[correctness]
Ensure that the roleName property exists on resource before calling _.toLower to avoid potential runtime errors.
| } | ||
| const dataToUpdate = _.omit(data, "constraints"); | ||
| const shouldRefreshPhaseNames = | ||
| Object.prototype.hasOwnProperty.call(data, "isOpen") || |
There was a problem hiding this comment.
[💡 style]
Consider using _.has(data, 'isOpen') and _.has(data, 'name') for consistency with other lodash usage in the codebase.
| select: { name: true }, | ||
| }); | ||
| const currentPhaseNames = _.uniq( | ||
| openPhases.map((phase) => phase.name).filter((name) => !_.isNil(name)) |
There was a problem hiding this comment.
[correctness]
The use of _.uniq and _.isNil is correct, but ensure that the name field is always expected to be a string. If there's a possibility of non-string values, consider additional validation or transformation.
3 participants
No description provided.