`add` command overrides other contributions if given any existing ones
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:
- Set up a project with an
.all-contributorsrccontaining a contributor with >=2 contributions - Run
npx all-contributors-cli addto give them a contribution they already have - 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!
Hi, Just got back to GH after a while!
Were you able to find the problematic lines or find a potential solution?
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.
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'])
},
)
})