fix(server): fixed illegal removal of unowned trigger #965
Conversation
![cr-gpt[bot]](https://avatars.githubusercontent.com/in/292659?s=60&v=4){kind=small}
| return ResponseUtil.error('Trigger not found') | ||
| } | ||
|
|
||
| const res = await this.triggerService.remove(appid, id) | ||
| return ResponseUtil.ok(res) | ||
| } | ||
| } |
There was a problem hiding this comment.
with a brief code review
This patch seems to be making sure that the trigger exists before it is removed.
First, it checks if the trigger exists by querying the trigger service with the given appid and id.
If the trigger is not found, it returns an error response.
Otherwise, it calls the trigger service remove method with the given appid and id.
It then returns an ok response.
No obvious bugs are visible in this patch, but there are some potential improvements that could be made.
First, it might be a good idea to check that the user has the necessary permissions to delete the trigger before proceeding.
Second, the code should be refactored to make sure that it adheres to the Single Responsibility Principle (SRP) and follows other best practices.
Finally, appropriate logging should be added to track when triggers are deleted.
| where: { id }, | ||
| async remove(appid: string, id: string) { | ||
| const res = await this.prisma.cronTrigger.updateMany({ | ||
| where: { id, appid }, | ||
| data: { | ||
| state: TriggerState.Deleted, | ||
| }, |
There was a problem hiding this comment.
with the code review:
-
It looks like the findOne and findAll methods have been modified to include additional parameters: appid and state (in the findAll method). This should help to ensure that only valid and active triggers are retrieved.
-
The remove method has also been modified to include an additional parameter - appid - which should help to ensure that only triggers belonging to the specified appid are removed.
-
The code is generally well written and adheres to coding standards. However, it could be improved to make it more efficient and readable. For example, the parameters for the various methods could be grouped together in an object, making them easier to read and maintain.
-
Additionally, some error handling could be added to the methods to ensure that errors are properly handled in case of any unexpected issues.
No description provided.