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

chore(core): refactor merchant data structure #3906

Merged
merged 2 commits into from
Feb 2, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 0 additions & 5 deletions bats/admin-gql/account-update-level.gql
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,12 @@ mutation accountUpdateLevel($input: AccountUpdateLevelInput!) {
username
level
status
title
owner {
id
language
phone
createdAt
}
coordinates {
latitude
longitude
}
wallets {
id
walletCurrency
Expand Down
5 changes: 0 additions & 5 deletions bats/admin-gql/account-update-status.gql
Original file line number Diff line number Diff line change
Expand Up @@ -8,17 +8,12 @@ mutation accountUpdateStatus($input: AccountUpdateStatusInput!) {
username
level
status
title
owner {
id
language
phone
createdAt
}
coordinates {
latitude
longitude
}
wallets {
id
walletCurrency
Expand Down
7 changes: 7 additions & 0 deletions bats/admin-gql/business-delete-map-info.gql
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
mutation businessDeleteMapInfo($input: BusinessDeleteMapInfoInput!) {
businessDeleteMapInfo(input: $input) {
errors {
message
}
}
}
18 changes: 18 additions & 0 deletions bats/admin-gql/business-update-map-info.gql
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
mutation businessUpdateMapInfo($input: BusinessUpdateMapInfoInput!) {
businessUpdateMapInfo(input: $input) {
errors {
message
}
merchant {
id
title
coordinates {
latitude
longitude
}
username
validated
createdAt
}
}
}
36 changes: 0 additions & 36 deletions bats/admin-gql/update-merchant-map.gql

This file was deleted.

24 changes: 21 additions & 3 deletions bats/core/api/merchant.bats
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ setup_file() {
login_admin
}

@test "merchants: add a merchant with admin api" {
@test "merchant: add a merchant with admin api" {
admin_token="$(read_value 'admin.token')"
latitude=40.712776
longitude=-74.005974
Expand All @@ -28,8 +28,8 @@ setup_file() {
'{input: {latitude: ($latitude | tonumber), longitude: ($longitude | tonumber), title: $title, username: $username}}'
)

exec_admin_graphql $admin_token 'update-merchant-map' "$variables"
latitude_result="$(graphql_output '.data.businessUpdateMapInfo.accountDetails.coordinates.latitude')"
exec_admin_graphql $admin_token 'business-update-map-info' "$variables"
latitude_result="$(graphql_output '.data.businessUpdateMapInfo.merchant.coordinates.latitude')"
[[ "$latitude_result" == "$latitude" ]] || exit 1
}

Expand All @@ -39,3 +39,21 @@ setup_file() {
fetch_username="$(graphql_output '.data.businessMapMarkers[0].username')"
[[ $username = $fetch_username ]] || exit 1
}

@test "merchant: delete merchant with admin api" {
admin_token="$(read_value 'admin.token')"
local username="$(read_value merchant.username)"

variables=$(jq -n \
--arg username "$username" \
'{input: {username: $username}}'
)

exec_admin_graphql $admin_token 'business-delete-map-info' "$variables"

exec_graphql 'anon' 'business-map-markers'
map_markers="$(graphql_output)"
markers_length=$(echo "$map_markers" | jq '.data.businessMapMarkers | length')

[[ $markers_length -eq 0 ]] || exit 1
}
23 changes: 0 additions & 23 deletions core/api/src/app/accounts/delete-business-map-info.ts

This file was deleted.

6 changes: 0 additions & 6 deletions core/api/src/app/accounts/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,12 +13,10 @@ export * from "./set-username"
export * from "./update-account-ip"
export * from "./update-account-level"
export * from "./update-account-status"
export * from "./update-business-map-info"
export * from "./update-contact-alias"
export * from "./update-default-walletid"
export * from "./update-display-currency"
export * from "./username-available"
export * from "./delete-business-map-info"
export * from "./upgrade-device-account"
export * from "./disable-notification-category"
export * from "./enable-notification-category"
Expand Down Expand Up @@ -51,7 +49,3 @@ export const hasPermissions = async (

return accountId === wallet.accountId
}

export const getBusinessMapMarkers = async () => {
return accounts.listBusinessesForMap()
}
8 changes: 6 additions & 2 deletions core/api/src/app/accounts/mark-account-for-deletion.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { deleteMerchantByUsername } from "../merchants"
nicolasburtey marked this conversation as resolved.
Show resolved Hide resolved

import { getBalanceForWallet, listWalletsByAccountId } from "@/app/wallets"

import { AccountStatus, AccountValidator } from "@/domain/accounts"
Expand Down Expand Up @@ -62,8 +64,10 @@ export const markAccountForDeletion = async ({
status: AccountStatus.Closed,
updatedByPrivilegedClientId,
})
account.title = null
account.coordinates = null

if (account.username) {
await deleteMerchantByUsername({ username: account.username })
}

const newAccount = await accountsRepo.update(account)
if (newAccount instanceof Error) return newAccount
Expand Down
37 changes: 0 additions & 37 deletions core/api/src/app/accounts/update-business-map-info.ts

This file was deleted.

3 changes: 3 additions & 0 deletions core/api/src/app/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import * as TransactionsMod from "./transactions"
import * as UsersMod from "./users"
import * as WalletsMod from "./wallets"
import * as PaymentsMod from "./payments"
import * as MerchantsMod from "./merchants"

import { wrapAsyncToRunInSpan } from "@/services/tracing"

Expand All @@ -21,6 +22,7 @@ const allFunctions = {
Callback: { ...CallbackMod },
Comm: { ...CommMod },
Quiz: { ...QuizMod },
Merchants: { ...MerchantsMod },
Lightning: { ...LightningMod },
OnChain: { ...OnChainMod },
Prices: { ...PricesMod },
Expand Down Expand Up @@ -50,6 +52,7 @@ export const {
Callback,
Comm,
Quiz,
Merchants,
Lightning,
OnChain,
Prices,
Expand Down
38 changes: 38 additions & 0 deletions core/api/src/app/merchants/delete-business-map-info.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import { checkedToUsername } from "@/domain/accounts"
import { MerchantsRepository } from "@/services/mongoose"

export const deleteBusinessMapInfo = async ({
nicolasburtey marked this conversation as resolved.
Show resolved Hide resolved
id,
}: {
id: MerchantId
}): Promise<void | ApplicationError> => {
Copy link
Contributor

Choose a reason for hiding this comment

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

In these sorts of cases we usually return true instead of void so that we don't miss any accidental early returns

Suggested change
}): Promise<void | ApplicationError> => {
}): Promise<true | ApplicationError> => {

Copy link
Member Author

Choose a reason for hiding this comment

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

that's a good point

Copy link
Member Author

@nicolasburtey nicolasburtey Feb 2, 2024

Choose a reason for hiding this comment

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

seems there are other place in the codebase that have Promise<void

const merchantsRepo = MerchantsRepository()

const result = await merchantsRepo.remove(id)
if (result instanceof Error) return result

return
}

export const deleteMerchantByUsername = async ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Small nit, consistent naming

Suggested change
export const deleteMerchantByUsername = async ({
export const deleteMerchantLocationsByUsername = async ({

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd prefer to keep it deleteMerchantByUsername for now because there is no dis-association as of now between a merchant and a location

username,
}: {
username: string
}): Promise<void | ApplicationError> => {
nicolasburtey marked this conversation as resolved.
Show resolved Hide resolved
const merchantsRepo = MerchantsRepository()

const usernameChecked = checkedToUsername(username)
if (usernameChecked instanceof Error) return usernameChecked

const merchants = await merchantsRepo.findByUsername(usernameChecked)
if (merchants instanceof Error) return merchants

if (merchants.length === 0) return

for (const merchant of merchants) {
const result = await merchantsRepo.remove(merchant.id)
if (result instanceof Error) return result
}

return
}
10 changes: 10 additions & 0 deletions core/api/src/app/merchants/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { MerchantsRepository } from "@/services/mongoose"

export * from "./update-business-map-info"
export * from "./delete-business-map-info"

const merchants = MerchantsRepository()

export const getMerchantsMapMarkers = async () => {
return merchants.listForMap()
}
45 changes: 45 additions & 0 deletions core/api/src/app/merchants/update-business-map-info.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
import { checkedCoordinates, checkedMapTitle, checkedToUsername } from "@/domain/accounts"
import { CouldNotFindMerchantFromUsernameError } from "@/domain/errors"
import { MerchantsRepository } from "@/services/mongoose"

export const updateBusinessMapInfo = async ({
username,
coordinates: { latitude, longitude },
title,
}: {
username: string
coordinates: { latitude: number; longitude: number }
title: string
}): Promise<BusinessMapMarker | ApplicationError> => {
const merchantsRepo = MerchantsRepository()

const usernameChecked = checkedToUsername(username)
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also confirm that username exists for a user here with AccountsRepository().findByUsername(username) before continuing with the use-case (or is this validated somewhere else)?

if (usernameChecked instanceof Error) return usernameChecked

const coordinates = checkedCoordinates({ latitude, longitude })
if (coordinates instanceof Error) return coordinates

const titleChecked = checkedMapTitle(title)
if (titleChecked instanceof Error) return titleChecked

const merchants = await merchantsRepo.findByUsername(usernameChecked)

if (merchants instanceof CouldNotFindMerchantFromUsernameError) {
return merchantsRepo.create({
username: usernameChecked,
coordinates,
title: titleChecked,
validated: true,
})
} else if (merchants instanceof Error) {
return merchants
}

// TODO: manage multiple merchants for a single username
Copy link
Member Author

Choose a reason for hiding this comment

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

will be done on follow up PRs, right now it's mostly a refactor

const merchant = merchants[0]

merchant.coordinates = coordinates
merchant.title = titleChecked

return merchantsRepo.update(merchant)
}
Loading
Loading