From 1d60392f15255fdbd5f005cb5051b3844bfb5b7f Mon Sep 17 00:00:00 2001 From: Manuel de la Torre Date: Sun, 11 Feb 2024 10:57:21 -0600 Subject: [PATCH] v2.13.0: Added exclude option to filter reviewers --- CHANGELOG.md | 7 ++ README.md | 28 ++++-- action.yml | 3 + dist/index.js | 99 ++++++++++++++----- package.json | 2 +- src/config/index.js | 9 +- src/execute.js | 2 +- src/index.js | 1 + .../__tests__/filterReviewer.test.js | 30 ++++++ .../getReviewers/__tests__/index.test.js | 12 ++- .../__tests__/parseExclude.test.js | 28 ++++++ .../getReviewers/filterReviewer.js | 5 + src/interactors/getReviewers/index.js | 15 ++- src/interactors/getReviewers/parseExclude.js | 17 ++++ .../postTeamsMessage/__tests__/index.test.js | 8 +- src/services/splitter/__tests__/base.test.js | 24 +++-- src/services/splitter/__tests__/slack.test.js | 10 +- src/services/splitter/__tests__/teams.test.js | 5 +- src/services/splitter/base.js | 16 +-- src/services/splitter/slack.js | 11 ++- src/services/splitter/teams.js | 7 +- 21 files changed, 267 insertions(+), 72 deletions(-) create mode 100644 src/interactors/getReviewers/__tests__/filterReviewer.test.js create mode 100644 src/interactors/getReviewers/__tests__/parseExclude.test.js create mode 100644 src/interactors/getReviewers/filterReviewer.js create mode 100644 src/interactors/getReviewers/parseExclude.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 22681d8..c77ee2b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,13 @@ # Changelog All notable changes to this project will be documented in this file. +## [2.13.0] - 2024-02-10 +### Added +- `exclude` option to exclude specific users from the stats. + +### Fixed +- Reduces the block size in the Slack messages to prevent hitting the characters limit. + ## [2.12.0] - 2024-02-06 ### Changed - [#85](https://github.com/flowwer-dev/pull-request-stats/pull/85) Use Node v20 (by [antonindrawan](https://github.com/antonindrawan)) diff --git a/README.md b/README.md index 90eab10..2de0725 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,8 @@ Just add this action to one of your [workflow files](https://docs.github.com/en/ uses: flowwer-dev/pull-request-stats@master ``` +If you are getting an empty table or an error, check the [troubleshooting section](#troubleshooting). + ### Action inputs The possible inputs for this action are: @@ -47,18 +49,19 @@ The possible inputs for this action are: | --------- | ----------- | ------- | | `token` | A [Personal Access Token](https://docs.github.com/en/github/authenticating-to-github/creating-a-personal-access-token) with `repo` permissions. Required to calculate stats for an organization or multiple repos. | `GITHUB_TOKEN` | | `repositories` | A comma-separated list of GitHub repositories to calculate the stats, e.g. `username/repo1,username/repo2`. When specifying other repo(s), **it is mandatory to pass a Personal Access Token** in the `token` parameter.| Current repository | -| `organization` | If you prefer, you may specify your organization's name to calculate the stats across all of its repos. When specifying an organization, **it is mandatory to pass a Personal Access Token** in the `token` parameter. | `null`| +| `organization` | If you prefer, you may specify your organization's name to calculate the stats across all of its repos. When specifying an organization, **it is mandatory to pass a Personal Access Token** in the `token` parameter. | `null` | | `period` | The period used to calculate the stats, expressed in days. | `30` | -| `limit` | The maximum number of rows to display in the table. A value of `0` means unlimited. |`0`| +| `limit` | The maximum number of rows to display in the table. A value of `0` means unlimited. | `0` | | `charts` | Whether to add a chart to the start. Possible values: `true` or `false`. | `false` | | `disableLinks` | If `true`, removes the links to the detailed charts. Possible values: `true` or `false`. | `false` | | `sortBy` | The column used to sort the data. Possible values: `REVIEWS`, `TIME`, `COMMENTS`. | `REVIEWS` | | `publishAs` | Where to publish the results. Possible values: as a `COMMENT`, on the pull request `DESCRIPTION`, or publish `NONE`. | `COMMENT` | +| `exclude` | A comma-separated list of usernames (case-insensitive) to be excluded from the results (e.g. `username1,username2`), or a regular expression enclosed between slashes (eg. `/^bot/i` will exclude all usernames that begin with "bot"). | `null` | | `telemetry` | Indicates if the action is allowed to send monitoring data to the developer. This data is [minimal](/src/services/telemetry/sendStart.js) and helps me improve this action. **This option is a premium feature reserved for [sponsors](#premium-features-).** |`true`| -| `slackWebhook` | **🔥 New.** A Slack webhook URL to post resulting stats. **This option is a premium feature reserved for [sponsors](#premium-features-).** See [full documentation here](/docs/slack.md). |`null`| -| `slackChannel` | The Slack channel where stats will be posted. Include the `#` character (eg. `#mychannel`). Required when a `slackWebhook` is configured. |`null`| -| `teamsWebhook` | **🔥 New.** A Microsoft Teams webhook URL to post resulting stats. **This option is a premium feature reserved for [sponsors](#premium-features-).** See [full documentation here](/docs/teams.md). |`null`| -| `webhook` | **🔥 New.** A webhook URL to send the resulting stats as JSON (integrate with Zapier, IFTTT...). See [full documentation here](/docs/webhook.md). |`null`| +| `slackWebhook` | **🔥 New.** A Slack webhook URL to post resulting stats. **This option is a premium feature reserved for [sponsors](#premium-features-).** See [full documentation here](/docs/slack.md). | `null` | +| `slackChannel` | The Slack channel where stats will be posted. Include the `#` character (eg. `#mychannel`). Required when a `slackWebhook` is configured. | `null` | +| `teamsWebhook` | **🔥 New.** A Microsoft Teams webhook URL to post resulting stats. **This option is a premium feature reserved for [sponsors](#premium-features-).** See [full documentation here](/docs/teams.md). | `null` | +| `webhook` | **🔥 New.** A webhook URL to send the resulting stats as JSON (integrate with Zapier, IFTTT...). See [full documentation here](/docs/webhook.md). | `null` | ### Action outputs @@ -179,6 +182,19 @@ Check the guide for the tool you want to integrate: 1. Make sure the repositories have pull request reviews during the configured `period`. 2. When specifying `repositories` or `organization` parameters, a [Personal Access Token](https://docs.github.com/en/github/authenticating-to-github/creating-a-personal-access-token) is required in the `token` parameter. 3. If providing a Personal Access Token, ensure it has the `repo` permission for the projects you want. + 4. If you are not providing a Personal Access Token (thus, the action is using the default `GITHUB_TOKEN`), make sure the job has the `contents: read` and `pull-requests: write` [permissions](https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs#defining-access-for-the-github_token-scopes) While these permissions are typically provided by default, certain organizations may customize or overwrite them. + + ```yml + jobs: + stats: + runs-on: ubuntu-latest + permissions: + contents: read + pull-requests: write + steps: + - name: Run pull request stats + uses: flowwer-dev/pull-request-stats@master + ```
diff --git a/action.yml b/action.yml index f6116db..033d5cf 100644 --- a/action.yml +++ b/action.yml @@ -35,6 +35,9 @@ inputs: description: 'Where to publish the results. Possible values: "COMMENT", "DESCRIPTION" or "NONE"' required: false default: 'COMMENT' + exclude: + description: 'A list or regular expression to exclude users from the stats' + required: false disableLinks: description: 'Prevents from adding any external links in the stats' required: false diff --git a/dist/index.js b/dist/index.js index 11721d3..a337c00 100644 --- a/dist/index.js +++ b/dist/index.js @@ -41192,11 +41192,14 @@ module.exports = parseParams /***/ 1855: /***/ ((module) => { -const getSlackCharsLimit = () => 39000; -const getTeamsBytesLimit = () => 27000; +const getSlackLimits = () => ({ + chars: 30_000, + blocks: 50, +}); +const getTeamsBytesLimit = () => 27_000; module.exports = { - getSlackCharsLimit, + getSlackLimits, getTeamsBytesLimit, }; @@ -41288,7 +41291,7 @@ const run = async (params) => { }); core.info(`Found ${pulls.length} pull requests to analyze`); - const reviewersRaw = getReviewers(pulls); + const reviewersRaw = getReviewers(pulls, { excludeStr: params.excludeStr }); core.info(`Analyzed stats for ${reviewersRaw.length} pull request reviewers`); const reviewers = setUpReviewers({ @@ -42062,6 +42065,18 @@ module.exports = (reviews) => { }; +/***/ }), + +/***/ 3966: +/***/ ((module) => { + +module.exports = (exclude, username) => { + if (exclude.test) return !exclude.test(username); + if (exclude.includes) return !exclude.includes(username); + return true; +}; + + /***/ }), /***/ 9633: @@ -42100,12 +42115,43 @@ module.exports = (pulls) => { /***/ ((module, __unused_webpack_exports, __nccwpck_require__) => { const calculateReviewsStats = __nccwpck_require__(3753); +const filterReviewer = __nccwpck_require__(3966); +const parseExclude = __nccwpck_require__(7960); const groupReviews = __nccwpck_require__(9633); -module.exports = (pulls) => groupReviews(pulls).map(({ author, reviews }) => { - const stats = calculateReviewsStats(reviews); - return { author, reviews, stats }; -}); +module.exports = (pulls, { excludeStr } = {}) => { + const exclude = parseExclude(excludeStr); + return groupReviews(pulls) + .filter(({ author }) => filterReviewer(exclude, author.login)) + .map(({ author, reviews }) => { + const stats = calculateReviewsStats(reviews); + return { author, reviews, stats }; + }); +}; + + +/***/ }), + +/***/ 7960: +/***/ ((module) => { + +const REGEXP_PATTERN = /^\/.+\/[a-z]*$/; + +// Github usernames can only contain alphanumeric characters and dashes (-) +const sanitize = (str = '') => (str || '').replace(/[^-a-zA-Z0-9]/g, '').toLowerCase(); + +const isRegExp = (str) => REGEXP_PATTERN.test(str); + +const parseRegExp = (str) => { + const [pattern, flags] = str.split('/').slice(1); + return new RegExp(pattern, flags); +}; + +module.exports = (excludeStr) => { + if (!sanitize(excludeStr)) return []; + if (isRegExp(excludeStr)) return parseRegExp(excludeStr); + return excludeStr.split(',').map(sanitize); +}; /***/ }), @@ -43086,13 +43132,10 @@ module.exports = { /***/ ((module) => { class BaseSplitter { - constructor({ message, limit = null }) { - this.limit = limit || this.constructor.defaultLimit(); + constructor({ message, limit = null, maxBlocksLength = null }) { this.message = message; - } - - static defaultLimit() { - return Infinity; + this.limit = limit || Infinity; + this.maxBlocksLength = maxBlocksLength || Infinity; } get blockSize() { @@ -43118,10 +43161,13 @@ class BaseSplitter { const blocksCount = this.constructor.getBlocksCount(message); const currentSize = this.constructor.calculateSize(message); const diff = currentSize - this.limit; - if (diff < 0 || blocksCount === 1) return 0; + const onLimit = diff <= 0 && blocksCount <= this.maxBlocksLength; + if (onLimit || blocksCount === 1) return 0; const blocksSpace = Math.ceil(diff / this.blockSize); - const blocksToSplit = Math.max(1, Math.min(blocksCount - 1, blocksSpace)); + const upperBound = Math.min(blocksCount - 1, blocksSpace); + const exceedingBlocks = Math.max(0, blocksCount - this.maxBlocksLength); + const blocksToSplit = Math.max(1, upperBound, exceedingBlocks); const [firsts] = this.constructor.splitBlocks(message, blocksToSplit); return this.calculateBlocksToSplit(firsts) || blocksToSplit; } @@ -43165,13 +43211,18 @@ module.exports = { /***/ 2843: /***/ ((module, __unused_webpack_exports, __nccwpck_require__) => { -const { getSlackCharsLimit } = __nccwpck_require__(1855); +const { getSlackLimits } = __nccwpck_require__(1855); const { median } = __nccwpck_require__(9988); const BaseSplitter = __nccwpck_require__(7027); class SlackSplitter extends BaseSplitter { - static defaultLimit() { - return getSlackCharsLimit(); + constructor(args = {}) { + const limits = getSlackLimits(); + super({ + ...args, + limit: limits.chars, + maxBlocksLength: limits.blocks, + }); } static splitBlocks(message, count) { @@ -43212,8 +43263,11 @@ const { median } = __nccwpck_require__(9988); const BaseSplitter = __nccwpck_require__(7027); class TeamsSplitter extends BaseSplitter { - static defaultLimit() { - return getTeamsBytesLimit(); + constructor(args = {}) { + super({ + ...args, + limit: getTeamsBytesLimit(), + }); } static splitBlocks(body, count) { @@ -47978,7 +48032,7 @@ module.exports = JSON.parse('{"name":"mixpanel","description":"A simple server-s /***/ ((module) => { "use strict"; -module.exports = JSON.parse('{"name":"pull-request-stats","version":"2.12.0","description":"Github action to print relevant stats about Pull Request reviewers","main":"dist/index.js","type":"commonjs","scripts":{"build":"eslint src && ncc build src/index.js -o dist -a","test":"jest","lint":"eslint ./"},"keywords":[],"author":"Manuel de la Torre","license":"MIT","jest":{"testEnvironment":"node","testMatch":["**/?(*.)+(spec|test).[jt]s?(x)"]},"dependencies":{"@actions/core":"^1.10.1","@actions/github":"^6.0.0","axios":"^1.6.7","humanize-duration":"^3.31.0","i18n-js":"^3.9.2","jsurl":"^0.1.5","lodash.get":"^4.4.2","markdown-table":"^2.0.0","mixpanel":"^0.18.0"},"devDependencies":{"@vercel/ncc":"^0.38.1","eslint":"^8.56.0","eslint-config-airbnb-base":"^15.0.0","eslint-plugin-import":"^2.29.1","eslint-plugin-jest":"^27.6.3","jest":"^29.7.0"},"funding":"https://github.com/sponsors/manuelmhtr","packageManager":"yarn@4.1.0"}'); +module.exports = JSON.parse('{"name":"pull-request-stats","version":"2.13.0","description":"Github action to print relevant stats about Pull Request reviewers","main":"dist/index.js","type":"commonjs","scripts":{"build":"eslint src && ncc build src/index.js -o dist -a","test":"jest","lint":"eslint ./"},"keywords":[],"author":"Manuel de la Torre","license":"MIT","jest":{"testEnvironment":"node","testMatch":["**/?(*.)+(spec|test).[jt]s?(x)"]},"dependencies":{"@actions/core":"^1.10.1","@actions/github":"^6.0.0","axios":"^1.6.7","humanize-duration":"^3.31.0","i18n-js":"^3.9.2","jsurl":"^0.1.5","lodash.get":"^4.4.2","markdown-table":"^2.0.0","mixpanel":"^0.18.0"},"devDependencies":{"@vercel/ncc":"^0.38.1","eslint":"^8.56.0","eslint-config-airbnb-base":"^15.0.0","eslint-plugin-import":"^2.29.1","eslint-plugin-jest":"^27.6.3","jest":"^29.7.0"},"funding":"https://github.com/sponsors/manuelmhtr","packageManager":"yarn@4.1.0"}'); /***/ }), @@ -48086,6 +48140,7 @@ const getParams = () => { disableLinks: core.getBooleanInput('disableLinks') || core.getBooleanInput('disable-links'), pullRequestId: getPrId(), limit: parseInt(core.getInput('limit'), 10), + excludeStr: core.getInput('exclude'), telemetry: core.getBooleanInput('telemetry'), webhook: core.getInput('webhook'), slack: { diff --git a/package.json b/package.json index 6932745..aabb455 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "pull-request-stats", - "version": "2.12.0", + "version": "2.13.0", "description": "Github action to print relevant stats about Pull Request reviewers", "main": "dist/index.js", "type": "commonjs", diff --git a/src/config/index.js b/src/config/index.js index f220e78..98cd278 100644 --- a/src/config/index.js +++ b/src/config/index.js @@ -1,7 +1,10 @@ -const getSlackCharsLimit = () => 39000; -const getTeamsBytesLimit = () => 27000; +const getSlackLimits = () => ({ + chars: 30_000, + blocks: 50, +}); +const getTeamsBytesLimit = () => 27_000; module.exports = { - getSlackCharsLimit, + getSlackLimits, getTeamsBytesLimit, }; diff --git a/src/execute.js b/src/execute.js index 23984b9..4b6515f 100644 --- a/src/execute.js +++ b/src/execute.js @@ -50,7 +50,7 @@ const run = async (params) => { }); core.info(`Found ${pulls.length} pull requests to analyze`); - const reviewersRaw = getReviewers(pulls); + const reviewersRaw = getReviewers(pulls, { excludeStr: params.excludeStr }); core.info(`Analyzed stats for ${reviewersRaw.length} pull request reviewers`); const reviewers = setUpReviewers({ diff --git a/src/index.js b/src/index.js index 774ba60..b596331 100644 --- a/src/index.js +++ b/src/index.js @@ -37,6 +37,7 @@ const getParams = () => { disableLinks: core.getBooleanInput('disableLinks') || core.getBooleanInput('disable-links'), pullRequestId: getPrId(), limit: parseInt(core.getInput('limit'), 10), + excludeStr: core.getInput('exclude'), telemetry: core.getBooleanInput('telemetry'), webhook: core.getInput('webhook'), slack: { diff --git a/src/interactors/getReviewers/__tests__/filterReviewer.test.js b/src/interactors/getReviewers/__tests__/filterReviewer.test.js new file mode 100644 index 0000000..082140a --- /dev/null +++ b/src/interactors/getReviewers/__tests__/filterReviewer.test.js @@ -0,0 +1,30 @@ +const filterReviewer = require('../filterReviewer'); + +describe('Interactors | getReviewers | .filterReviewer', () => { + const reviewers = [ + 'manuelmhtr', + 'jartmez', + 'bot1', + 'bot2', + ]; + + it('filters out reviewers by a list of usernames', () => { + const exclude = ['manuelmhtr', 'jartmez']; + const results = reviewers.filter((reviewer) => filterReviewer(exclude, reviewer)); + expect(results.length).toEqual(2); + expect(results).toEqual([ + 'bot1', + 'bot2', + ]); + }); + + it('filters out reviewers by a regular expression', () => { + const exclude = /bot/; + const results = reviewers.filter((reviewer) => filterReviewer(exclude, reviewer)); + expect(results.length).toEqual(2); + expect(results).toEqual([ + 'manuelmhtr', + 'jartmez', + ]); + }); +}); diff --git a/src/interactors/getReviewers/__tests__/index.test.js b/src/interactors/getReviewers/__tests__/index.test.js index c47538a..146c198 100644 --- a/src/interactors/getReviewers/__tests__/index.test.js +++ b/src/interactors/getReviewers/__tests__/index.test.js @@ -1,13 +1,13 @@ const input = require('./mocks/pullRequests'); const getReviewers = require('../index'); +const getAuthors = (reviewers) => reviewers.map((r) => r.author.login); + describe('Interactors | getReviewers', () => { it('groups reviews by author and calculate its stats', () => { const result = getReviewers(input); expect(result.length).toEqual(2); - - const authors = result.map((r) => r.author.login); - expect(authors).toContain('manuelmhtr', 'jartmez'); + expect(getAuthors(result)).toContain('manuelmhtr', 'jartmez'); result.forEach((reviewer) => { expect(reviewer).toHaveProperty('author'); @@ -21,4 +21,10 @@ describe('Interactors | getReviewers', () => { expect(reviewer.stats).toHaveProperty('timeToReview'); }); }); + + it('excludes reviewers when the option is passed', () => { + const result = getReviewers(input, { excludeStr: 'manuelmhtr' }); + expect(result.length).toEqual(1); + expect(getAuthors(result)).not.toContain('manuelmhtr'); + }); }); diff --git a/src/interactors/getReviewers/__tests__/parseExclude.test.js b/src/interactors/getReviewers/__tests__/parseExclude.test.js new file mode 100644 index 0000000..a1ad802 --- /dev/null +++ b/src/interactors/getReviewers/__tests__/parseExclude.test.js @@ -0,0 +1,28 @@ +const parseExclude = require('../parseExclude'); + +describe('Interactors | getReviewers | .parseExclude', () => { + it('returns an empty array when the input does not contain usernames or regexp', () => { + expect(parseExclude()).toEqual([]); + expect(parseExclude(null)).toEqual([]); + expect(parseExclude('')).toEqual([]); + expect(parseExclude('@')).toEqual([]); + expect(parseExclude('/@/%^')).toEqual([]); + }); + + it('splits usernames into an array', () => { + expect(parseExclude('user1,user2')).toEqual(['user1', 'user2']); + }); + + it('removes spaces and converts usernames to lowercase', () => { + expect(parseExclude('User1, USER2')).toEqual(['user1', 'user2']); + }); + + it('removes invalid characters from usernames', () => { + expect(parseExclude('@user1, @user%2ñ, keep-dashes-ok')).toEqual(['user1', 'user2', 'keep-dashes-ok']); + }); + + it('parses regexp strings', () => { + expect(parseExclude('/user[0-9]/')).toEqual(/user[0-9]/); + expect(parseExclude('/^bot-.*/ig')).toEqual(/^bot-.*/ig); + }); +}); diff --git a/src/interactors/getReviewers/filterReviewer.js b/src/interactors/getReviewers/filterReviewer.js new file mode 100644 index 0000000..d076fd6 --- /dev/null +++ b/src/interactors/getReviewers/filterReviewer.js @@ -0,0 +1,5 @@ +module.exports = (exclude, username) => { + if (exclude.test) return !exclude.test(username); + if (exclude.includes) return !exclude.includes(username); + return true; +}; diff --git a/src/interactors/getReviewers/index.js b/src/interactors/getReviewers/index.js index 2112927..770e192 100644 --- a/src/interactors/getReviewers/index.js +++ b/src/interactors/getReviewers/index.js @@ -1,7 +1,14 @@ const calculateReviewsStats = require('./calculateReviewsStats'); +const filterReviewer = require('./filterReviewer'); +const parseExclude = require('./parseExclude'); const groupReviews = require('./groupReviews'); -module.exports = (pulls) => groupReviews(pulls).map(({ author, reviews }) => { - const stats = calculateReviewsStats(reviews); - return { author, reviews, stats }; -}); +module.exports = (pulls, { excludeStr } = {}) => { + const exclude = parseExclude(excludeStr); + return groupReviews(pulls) + .filter(({ author }) => filterReviewer(exclude, author.login)) + .map(({ author, reviews }) => { + const stats = calculateReviewsStats(reviews); + return { author, reviews, stats }; + }); +}; diff --git a/src/interactors/getReviewers/parseExclude.js b/src/interactors/getReviewers/parseExclude.js new file mode 100644 index 0000000..d11ad3c --- /dev/null +++ b/src/interactors/getReviewers/parseExclude.js @@ -0,0 +1,17 @@ +const REGEXP_PATTERN = /^\/.+\/[a-z]*$/; + +// Github usernames can only contain alphanumeric characters and dashes (-) +const sanitize = (str = '') => (str || '').replace(/[^-a-zA-Z0-9]/g, '').toLowerCase(); + +const isRegExp = (str) => REGEXP_PATTERN.test(str); + +const parseRegExp = (str) => { + const [pattern, flags] = str.split('/').slice(1); + return new RegExp(pattern, flags); +}; + +module.exports = (excludeStr) => { + if (!sanitize(excludeStr)) return []; + if (isRegExp(excludeStr)) return parseRegExp(excludeStr); + return excludeStr.split(',').map(sanitize); +}; diff --git a/src/interactors/postTeamsMessage/__tests__/index.test.js b/src/interactors/postTeamsMessage/__tests__/index.test.js index 8b73e5a..cd8bede 100644 --- a/src/interactors/postTeamsMessage/__tests__/index.test.js +++ b/src/interactors/postTeamsMessage/__tests__/index.test.js @@ -3,11 +3,9 @@ const buildMessage = require('../buildMessage'); const buildPayload = require('../buildPayload'); const postTeamsMessage = require('../index'); -const MESSAGE = { - blocks: [ - { type: 'section', text: 'MESSAGE' }, - ], -}; +const MESSAGE = [ + { type: 'section', text: 'MESSAGE' }, +]; jest.mock('../../../fetchers', () => ({ postToWebhook: jest.fn(() => Promise.resolve()) })); jest.mock('../buildMessage', () => jest.fn()); diff --git a/src/services/splitter/__tests__/base.test.js b/src/services/splitter/__tests__/base.test.js index 3f45ac5..25ddd7a 100644 --- a/src/services/splitter/__tests__/base.test.js +++ b/src/services/splitter/__tests__/base.test.js @@ -10,15 +10,10 @@ describe('Services | Splitter | BaseSplitter', () => { expect(splitter.message).toEqual(message); }); - it('assigns a default limit when not specified', () => { + it('assigns default limits when not specified', () => { const splitter = new BaseSplitter({ message: '' }); - expect(splitter.limit).toEqual(BaseSplitter.defaultLimit()); - }); - }); - - describe('.defaultLimit', () => { - it('returns the highest possible number', () => { - expect(BaseSplitter.defaultLimit()).toEqual(Infinity); + expect(splitter.limit).toEqual(Infinity); + expect(splitter.maxBlocksLength).toEqual(Infinity); }); }); @@ -179,5 +174,18 @@ describe('Services | Splitter | BaseSplitter', () => { [block5, block6, block7], ]); }); + + it('allows a maximum blocks length to be specified', () => { + const limit = Infinity; + const maxBlocksLength = 2; + const splitter = new BaseSplitter({ message, limit, maxBlocksLength }); + const result = splitter.chunks; + expect(result).toEqual([ + [block1], + [block2, block3], + [block4, block5], + [block6, block7], + ]); + }); }); }); diff --git a/src/services/splitter/__tests__/slack.test.js b/src/services/splitter/__tests__/slack.test.js index 46d8647..c7f14ba 100644 --- a/src/services/splitter/__tests__/slack.test.js +++ b/src/services/splitter/__tests__/slack.test.js @@ -2,7 +2,7 @@ const SlackSplitter = require('../slack'); const { median } = require('../../../utils'); jest.mock('../../../config', () => ({ - getSlackCharsLimit: () => 100, + getSlackLimits: () => ({ chars: 100, blocks: 5 }), })); describe('Services | Splitter | SlackSplitter', () => { @@ -37,9 +37,11 @@ describe('Services | Splitter | SlackSplitter', () => { ], }; - describe('.defaultLimit', () => { - it('returns limit from config', () => { - expect(SlackSplitter.defaultLimit()).toEqual(100); + describe('limits', () => { + it('returns limits from config', () => { + const splitter = new SlackSplitter(); + expect(splitter.limit).toEqual(100); + expect(splitter.maxBlocksLength).toEqual(5); }); }); diff --git a/src/services/splitter/__tests__/teams.test.js b/src/services/splitter/__tests__/teams.test.js index 2c3fcdc..84ed782 100644 --- a/src/services/splitter/__tests__/teams.test.js +++ b/src/services/splitter/__tests__/teams.test.js @@ -37,9 +37,10 @@ describe('Services | Splitter | TeamsSplitter', () => { block7, ]; - describe('.defaultLimit', () => { + describe('limits', () => { it('returns limit from config', () => { - expect(TeamsSplitter.defaultLimit()).toEqual(5000); + const splitter = new TeamsSplitter(); + expect(splitter.limit).toEqual(5000); }); }); diff --git a/src/services/splitter/base.js b/src/services/splitter/base.js index f0246db..fdc8110 100644 --- a/src/services/splitter/base.js +++ b/src/services/splitter/base.js @@ -1,11 +1,8 @@ class BaseSplitter { - constructor({ message, limit = null }) { - this.limit = limit || this.constructor.defaultLimit(); + constructor({ message, limit = null, maxBlocksLength = null }) { this.message = message; - } - - static defaultLimit() { - return Infinity; + this.limit = limit || Infinity; + this.maxBlocksLength = maxBlocksLength || Infinity; } get blockSize() { @@ -31,10 +28,13 @@ class BaseSplitter { const blocksCount = this.constructor.getBlocksCount(message); const currentSize = this.constructor.calculateSize(message); const diff = currentSize - this.limit; - if (diff < 0 || blocksCount === 1) return 0; + const onLimit = diff <= 0 && blocksCount <= this.maxBlocksLength; + if (onLimit || blocksCount === 1) return 0; const blocksSpace = Math.ceil(diff / this.blockSize); - const blocksToSplit = Math.max(1, Math.min(blocksCount - 1, blocksSpace)); + const upperBound = Math.min(blocksCount - 1, blocksSpace); + const exceedingBlocks = Math.max(0, blocksCount - this.maxBlocksLength); + const blocksToSplit = Math.max(1, upperBound, exceedingBlocks); const [firsts] = this.constructor.splitBlocks(message, blocksToSplit); return this.calculateBlocksToSplit(firsts) || blocksToSplit; } diff --git a/src/services/splitter/slack.js b/src/services/splitter/slack.js index 81e3771..33fc859 100644 --- a/src/services/splitter/slack.js +++ b/src/services/splitter/slack.js @@ -1,10 +1,15 @@ -const { getSlackCharsLimit } = require('../../config'); +const { getSlackLimits } = require('../../config'); const { median } = require('../../utils'); const BaseSplitter = require('./base'); class SlackSplitter extends BaseSplitter { - static defaultLimit() { - return getSlackCharsLimit(); + constructor(args = {}) { + const limits = getSlackLimits(); + super({ + ...args, + limit: limits.chars, + maxBlocksLength: limits.blocks, + }); } static splitBlocks(message, count) { diff --git a/src/services/splitter/teams.js b/src/services/splitter/teams.js index e57d5c3..5b42332 100644 --- a/src/services/splitter/teams.js +++ b/src/services/splitter/teams.js @@ -3,8 +3,11 @@ const { median } = require('../../utils'); const BaseSplitter = require('./base'); class TeamsSplitter extends BaseSplitter { - static defaultLimit() { - return getTeamsBytesLimit(); + constructor(args = {}) { + super({ + ...args, + limit: getTeamsBytesLimit(), + }); } static splitBlocks(body, count) {