feat: add fees to admin UI #2050

JoblersTune · 2023-10-18T12:50:18Z

Changes proposed in this pull request

added a query to fetch all historic fees associated with a particular asset
added current fee to asset overview on admin UI
added the ability to set a new fee on asset overview
displays a table of all historic fees for a particular asset (this view can be filtered by fee type)

Context

Checklist

netlify · 2023-10-18T12:50:23Z

✅ Deploy Preview for brilliant-pasca-3e80ec canceled.

Name	Link
🔨 Latest commit	`fd040cb`
🔍 Latest deploy log	https://app.netlify.com/sites/brilliant-pasca-3e80ec/deploys/65362703b70d03000778ab18

sabineschaller

Rafiki currently does not handle receiving fees (even though we have that fee type included). Can we just gray out that entire section? Or should we remove it for now?

sabineschaller · 2023-10-19T10:59:35Z

@mkurapov @BlairCurrey how should we deal with receiving fees?

JoblersTune · 2023-10-19T12:22:47Z

I can easily remove all the receiving fees code for now. And we can go with this

sabineschaller · 2023-10-19T13:01:44Z

I can easily remove all the receiving fees code for now. And we can go with this

I think that looks good.

sabineschaller · 2023-10-19T13:36:48Z

packages/frontend/app/routes/assets.$assetId.tsx

+                    error={response?.errors.sendingFee.fieldErrors.basisPoints}
+                  />
+                  <p className='text-gray-500 text-sm mt-2'>
+                    A single basis point is equal to 0.01% of the total fee.


I'm not sure that makes it clear. Should we say "A basis point equals 0.01% or 0.0001"? Or maybe add an example? "A fee of 1 basis point on $100 equals 0.0001*$100 = $0.01." Or both?

maybe a stretch, but it could also be nice to make it response to user input. For example they type in 50 and see "50 basis points is equal to 0.50% of the total fee" (or the worked example Sabine is suggesting).

How about "A single basis point is a fee equal to 0.01% of the total amount. A fee of 1 basis point on $100 is $0.01."

That works. Maybe we should create an issue to make this line adapt like @BlairCurrey suggested. That could even be a last minute hacktoberfest contribution.

packages/frontend/app/routes/assets.view-fees.$assetId.tsx

packages/frontend/app/routes/assets.$assetId.tsx

BlairCurrey · 2023-10-19T19:47:52Z

@mkurapov @BlairCurrey how should we deal with receiving fees?

Last I knew there was a chance that Mojaloop payments might need them. Any more insight on that? If they wont need them then I'd be in favor of removing the type and implicitly treating all fees as sending. The type was added with the expectation that receiving fees would be implemented but they were scrapped.

sabineschaller · 2023-10-20T06:57:45Z

Last I knew there was a chance that Mojaloop payments might need them. Any more insight on that? If they wont need them then I'd be in favor of removing the type and implicitly treating all fees as sending. The type was added with the expectation that receiving fees would be implemented but they were scrapped.

I think this is still the case, but not for the PoC we are working on right now with PCH but for the general MJL peer-to-peer payment method.

raducristianpopa · 2023-10-20T10:39:18Z

packages/backend/src/graphql/resolvers/asset.ts

@@ -160,6 +160,31 @@ export const getAssetReceivingFee: AssetResolvers<ApolloContext>['receivingFee']
    return feeToGraphql(fee)
  }

+export const getFees: AssetResolvers<ApolloContext>['fees'] = async (


Can we sort the fees by createdAt in a descending order? This way we will have latest fee at the top when showing the fees table.

I also thought about this. But wasn't sure it was worth writing a new getPage function to do that. But I can.

I think it makes sense to also display the payments and webhooks in descending order tbh. Can we pass a sorting option to the getPage function?

Will do. Moving this out into a separate issue - Add a sorting option to the getPage function #2088.

…view

…ees are all in a single card

…ling

Co-authored-by: Radu-Cristian Popa <[email protected]>

…view asset fees.

sabineschaller

LGTM

JoblersTune self-assigned this Oct 18, 2023

github-actions bot added type: tests Testing related pkg: backend Changes in the backend package. pkg: frontend Changes in the frontend package. type: source Changes business logic pkg: mock-ase labels Oct 18, 2023

JoblersTune modified the milestone: v1.0.0-alpha.4 Oct 18, 2023

sabineschaller changed the title ~~Sj/1834/add fees to admin UI~~ feat: add fees to admin UI Oct 19, 2023

sabineschaller requested a review from raducristianpopa October 19, 2023 08:50

sabineschaller reviewed Oct 19, 2023

View reviewed changes

JoblersTune force-pushed the sj/1834/add-fees-to-admin-ui branch from 1cbdddd to 50a48de Compare October 19, 2023 13:18

sabineschaller reviewed Oct 19, 2023

View reviewed changes

raducristianpopa reviewed Oct 19, 2023

View reviewed changes

packages/frontend/app/routes/assets.view-fees.$assetId.tsx Outdated Show resolved Hide resolved

packages/frontend/app/routes/assets.$assetId.tsx Outdated Show resolved Hide resolved

packages/frontend/app/routes/assets.$assetId.tsx Outdated Show resolved Hide resolved

raducristianpopa reviewed Oct 20, 2023

View reviewed changes

JoblersTune added 9 commits October 23, 2023 09:54

feat: added fees query and tests to GraphQl

25a3212

Added last test for the fees query, Can get fees

d2d4734

feat: added current fees to the asset overview

6db6d5c

feat: allow setting new fee on asset overview

c54cada

chore: add some helper text for the basis points fee field

dec30ca

feat: first attempt at historic fees table

08f31ab

chore: updated view-fees to be a separate page

95792b3

chore: added the creation date to the sending and receiving fees over…

f0cc990

…view

fix: reordering the fees so that sending and receiving and historic f…

9772e9f

…ees are all in a single card

JoblersTune and others added 6 commits October 23, 2023 09:54

fix: prevent wrapping of fee creation date since it messes up the sty…

60362d4

…ling

chore: minor fixes

76e49f3

chore: remove receiving fees

7944f56

Apply suggestions from code review

cb02fba

Co-authored-by: Radu-Cristian Popa <[email protected]>

chore: replaced missing comma

cd56772

chore: updated basis point explanation and file name for the page to …

fd040cb

…view asset fees.

JoblersTune force-pushed the sj/1834/add-fees-to-admin-ui branch from 0eb8bb9 to fd040cb Compare October 23, 2023 07:55

sabineschaller approved these changes Oct 23, 2023

View reviewed changes

JoblersTune merged commit 111a504 into main Oct 23, 2023
21 checks passed

JoblersTune deleted the sj/1834/add-fees-to-admin-ui branch October 23, 2023 08:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add fees to admin UI #2050

feat: add fees to admin UI #2050

JoblersTune commented Oct 18, 2023 •

edited

Loading

netlify bot commented Oct 18, 2023 •

edited

Loading

sabineschaller left a comment

sabineschaller commented Oct 19, 2023

JoblersTune commented Oct 19, 2023

sabineschaller commented Oct 19, 2023

sabineschaller Oct 19, 2023

BlairCurrey Oct 19, 2023

JoblersTune Oct 20, 2023

sabineschaller Oct 23, 2023

BlairCurrey commented Oct 19, 2023 •

edited

Loading

sabineschaller commented Oct 20, 2023

raducristianpopa Oct 20, 2023

JoblersTune Oct 20, 2023

sabineschaller Oct 20, 2023

JoblersTune Oct 23, 2023 •

edited

Loading

sabineschaller left a comment

feat: add fees to admin UI #2050

feat: add fees to admin UI #2050

Conversation

JoblersTune commented Oct 18, 2023 • edited Loading

Changes proposed in this pull request

Context

Checklist

netlify bot commented Oct 18, 2023 • edited Loading

✅ Deploy Preview for brilliant-pasca-3e80ec canceled.

sabineschaller left a comment

Choose a reason for hiding this comment

sabineschaller commented Oct 19, 2023

JoblersTune commented Oct 19, 2023

sabineschaller commented Oct 19, 2023

sabineschaller Oct 19, 2023

Choose a reason for hiding this comment

BlairCurrey Oct 19, 2023

Choose a reason for hiding this comment

JoblersTune Oct 20, 2023

Choose a reason for hiding this comment

sabineschaller Oct 23, 2023

Choose a reason for hiding this comment

BlairCurrey commented Oct 19, 2023 • edited Loading

sabineschaller commented Oct 20, 2023

raducristianpopa Oct 20, 2023

Choose a reason for hiding this comment

JoblersTune Oct 20, 2023

Choose a reason for hiding this comment

sabineschaller Oct 20, 2023

Choose a reason for hiding this comment

JoblersTune Oct 23, 2023 • edited Loading

Choose a reason for hiding this comment

sabineschaller left a comment

Choose a reason for hiding this comment

JoblersTune commented Oct 18, 2023 •

edited

Loading

netlify bot commented Oct 18, 2023 •

edited

Loading

BlairCurrey commented Oct 19, 2023 •

edited

Loading

JoblersTune Oct 23, 2023 •

edited

Loading