Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: delete entry services refactoring #191

Closed
wants to merge 6 commits into from

Conversation

davidzlu
Copy link
Collaborator

@davidzlu davidzlu commented Nov 8, 2024

This PR refactors delete operations in controllers/entry into separate functions and moves them to a new file /models/services/entry/entry.ts. It also adds unit tests for the delete service functions.

In order to test deletes with the memory mongodb, I needed to deploy it as a replica set instead of as a standalone instance. This required changes to jest.setup.cjs. Although all unit tests pass, occasionally on teardown, the following error will appear:

Ran all test suites.
Teardown: Dropping test database...
Error: read ECONNRESET
    at TCP.onStreamRead (node:internal/stream_base_commons:218:20) {
  errno: -4077,
  code: 'ECONNRESET',
  syscall: 'read'
}

This doesn't seem to affect anything, but I haven't been able to figure out why exactly it's happening. My best guess is that jest closes before every node in the replica set disconnects. This doesn't break anything, and it doesn't even affect the unit tests because it happens after they all finish running, but it's odd.

Move updateEntryAnalysis logic into service layer. Able to reuse
much of the code. Wrote one unit test for happy path as previous
paths already covered.
Changed update method to use pulled entryConversation instead of calling
findByIdAndUpdate.
Transactions only allowed on replica sets, so need to first configure
memory mongodb as replica set, otherwise tests will fail
Set up replica set with 3 nodes in jest.setup.cjs for delete tests
When running test suite with replica set, there's occasionally a
ECONNRESET error, but it's not the same number of errors, and appears
inconsistently.
@davidzlu davidzlu added refactor Code refactoring test Add or improve testing labels Nov 8, 2024
@davidzlu davidzlu self-assigned this Nov 8, 2024
@davidzlu davidzlu requested a review from hiyaryan November 8, 2024 22:26
@davidzlu davidzlu closed this Nov 12, 2024
@davidzlu
Copy link
Collaborator Author

Will be rebasing this branch onto 139-ts-migration after #187 and #185 merged into it

@davidzlu davidzlu deleted the 139-delete-entry-services branch November 12, 2024 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor Code refactoring test Add or improve testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant