creating new webhook for the options-automator #554

kneeyo1 · 2023-07-26T20:54:48Z

This is a work in progress associated with this pr . The configoptions script runs as part of a kubernetes deployment and I would like to post the logs from that script to slack.

Ideally, that linked script from sentry would send post requests to the webhook defined here, which then pushes it to slack.

todo: figure out env variables and the slack channel I'm pushing to

kneeyo1 · 2023-08-01T16:10:31Z

src/webhooks/options-automator/options-automator.ts

+      blocks: failedBlock,
+      channel: FEED_OPTIONS_AUTOMATOR_CHANNEL_ID,
+      unfurl_links: false,
+    });


CI tests are yelling bc it says it needs text, but

export interface ChatPostMessageArguments extends WebAPICallOptions, TokenOverridable { channel: string; text?: string; as_user?: boolean; attachments?: MessageAttachment[]; blocks?: (KnownBlock | Block)[]; icon_emoji?: string; icon_url?: string; metadata?: MessageMetadata; link_names?: boolean; mrkdwn?: boolean; parse?: 'full' | 'none'; reply_broadcast?: boolean; thread_ts?: string; unfurl_links?: boolean; unfurl_media?: boolean; username?: string; }

Shows that its not necessary, confused

Verify which version of the module you're getting w/ Yarn. It's not showing the ? in the CI window so you may be tethered to an older version where that wasn't optional.

Looks like we'd need to bump slack/web-api for this to pick up text as optional parameter:
https://github.com/slackapi/node-slack-sdk/releases/tag/%40slack%2Fweb-api%406.2.0

I faintly recall needing to set text but it being ignored and just the blocks were shown.

src/webhooks/options-automator/options-automator.ts

hubertdeng123 · 2023-08-01T17:15:21Z

src/webhooks/options-automator/options-automator.ts

+  ];
+
+  try {
+    // @ts-ignore


Is this here bc CI tests are yelling?

yeah pretty much

mattgauntseo-sentry · 2023-08-01T17:24:11Z

src/blocks/slackBlocks.ts

 export function section(block: MrkdwnElement): KnownBlock {
  return {
    type: 'section',
    text: block,
  };
 }
+
+export function SectionBlock(fields: MrkdwnElement[]): KnownBlock {


Would be good to add this to the above section() function.

export function section(block: MrkdwnElement, fields: MrkdownElement[])

since section is already being used by other functions rewriting it is probably out of scope, spoke with Hubert and trying to make either parameter optional just makes the code too messy and unreadable, so I'm going to keep it as two functions

src/webhooks/options-automator/options-automator.ts

mattgauntseo-sentry · 2023-08-01T17:30:36Z

src/webhooks/options-automator/options-automator.ts

+      blocks: failedBlock,
+      channel: FEED_OPTIONS_AUTOMATOR_CHANNEL_ID,
+      unfurl_links: false,
+    });


I faintly recall needing to set text but it being ignored and just the blocks were shown.

fpacifici · 2023-08-03T21:44:15Z

src/config/index.ts

@@ -24,6 +24,8 @@ export const GOCD_ORIGIN =
  process.env.GOCD_ORIGIN || 'https://deploy.getsentry.net';
 export const FEED_DEPLOY_CHANNEL_ID =
  process.env.FEED_DEPLOY_CHANNEL_ID || 'C051ED5GLN4';
+export const FEED_OPTIONS_AUTOMATOR_CHANNEL_ID =
+  process.env.FEED_OPTIONS_AUTOMATOR_CHANNEL_ID || 'C05JJ5JNZAB';


Where did you pick this channel id?
#proj-options-automator seems to be C04URUC21C5

this was my test channel id while I was testing locally, will correct it before merging.

fpacifici · 2023-08-03T21:45:44Z

src/types/index.ts

@@ -65,7 +65,7 @@ export type CheckRun = EmitterWebhookEvent<'check_run'>['payload']['check_run'];
 //
 // Need to do this as we can't convert a string literal union to an array of literals
 export const CHECK_RUN_PROPERTIES = ['id', 'head_sha', 'html_url'] as const;
-export type CheckRunProperty = typeof CHECK_RUN_PROPERTIES[number];
+export type CheckRunProperty = (typeof CHECK_RUN_PROPERTIES)[number];


Why this change ?

seems like an autoformatting change I didn't make this change

src/webhooks/options-automator/options-automator.ts

codecov · 2023-08-07T18:23:36Z

Codecov Report

Patch coverage: 97.91% and project coverage change: +0.19% 🎉

Comparison is base (5448e7f) 85.15% compared to head (a889c6c) 85.34%.
Report is 10 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #554      +/-   ##
==========================================
+ Coverage   85.15%   85.34%   +0.19%     
==========================================
  Files          98      101       +3     
  Lines        2445     2539      +94     
  Branches      479      505      +26     
==========================================
+ Hits         2082     2167      +85     
- Misses        357      366       +9     
  Partials        6        6

