Skip to content
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

feat: Handle scheduled request events in addons #5403

Merged
merged 4 commits into from
Nov 23, 2023

Conversation

andreas-unleash
Copy link
Contributor

@andreas-unleash andreas-unleash commented Nov 23, 2023

  • Create 2 new events to replace the SCHEDULED_CHANGE_REQUEST_EXECUTED event
  • Handle the 3 events in slack-app and webhook addon definitions

3 events handled:

  • CHANGE_REQUEST_SCHEDULED
  • CHANGE_REQUEST_SCHEDULED_APPLICATION_SUCCESS
  • CHANGE_REQUEST_SCHEDULED_APPLICATION_FAILURE

Closes # 1-1555

Note: SCHEDULED_CHANGE_REQUEST_EXECUTED will be removed in follow up PR not to break current enterprise build

Copy link

vercel bot commented Nov 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
unleash-monorepo-frontend ✅ Ready (Inspect) Visit Preview 💬 Add feedback Nov 23, 2023 0:39am
1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
unleash-docs ⬜️ Ignored (Inspect) Visit Preview Nov 23, 2023 0:39am

Copy link
Contributor

@thomasheartman thomasheartman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good! A few minor suggestions that we could use to approve this, but nothing that needs to go in right now

@@ -140,6 +142,18 @@ const EVENT_MAP: Record<string, IEventData> = {
action: '*{{user}}* sent to review change request {{changeRequest}}',
path: '/projects/{{event.project}}/change-requests/{{event.data.changeRequestId}}',
},
[CHANGE_REQUEST_SCHEDULED]: {
action: '*{{user}}* scheduled change request {{changeRequest}} in project *{{event.project}}*',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we add when it was scheduled for? Something like this?

Suggested change
action: '*{{user}}* scheduled change request {{changeRequest}} in project *{{event.project}}*',
action: '*{{user}}* scheduled change request {{changeRequest}} to be applied at {{time}} in project *{{event.project}}*',

path: '/projects/{{event.project}}/change-requests/{{event.data.changeRequestId}}',
},
[CHANGE_REQUEST_SCHEDULED_APPLICATION_SUCCESS]: {
action: '*{{user}}* successfully applied scheduled change request {{changeRequest}} in project *{{event.project}}*',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who is the user in this case? The same as the one that's usually used for CR applications?

And if we're changing the wording of the failed event, we could also change this one a bit:

Suggested change
action: '*{{user}}* successfully applied scheduled change request {{changeRequest}} in project *{{event.project}}*',
action: 'Successfully applied the scheduled change request {{changeRequest}} by *{{user}}* in project *{{project}}*.',

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, the 'applier'.

path: '/projects/{{event.project}}/change-requests/{{event.data.changeRequestId}}',
},
[CHANGE_REQUEST_SENT_TO_REVIEW]: {
action: '*{{user}}* failed to apply scheduled change request {{changeRequest}} in project *{{event.project}}*',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because this is an urgent event, maybe we should put thefailed bit first? Not very important, just a suggestion.

Suggested change
action: '*{{user}}* failed to apply scheduled change request {{changeRequest}} in project *{{event.project}}*',
action: '*Failed* to apply the scheduled change request {{changeRequest}} by *{{user}}* in project *{{project}}*.',

@andreas-unleash andreas-unleash merged commit 2e17909 into main Nov 23, 2023
14 checks passed
@andreas-unleash andreas-unleash deleted the feat/handle_scheduled_events_addons branch November 23, 2023 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants