cli icon indicating copy to clipboard operation
cli copied to clipboard

`add` command overrides other contributions if given any existing ones

Open JoshuaKGoldberg opened this issue 11 months ago • 3 comments

Describe the bug

Running npx all-contributors-cli add <username> <...contributions> with any existing contributions seems to override all others

To Reproduce

See https://github.com/JoshuaKGoldberg/repros/tree/all-contributors-cli-adding-existing-contributions.

Steps to reproduce the behavior:

  1. Set up a project with an .all-contributorsrc containing a contributor with >=2 contributions
  2. Run npx all-contributors-cli add to give them a contribution they already have
  3. See that their list of contributions has shrunk to include only the existing contribution

Expected behavior Their new list of contributions should contain all added and existing contributions.

Additional context Originally filed as https://github.com/all-contributors/cli/issues/355, but without a certain reproduction. We have that now!

JoshuaKGoldberg avatar Mar 28 '25 19:03 JoshuaKGoldberg

Hi, Just got back to GH after a while!

Were you able to find the problematic lines or find a potential solution?

Berkmann18 avatar Aug 10 '25 19:08 Berkmann18

Oh hey great to see you back! 🙌

No, I was waiting for confirmation this was a legitimate bug and didn't take a look. I'm mostly out this week but can take a look later this month or the next if nobody else has by then.

JoshuaKGoldberg avatar Aug 12 '25 13:08 JoshuaKGoldberg

I've been having a look at this, as I ran into it while trying to automate bug crediting, and the problem is that formatContrtbutions is intentionally designed to overwrite the list if there's any overlap between the existing and new lists. You can see it at https://github.com/all-contributors/cli/blob/74bc388bd6f0ae2658e6495e9d3781d737438a97/src/contributors/add.js#L9 and then https://github.com/all-contributors/cli/blob/74bc388bd6f0ae2658e6495e9d3781d737438a97/src/contributors/add.js#L19

I've written a test that replicates the issue, and it's not a difficult fix. Problem is that making the change would mean that add no longer removes contribution types. As this will be an actual change in the behaviour of the method, not a bugfix as such, I'm reticent to just make changes and send a PR without checking in here first. Thoughts?

Test case:

test(`should preserve an existing contributor's contributions if an existing type is re-added`, () => {
  const username = 'login1'
  const options = {
    contributors: [
      {
        "login": username,
        "contributions": ["code", "bug"]
      }
    ]
  }
  return addContributor(options, username, ['code'], mockInfoFetcher).then(
    contributors => {
      expect(contributors).toHaveLength(options.contributors.length)
      expect(contributors[0]["contributions"]).toEqual(['code', 'bug'])
    },
  )
})

Floppy avatar Oct 17 '25 15:10 Floppy