-
Notifications
You must be signed in to change notification settings - Fork 22
feat: add ticket-greeter-bot snap-in #62
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
base: main
Are you sure you want to change the base?
Conversation
|
General feedback: Please add comments to your code for clarity |
juaniq99
left a comment
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.
LGTM!
Leaving some comments
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.
For actual snap-ins, do remember to fill in the readme here. It serves as a quick way for internal folks looking at the code to understand what the snap-in does, supports, how to configure, etc.
I would add it for this one as well so it is clear what it does if someone comes to look at the example
| @@ -0,0 +1,60 @@ | |||
| { | |||
| "name": "devrev-snaps-typescript-template", | |||
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.
You could rename here and in 2 places similar to this one on the lock file to the snap-in's name
| return; | ||
| } | ||
|
|
||
| if (work.type !== "ticket") { |
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.
I would put this check before picking up severity, because severity is a field specific to tickets and it is not present in the other work types (issue, opportunity, task)
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.
Also, you could move this "easy" checks to the manifest itself in the form of jq_query. You can check out this example
The advantage (as far as I understand it 🧐) is that we would not be waking up the lambda function that actually executes the code at all for the cases that don't meet the condition. So we don't load the system just to exit from these checks and we save some bucks on AWS. Not a dealbreaker at all, but a minor gain that is normally easy to add in.
| async function greetTicket(event: any) { | ||
| const use_motivating_message = event.input_data.global_values.use_motivating_message; | ||
| const happy_message = event.input_data.global_values.happy_message; | ||
| const motivating_message = event.input_data.global_values.motivating_message; |
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.
Since these fields are not required on the manifest, I guess there would be a chance that the configuring user deletes the original default value and saves the config with no message at all
Summary
Adds a new snap-in bot that automatically greets users when they create tickets.
The bot supports configurable messages (happy or motivating) based on organization
settings, providing a welcoming experience for new ticket creators.
Connected Issues
ISS-223903 https://app.devrev.ai/devrev/works/ISS-223903
This is an onboarding task.