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

Remove Account implementation and integration #1652

Merged
merged 4 commits into from
Jun 14, 2024
Merged

Conversation

iamacook
Copy link
Member

Summary

The current account implementation was built according to email notifications. As we are now putting more focus on accounts as a fundamental feature, we are reassessing the implementation.

This removes the current account/subscription integration, as well as their integration with alerts and emails.

Changes

  • Remove account datasource
    • Remove test datasource from tests
  • Remove account domain
  • Remove subscription domain
  • Decouple account/subscription integration from alerts:
    • Remove business logic
    • Remove redundant code
    • Remove relative test logic and skip those eventually required

@iamacook iamacook self-assigned this Jun 14, 2024
@iamacook iamacook requested a review from a team as a code owner June 14, 2024 09:33
@coveralls
Copy link

coveralls commented Jun 14, 2024

Pull Request Test Coverage Report for Build 9513954877

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 35 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.2%) to 92.304%

Files with Coverage Reduction New Missed Lines %
src/domain/alerts/alerts.repository.ts 35 24.66%
Totals Coverage Status
Change from base Build 9513933537: -0.2%
Covered Lines: 6822
Relevant Lines: 7103

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 14, 2024

Pull Request Test Coverage Report for Build 9514300114

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 50 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.3%) to 92.228%

Files with Coverage Reduction New Missed Lines %
src/domain/swaps/contracts/decoders/set-pre-signature-decoder.helper.ts 2 78.95%
src/datasources/balances-api/zerion-balances-api.service.ts 13 81.61%
src/domain/alerts/alerts.repository.ts 35 24.66%
Totals Coverage Status
Change from base Build 9513933537: -0.3%
Covered Lines: 6806
Relevant Lines: 7090

💛 - Coveralls

process.env.EMAIL_VERIFICATION_CODE_TTL_MS ?? `${5 * 60 * 1000}`,
),
},
// TODO: Decide whether we reuse these when implementing the email service
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: not an strong opinion, but I'd more in favour of deleting this code instead of commenting it. We can back to the commits history if we need to reuse this. Wdyt?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed in 7e7818d.

@hectorgomezv hectorgomezv self-requested a review June 14, 2024 14:26
@iamacook iamacook mentioned this pull request Jun 14, 2024
Adds a new datasource for accounts, implementing a basic database with tables for `accounts` and `groups`:

- Create `00001_accounts` migration to create database
- Create `Account` entity with test coverage
- Create `Group` entity with test coverage
- Create `IAccountsDatasource` with implementation and test override
@hectorgomezv hectorgomezv self-requested a review June 14, 2024 14:30
@iamacook iamacook enabled auto-merge (squash) June 14, 2024 14:31
@coveralls
Copy link

coveralls commented Jun 14, 2024

Pull Request Test Coverage Report for Build 9517736861

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 50 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.2%) to 92.28%

Files with Coverage Reduction New Missed Lines %
src/domain/swaps/contracts/decoders/set-pre-signature-decoder.helper.ts 2 78.95%
src/datasources/balances-api/zerion-balances-api.service.ts 13 81.61%
src/domain/alerts/alerts.repository.ts 35 24.66%
Totals Coverage Status
Change from base Build 9513933537: -0.2%
Covered Lines: 6808
Relevant Lines: 7090

💛 - Coveralls

@iamacook iamacook merged commit 5855b1a into main Jun 14, 2024
15 checks passed
@iamacook iamacook deleted the remove-accounts branch June 14, 2024 14:33
@coveralls
Copy link

coveralls commented Jun 14, 2024

Pull Request Test Coverage Report for Build 9517797503

Details

  • 32 of 45 (71.11%) changed or added relevant lines in 8 files are covered.
  • 50 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.3%) to 92.184%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/domain/interfaces/accounts.datasource.interface.ts 0 1 0.0%
src/datasources/accounts/accounts.datasource.ts 17 19 89.47%
src/datasources/accounts/tests/test.accounts.datasource.modulte.ts 0 5 0.0%
src/datasources/accounts/accounts.datasource.module.ts 0 5 0.0%
Files with Coverage Reduction New Missed Lines %
src/domain/swaps/contracts/decoders/set-pre-signature-decoder.helper.ts 2 78.95%
src/datasources/balances-api/zerion-balances-api.service.ts 13 81.61%
src/domain/alerts/alerts.repository.ts 35 24.66%
Totals Coverage Status
Change from base Build 9513933537: -0.3%
Covered Lines: 6841
Relevant Lines: 7135

💛 - Coveralls

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants