feat(SPV-1538): migrate contact endpoints to v2 API #931

pawellewandowski98 · 2025-02-26T08:25:31Z

Description:

Migrate contact endpoints to v2 version (user and admin)
Create new UserContacts table
Create v2 contacts service and repository
Cover all endpoints in tests

Pull Request Checklist

📖 I created my PR using provided : CODE_STANDARDS
📖 I have read the short Code of Conduct: CODE_OF_CONDUCT
🏠 I tested my changes locally.
✅ I have provided tests for my changes.
📝 I have used conventional commits.
📗 I have updated any related documentation.
💾 PR was issued based on the Github or Jira issue.

github-actions · 2025-02-26T08:25:45Z

Manual Tests

ℹ️ Remember to ask team members to perform manual tests and to assign tested label after testing.

…-1538-migrate-contacts # Conflicts: # api/components/requests.yaml # api/gen.api.yaml # api/gen.models.go

…-1538-migrate-contacts

codecov-commenter · 2025-03-05T09:58:47Z

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 9.10152% with 1558 lines in your changes missing coverage. Please review.

Project coverage is 32.68%. Comparing base (3ad6ae4) to head (7cc7c62).

Files with missing lines	Patch %	Lines
api/gen.models.go	0.00%	674 Missing ⚠️
engine/v2/contacts/contacts_service.go	0.00%	294 Missing ⚠️
api/gen.api.go	0.00%	263 Missing ⚠️
engine/v2/database/repository/contacts.go	0.00%	143 Missing ⚠️
engine/testabilities/fixture_contacts.go	0.00%	52 Missing ⚠️
actions/v2/internal/mapping/contact.go	0.00%	23 Missing ⚠️
actions/v2/contacts/upsert.go	64.28%	11 Missing and 4 partials ⚠️
...ngine/v2/paymailserver/paymail_service_provider.go	0.00%	12 Missing ⚠️
engine/tester/jsonrequire/getter.go	0.00%	10 Missing ⚠️
engine/testabilities/fixture_engine.go	0.00%	9 Missing ⚠️
... and 17 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #931      +/-   ##
==========================================
- Coverage   34.50%   32.68%   -1.83%     
==========================================
  Files         437      457      +20     
  Lines       20826    22469    +1643     
==========================================
+ Hits         7186     7343     +157     
- Misses      13041    14511    +1470     
- Partials      599      615      +16

Flag	Coverage Δ
unittests	`32.68% <9.10%> (-1.83%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...ions/testabilities/assert_spvwallet_application.go	`0.00% <ø> (ø)`
actions/v2/admin/contacts/accept.go	`100.00% <100.00%> (ø)`
actions/v2/admin/contacts/reject.go	`100.00% <100.00%> (ø)`
actions/v2/admin/contacts/server.go	`100.00% <100.00%> (ø)`
actions/v2/admin/contacts/update.go	`100.00% <100.00%> (ø)`
actions/v2/admin/server.go	`100.00% <100.00%> (ø)`
actions/v2/contacts/server.go	`100.00% <100.00%> (ø)`
engine/v2/database/user.go	`0.00% <ø> (ø)`
actions/v2/api.go	`0.00% <0.00%> (ø)`
engine/v2/database/dbquery/paginate.go	`0.00% <0.00%> (ø)`
... and 25 more

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3ad6ae4...7cc7c62. Read the comment docs.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

chris-4chain · 2025-03-05T11:42:15Z

actions/v2/admin/contacts/accept_test.go

+
+		// when:
+		res, _ := client.R().
+			Post("/api/v2/admin/invitations/" + strconv.Itoa(int(contact.ID)))


Suggested change

Post("/api/v2/admin/invitations/" + strconv.Itoa(int(contact.ID)))

Post(fmt.Sprintf("/api/v2/admin/invitations/%d",contact.ID))

Also, applicable to other, similar places.

chris-4chain · 2025-03-05T11:44:29Z

engine/testabilities/fixture_engine.go

 }

+// ContactsFixture is a test fixture for the contacts service
+type ContactsFixture interface {


I would rename it to UserFixture because, in the future, it might be extended by other, not-contact-related features.

chris-4chain · 2025-03-05T11:49:29Z

engine/testabilities/fixture_contacts.go

+	return contact
+}
+
+func (f *contactFixture) removeContactIfExists(userB fixtures.User) {


I would try to avoid situations when fixtures need to remove/clear some data (most probably after previous test case).

Maybe you should separate test cases and avoid givenForAllTests and create "fresh" given for each case (given, when := testabilities.New())

Done, removed deleting contact and created fresh tests where it was required

chris-4chain · 2025-03-05T11:53:14Z

actions/v2/admin/contacts/accept_test.go

+				"status":   contactsmodels.ContactNotConfirmed,
+			})
+
+		// and:


Remove unnecessary and

chris-4chain · 2025-03-05T11:55:06Z

actions/v2/admin/contacts/accept.go

+
+// AdminAcceptInvitation accepts an invitation from a contact.
+func (s *APIAdminContacts) AdminAcceptInvitation(c *gin.Context, id uint) {
+	contact, err := s.engine.ContactService().AcceptContactByID(c.Request.Context(), id)


Suggested change

contact, err := s.engine.ContactService().AcceptContactByID(c.Request.Context(), id)

contact, err := s.engine.ContactService().AcceptContactByID(c, id)

And for other, similar places.

chris-4chain · 2025-03-05T11:59:59Z

actions/v2/admin/contacts/confirm_test.go

+		then.Response(res).IsOK()
+
+		// and:
+		// when:


Remove unnecessary // and:

chris-4chain · 2025-03-05T12:10:00Z

actions/v2/admin/contacts/confirm_test.go

+			}`, map[string]any{
+				"code":    spverrors.ErrNotAnAdminKey.Code,
+				"message": spverrors.ErrNotAnAdminKey.Message,
+			})


