fix: refactor function handler for follow up auto ack behavior by WilliamBergamin · Pull Request #2424 · slackapi/bolt-js

WilliamBergamin · 2025-02-14T21:47:06Z

Summary

This PR aims to

Increase unit test coverage for the custom function handler
Align the function handler with the middleware pattern used by actions and events
Set up the project to allow simple introduction of auto acknowledgment flag

Manual tests

These changes were manually tested against

bolt-js-custom-step-template -> no breaking change observed 🟢
bolt-ts-starter-template -> no breaking change observed 🟢

Requirements

I've read and understood the Contributing Guidelines and have done my best effort to follow them.
I've read and agree to the Code of Conduct.

codecov · 2025-02-14T21:50:19Z

Codecov Report

Attention: Patch coverage is 96.10390% with 6 lines in your changes missing coverage. Please review.

Project coverage is 92.97%. Comparing base (657416f) to head (f7f210d).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/App.ts	89.28%	5 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2424      +/-   ##
==========================================
+ Coverage   92.59%   92.97%   +0.37%     
==========================================
  Files          36       36              
  Lines        7472     7441      -31     
  Branches      653      652       -1     
==========================================
- Hits         6919     6918       -1     
+ Misses        545      518      -27     
+ Partials        8        5       -3

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

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

…ck-changes

zimeg

@WilliamBergamin Lots of awesome changes are happening here! I'm also hopeful that these will make later additions and future maintenance more clear 🙏 ✨

I left handfuls of comments for now with suggestions and thoughts, but nothing seems to be so blocking. A few notes around typings might be worth considering though!

It took me a bit to familiarize myself with parts of this code, but tomorrow I plan to review the test changes. Thank you in advanced for these changes and care for the code 👾

src/App.ts

zimeg · 2025-04-28T21:52:31Z

src/App.ts

-    // If functionBotAccessToken exists on context, the incoming event is function-related *and* the
-    // user has `attachFunctionToken` enabled. In that case, subsequent calls with the client should
-    // use the function-related/JIT token in lieu of the botToken or userToken.
-    const token = context.functionBotAccessToken ? context.functionBotAccessToken : selectToken(context);


@WilliamBergamin Super nice catch here! It seems like attachFunctionToken might've been ignored before these changes 👁️‍🗨️

📝 Moving all of this logic into selectToken is also a nice change that aligns with the expected behavior noted in #2500!

📣 Following up on this with #2513 since changes to selectToken allow us to share this token in context without unexpected overrides I believe!

Thanks for creating #2513 🙇 I left one comment and I've approved it 🟢

src/App.ts

zimeg · 2025-04-28T23:56:47Z

src/App.ts

    // Factory for say() utility
    const createSay = (channelId: string): SayFn => {
-      const token = selectToken(context);
+      const token = selectToken(context, this.attachFunctionToken);


🤔 Thinking aloud here, but I'm wondering if inlining createSay might allow us to use the same token from below and perhaps even a client from the pool:

bolt-js/src/App.ts

Lines 1117 to 1120 in d61f2cd

// Set say() utility

if (conversationId !== undefined && type !== IncomingEventType.Options) {

listenerArgs.say = createSay(conversationId);

}

AFAICT we do not use this factory function in other places. No blocker since we're overriding the token in these arguments at the moment, though perhaps that wouldn't be needed?

This is a great point 💯 but I think we should dedicate a specific PR for this change, since the scope of these changes is already large

I added a TODO comment for this

src/App.ts

src/CustomFunction.ts

test/types/action.test-d.ts

src/CustomFunction.ts

zimeg

📝 Leaving another note of a related change in mind from another PR!

I'm also wondering if this is a patch change, since at a glance no changes to behavior were noticed in testing. Though I still have not reviewed the tests 😉

zimeg · 2025-04-29T02:25:49Z

src/App.ts

-    // If functionBotAccessToken exists on context, the incoming event is function-related *and* the
-    // user has `attachFunctionToken` enabled. In that case, subsequent calls with the client should
-    // use the function-related/JIT token in lieu of the botToken or userToken.
-    const token = context.functionBotAccessToken ? context.functionBotAccessToken : selectToken(context);


📣 Following up on this with #2513 since changes to selectToken allow us to share this token in context without unexpected overrides I believe!

Co-authored-by: Eden Zimbelman <[email protected]>

zimeg

@WilliamBergamin Thanks so much for sifting through all of those comments! 🤖

Another pass at reviewing and more test runs with a sample app have me excited for these changes.

I left two more comments around documentation and test cases that would be nice to include before merging this, but once the time is right please feel free to merge 🚢 💨

The improvements to test helpers are a nice addition.

Thank you for your patience in these reviews as well 🙏 ✨

src/types/actions/block-action.ts

test/unit/CustomFunction.spec.ts

Co-authored-by: Eden Zimbelman <[email protected]>

fix: refactor function handler for follow up auto ack behavior

9efc199

WilliamBergamin added enhancement M-T: A feature request for new functionality semver:minor labels Feb 14, 2025

WilliamBergamin self-assigned this Feb 14, 2025

WilliamBergamin and others added 2 commits February 17, 2025 16:03

Add more unit test to cover function scoped actions

5c5e84a

Merge branch 'main' into refactor-function-listener-for-future-auto-a…

8422fff

…ck-changes

WilliamBergamin requested review from misscoded and zimeg February 17, 2025 21:39

WilliamBergamin marked this pull request as ready for review February 17, 2025 21:42

WilliamBergamin mentioned this pull request Feb 17, 2025

feat: support for synchronous custom step handling #2408

Merged

4 tasks

Merge branch 'main' into refactor-function-listener-for-future-auto-a…

d61f2cd

…ck-changes

WilliamBergamin removed the request for review from misscoded April 28, 2025 17:58

zimeg reviewed Apr 29, 2025

View reviewed changes

zimeg mentioned this pull request Apr 29, 2025

fix: always attach the function bot access token to the context if available #2513

Merged

2 tasks

zimeg reviewed Apr 29, 2025

View reviewed changes

WilliamBergamin and others added 14 commits April 29, 2025 10:42

Update test/types/action.test-d.ts

6b0c3b9

Co-authored-by: Eden Zimbelman <[email protected]>

Update src/CustomFunction.ts

32f698f

Co-authored-by: Eden Zimbelman <[email protected]>

Update src/CustomFunction.ts

0d7a953

Co-authored-by: Eden Zimbelman <[email protected]>

Add a TODO for create say

46ebf61

Update src/App.ts

3b4342d

Co-authored-by: Eden Zimbelman <[email protected]>

Update src/CustomFunction.ts

28c73d8

Co-authored-by: Eden Zimbelman <[email protected]>

Update src/CustomFunction.ts

425c963

Co-authored-by: Eden Zimbelman <[email protected]>

Update src/CustomFunction.ts

07ef57a

Co-authored-by: Eden Zimbelman <[email protected]>

Update src/CustomFunction.ts

a602dc6

Co-authored-by: Eden Zimbelman <[email protected]>

fix imports

70deb4f

Update src/App.ts

f9b2fae

Co-authored-by: Eden Zimbelman <[email protected]>

Improve client pool behavior

0ba5ea8

depricate types instead of removing them

9e6ef71

Update src/CustomFunction.ts

a01e3f1

Co-authored-by: Eden Zimbelman <[email protected]>

WilliamBergamin requested a review from zimeg April 29, 2025 21:10

zimeg approved these changes Apr 30, 2025

View reviewed changes

src/types/actions/block-action.ts Show resolved Hide resolved

test/unit/CustomFunction.spec.ts Show resolved Hide resolved

test/unit/CustomFunction.spec.ts Show resolved Hide resolved

WilliamBergamin and others added 3 commits April 30, 2025 09:53

Update test/unit/CustomFunction.spec.ts

1b12aa4

Co-authored-by: Eden Zimbelman <[email protected]>

Update test/unit/CustomFunction.spec.ts

e5cde86

Co-authored-by: Eden Zimbelman <[email protected]>

Update src/types/actions/block-action.ts

f7f210d

Co-authored-by: Eden Zimbelman <[email protected]>

WilliamBergamin merged commit 46744f7 into main Apr 30, 2025
18 checks passed

WilliamBergamin deleted the refactor-function-listener-for-future-auto-ack-changes branch April 30, 2025 14:16

WilliamBergamin added this to the 4.3.0 milestone May 7, 2025

hello-ashleyintech modified the milestones: 4.3.0, 4.2.2 May 7, 2025

WilliamBergamin mentioned this pull request May 14, 2025

feat: expose auto acknowledgement flag on the function handler #2334

Closed

2 tasks

	// Set say() utility
	if (conversationId !== undefined && type !== IncomingEventType.Options) {
	listenerArgs.say = createSay(conversationId);
	}

Comments

Conversation

WilliamBergamin commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Manual tests

Requirements

Uh oh!

codecov bot commented Feb 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zimeg Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

zimeg Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

zimeg Apr 28, 2025

Choose a reason for hiding this comment

Uh oh!

WilliamBergamin Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

zimeg Apr 29, 2025

Choose a reason for hiding this comment

Uh oh!

zimeg left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

WilliamBergamin commented Feb 14, 2025 •

edited

Loading

codecov bot commented Feb 14, 2025 •

edited

Loading