Files Changed	Coverage Δ
...rc/webhooks/options-automator/options-automator.ts	`97.43% <97.43%> (ø)`
src/blocks/slackBlocks.ts	`100.00% <100.00%> (ø)`
src/config/index.ts	`98.93% <100.00%> (+0.02%)`	⬆️
src/webhooks/options-automator/index.ts	`100.00% <100.00%> (ø)`

... and 8 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

hubertdeng123

lot of codecov warnings, perhaps this could use more coverage?

hubertdeng123 · 2023-08-07T20:47:44Z

test/payloads/options-automator/testpayload.json

+    ],
+    "unregistered_options": [],
+    "invalid_type_options": []
+}


nit: newline

hubertdeng123 · 2023-08-07T20:55:42Z

src/blocks/slackBlocks.ts

  };
 }
+
+export function section(block?: MrkdwnElement, fields?: MrkdwnElement[]): KnownBlock {


You're passing in undefined a lot to this function. I think it's a bit cleaner if an object is passed in instead, this makes it more readable, especially when this function is called.

Suggested change

export function section(block?: MrkdwnElement, fields?: MrkdwnElement[]): KnownBlock {

export function section({ block, fields }): KnownBlock {

hubertdeng123 · 2023-08-07T21:33:09Z

src/blocks/slackBlocks.ts

+      fields: fields,
+    };
+  } else {
+    throw new Error('Either block or fields must be provided.');


Feels like this error should be thrown before undefined is passed in for both block and fields here.

kneeyo1 added 8 commits July 26, 2023 13:51

creating new webhook for the options-automator

57e0609

Merge branch 'main' into kneeyo1/options-automator-webhook

a723d02

not sure if im doing this right

2859a36

set up the webhook.

15605c4

will probably need to create a block to format message

a0d4f27

added support to print out options automator stuff

0c06fbb

fixed typing

890703b

quick fix

7bbe577

kneeyo1 commented Aug 1, 2023

View reviewed changes

kneeyo1 requested review from mattgauntseo-sentry and hubertdeng123 August 1, 2023 16:17

hubertdeng123 reviewed Aug 1, 2023

View reviewed changes

mattgauntseo-sentry reviewed Aug 1, 2023

View reviewed changes

kneeyo1 added 2 commits August 3, 2023 11:41

added some styling so it hides the empty file stuff

e139909

beautiful

58156d0

fpacifici reviewed Aug 3, 2023

View reviewed changes

kneeyo1 added 7 commits August 4, 2023 12:01

addressed some comments

abb877a

this is the only way to do it trust me

9b8fbb9

testcases

b9a5b8a

added test cases, debugging still

0f72c93

add test payload

a5a90ee

new problem yay

828ef15

im stumped

a539189

kneeyo1 marked this pull request as ready for review August 7, 2023 17:58

kneeyo1 added 5 commits August 7, 2023 13:00

need to fix last test case

383808e

need to fix last test case

533367a

updated channel id

76fcfa0

hmmm weird test behavior but ok

82df7bb

fixing up test cases

5ffcdd0

ok now its checking both messages.

ff72728

hubertdeng123 reviewed Aug 7, 2023

View reviewed changes

kneeyo1 added 2 commits August 7, 2023 17:25

more test coverage

096fe63

added more test coverage

a889c6c

mattgauntseo-sentry approved these changes Aug 8, 2023

View reviewed changes

hubertdeng123 approved these changes Aug 8, 2023

View reviewed changes

kneeyo1 merged commit fc57001 into main Aug 8, 2023
7 checks passed

kneeyo1 deleted the kneeyo1/options-automator-webhook branch August 8, 2023 16:11

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

creating new webhook for the options-automator #554

creating new webhook for the options-automator #554

kneeyo1 commented Jul 26, 2023 •

edited

Loading

kneeyo1 Aug 1, 2023

owensk Aug 1, 2023

hubertdeng123 Aug 1, 2023

mattgauntseo-sentry Aug 1, 2023

hubertdeng123 Aug 1, 2023

kneeyo1 Aug 4, 2023

mattgauntseo-sentry Aug 1, 2023

kneeyo1 Aug 7, 2023

mattgauntseo-sentry Aug 1, 2023

fpacifici Aug 3, 2023

kneeyo1 Aug 4, 2023

fpacifici Aug 3, 2023

kneeyo1 Aug 4, 2023

codecov bot commented Aug 7, 2023 •

edited

Loading

hubertdeng123 left a comment

hubertdeng123 Aug 7, 2023

hubertdeng123 Aug 7, 2023

hubertdeng123 Aug 7, 2023

	export function section(block?: MrkdwnElement, fields?: MrkdwnElement[]): KnownBlock {
	export function section({ block, fields }): KnownBlock {

creating new webhook for the options-automator #554

creating new webhook for the options-automator #554

Conversation

kneeyo1 commented Jul 26, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Aug 7, 2023 • edited Loading

Codecov Report

hubertdeng123 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kneeyo1 commented Jul 26, 2023 •

edited

Loading

codecov bot commented Aug 7, 2023 •

edited

Loading