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

[JoshuaKGoldberg]

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 #355, but without a certain reproduction. We have that now!

[Berkmann18]

Hi,
Just got back to GH after a while!

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

[JoshuaKGoldberg]

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.

[Floppy]

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

cli/src/contributors/add.js

Line 9 in 74bc388

const remove = types.length < existing.length && same.length

and then

cli/src/contributors/add.js

Line 19 in 74bc388

if (remove) {

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'])
    },
  )
})