numberOfAssignees is overwritten by numberOfReviewers when 0 is set

[koyama-yoshihito]

Describe the bug When numberOfAssignees is set to 0, it is overwritten by the value of numberOfReviewers

To Reproduce Create an auto_assign.yml like this, and place it in a GitHub repository.

addReviewers: true
addAssignees: true

reviewers:
  - reviewerA
  - reviewerB
  - reviewerC

numberOfReviewers: 2

assignees:
  - assigneeA
  - assigneeB

numberOfAssignees: 0

Expected behavior Assignee is assumed to be everyone, but only 2 assignees were set. Meanwhile, reviewers were properly set to 2 as I intended.

Causes According to my research, these lines cause this issue. https://github.com/kentaro-m/auto-assign/blob/9c0678cf0b61cfdb49487c3ca8db96c425cfae91/src/util.ts#L116 https://github.com/kentaro-m/auto-assign/blob/9c0678cf0b61cfdb49487c3ca8db96c425cfae91/src/util.ts#L122 When numberOfAssignees is 0, it is overwritten by numberOfReviewers.

I think this issue will be fixed if we rewrite like this.

numberOfAssignees ?? numberOfReviewers

or

numberOfAssignees === 0 ? 0 : numberOfAssignees || numberOfReviewers

I hope it works well.

Desktop (please complete the following information):

I don't think the local environment matters this issue.

• OS: macOS
• Browser: Chrome
• Version: latest version (chrome)

[kentaro-m]

@koyama-yoshihito Thanks for reporting your issue. I wrote a test to check the behavior and was able to reproduce the problem.

// Failed test
test('adds all assingnees to pull requests when 0 is set to numberOfAssignees', async () => {
    context.config = jest.fn().mockImplementation(async () => {
      return {
        addAssignees: true,
        addReviewers: true,
        numberOfAssignees: 0,
        assignees: ['assigneeA', 'assigneeB'],
        numberOfReviewers: 1,
        reviewers: ['reviewerA', 'reviewerB', 'reviewerC'],
      }
    })

    context.octokit.issues = {
      addAssignees: jest.fn().mockImplementation(async () => {}),
    } as any

    context.octokit.pulls = {
      requestReviewers: jest.fn().mockImplementation(async () => {}),
    } as any

    const addAssigneesSpy = jest.spyOn(context.octokit.issues, 'addAssignees')
    const requestReviewersSpy = jest.spyOn(context.octokit.pulls, 'requestReviewers')

    await handlePullRequest(context)

    // Expected length: 2, Received length: 1
    expect(addAssigneesSpy.mock.calls[0][0]?.assignees).toHaveLength(2)
    expect(requestReviewersSpy.mock.calls[0][0]?.reviewers).toHaveLength(1)
 })

As you researched, there was a problem with the following line:

https://github.com/kentaro-m/auto-assign/blob/9c0678cf0b61cfdb49487c3ca8db96c425cfae91/src/util.ts#L116 https://github.com/kentaro-m/auto-assign/blob/9c0678cf0b61cfdb49487c3ca8db96c425cfae91/src/util.ts#L122

When numberOfAssignees was set to 0, it seemed to be evaluated as a falsy value and numberOfReviewers was used.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Nullish_coalescing_operator#assigning_a_default_value_to_a_variable

I created the PR https://github.com/kentaro-m/auto-assign/pull/178.

[koyama-yoshihito]

@kentaro-m Thank you for the verification. I read the failed test code and saw there's no problems.

And I appreciate your quick fix!!

[kentaro-m]

@koyama-yoshihito PR #178 is merged. I deployed the app for the new version. https://probot.github.io/apps/auto-assign/

[koyama-yoshihito]

@kentaro-m Thank you very much for your continued support. auto-assign is a very useful tool. I'll keep using it :)