See how it's done in other places: https://github.com/bitcoin-sv/spv-wallet/blob/f2f26fdcc181c4de4cdab3ef14e0712302e6430a/actions/v2/admin/users/paymail_add_test.go#L225-L227

chris-4chain · 2025-03-05T12:14:33Z

actions/v2/admin/contacts/search.go

+)
+
+// AdminGetContacts returns a list of contacts for the admin.
+func (s *APIAdminContacts) AdminGetContacts(c *gin.Context, params api.AdminGetContactsParams) {


@mgosek-4chain is doing a task for filters and pagination.
I would postpone this after he finishes.

For now, I would suggest to remove all code/endpoints related to this and create a task for the future.

Done, removed endpoints but created a copy of the branch (feat/add-contacts-search-v2) to use them when new changes will be merged

chris-4chain · 2025-03-05T12:18:49Z

engine/v2/database/repository/mapping.go

+
+import "gorm.io/gorm"
+
+func mapConditionsToScopes(conditions map[string]interface{}) []func(tx *gorm.DB) *gorm.DB {


I think this should be handled differently. And, as I said above - it is already been researched by @mgosek-4chain.

I would remove this function for now.

…-1538-migrate-contacts

pawellewandowski98 added 5 commits February 21, 2025 11:42

feat(SPV-1538): implement upsert contact

51e690f

feat: add mapping for contact

ceb7d4b

feat(SPV-1538): add new contact endpoints

bc64687

feat: add admin contact endpoints

e58bfab

feat: add accept and reject admin endpoints

e8ded44

pawellewandowski98 self-assigned this Feb 26, 2025

pawellewandowski98 added 8 commits February 26, 2025 11:03

feat: update errors

2e2850e

fix: fix api methods and regenerate

604d4c2

Merge remote-tracking branch 'refs/remotes/origin/main' into feat/SPV…

aaf313f

…-1538-migrate-contacts # Conflicts: # api/components/requests.yaml # api/gen.api.yaml # api/gen.models.go

chore: generate after merge

ed05818

test: add tests for admin endpoints

8b056d2

tests: add eew tests for contacts

0c72c52

tests: add contact test for the rest ofthe endpoints

ce2f845

Merge remote-tracking branch 'refs/remotes/origin/main' into feat/SPV…

d6bb02a

…-1538-migrate-contacts

pawellewandowski98 marked this pull request as ready for review March 5, 2025 09:55

pawellewandowski98 requested a review from a team as a code owner March 5, 2025 09:55

pawellewandowski98 added 4 commits March 5, 2025 10:59

fix: yaml linter

c1a04a4

fix: linter errors

d6a44ae

fix: linter errors

f1d5f08

fix: linter error

c077248

chris-4chain suggested changes Mar 5, 2025

View reviewed changes

pawellewandowski98 added 7 commits March 6, 2025 10:54

tests: refactor the way of creating endpoint url

22428a8

test: rename ContactFixture

c1186bc

tests: create new tests when needed

07d1058

chore: use gin context

67deaa0

test: refactor the way of checking errors

e085f6f

chore: remove search endpoints

7da9cd8

Merge remote-tracking branch 'refs/remotes/origin/main' into feat/SPV…

ae36b57

…-1538-migrate-contacts

pawellewandowski98 and others added 6 commits March 7, 2025 09:42

fix: fix creating api v2 after merge

aada719

fix: linter errors

8e18e4d

feat: use contacts service interface instead of engine

12b5e39

tests: fix tests

df89efc

tests: fix tests

c74a6c7

Merge branch 'main' into feat/SPV-1538-migrate-contacts

7cc7c62

	Post("/api/v2/admin/invitations/" + strconv.Itoa(int(contact.ID)))
	Post(fmt.Sprintf("/api/v2/admin/invitations/%d",contact.ID))

	contact, err := s.engine.ContactService().AcceptContactByID(c.Request.Context(), id)
	contact, err := s.engine.ContactService().AcceptContactByID(c, id)


		import "gorm.io/gorm"

		func mapConditionsToScopes(conditions map[string]interface{}) []func(tx gorm.DB) gorm.DB {

feat(SPV-1538): migrate contact endpoints to v2 API #931

Are you sure you want to change the base?

feat(SPV-1538): migrate contact endpoints to v2 API #931

Uh oh!

Conversation

pawellewandowski98 commented Feb 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description:

Pull Request Checklist

Uh oh!

github-actions bot commented Feb 26, 2025

Manual Tests

Uh oh!

codecov-commenter commented Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

chris-4chain Mar 5, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pawellewandowski98 commented Feb 26, 2025 •

edited

Loading

codecov-commenter commented Mar 5, 2025 •

edited

Loading

chris-4chain Mar 5, 2025 •

edited

Loading