Throw error if no invite has been removed when it should

[maxceem]

When we send a Kafka message to update or delete invite, in both cases we delete invite from the ES index as per logic here https://github.com/topcoder-platform/project-processor-es/blob/develop/src/services/ProcessorServiceProjectMemberInvite.js#L57-L63

At the moment if no invite has been found and removed handlers to update or delete still return success.

Instead of this, if no invite has been found and removed, the handlers should throw an error:
Invite with email "{email}" and userId "{userId}" is not found and not removed.

Create a 2 unit tests that would test this case, one for update handler and one for delete handler.

[narekcat]

Hello @maxceem .
If there aren't invites, invites array is empty, and we try to update or remove invite, need handles throw an error ?
Regards.

[maxceem]

Yes, if there is no invites at all, or there are some invites but we didn't find the invite to romove, then we have to throw an error.

[narekcat]

Ok, thank you.

[narekcat]

@maxceem Also there are two test cases that tries to update or delete invite by it's id, but in code there isn't check for invite id. Need I delete unnecessary test cases or add checking rule for invite id ?
First test case: https://github.com/topcoder-platform/project-processor-es/blob/develop/test/e2e/processor.project.index.test.js#L887-L893
Second test case: https://github.com/topcoder-platform/project-processor-es/blob/develop/test/e2e/processor.project.index.test.js#L905-L911

[maxceem]

That's interesting finding @narekcat.

• Let's update the logic to remove the invitation by id instead of by email or userId.
• Still, implement throwing an error if no invite has been removed
• Update these 2 unit tests, after fixing this issue there suppose to be error thrown when we try to delete invite with not found id.

Does it make sense?

[narekcat]

Yes, I'll change the error message to this one Invite with id "{id}" is not found and not removed.
Is it ok ?

[narekcat]

But also I can just add logic for invite id, while keeping email and userId.

[maxceem]

Yes, I'll change the error message to this one Invite with id "{id}" is not found and not removed.
Is it ok ?

Yes, please.

But also I can just add logic for invite id, while keeping email and userId.

No, let's remove logic for email and userId it would keep this place more straigforward.

If there is more work for this, we can adjust the prize for this issue accordingly.

[narekcat]

No I'll finish in less than an hour.

[narekcat]

@maxceem I have finished this issue. I had created pull request for issue topcoder-archive/connect-app#3723 using my fork and now that pull request contains both of changes. Is it ok ?

[maxceem]

@narekcat No, please create separate pull requests with the changes to each issue.

[narekcat]

@maxceem I have created separate pull request for this issue and I have remove unnecessary commits from other pull request. Is it ok ? Need I create new branch and pull request for issue topcoder-archive/connect-app#3723 ?

[maxceem]

@narekcat all looks good now: 2 separate pull requests and each of them contains only the changes for the corresponding fix and no other changes. This is what we need.

You don't have to create a new branch and can use develop as it's your fork and you can use any strcuture there. Just don't forget that you cannot push to develop branch now, or such changes would be added to pull request, then it would be not good :-)

[narekcat]

@maxceem Ok, no problem.