Skip to content

Properly tracks fees as entered#58

Merged
contre95 merged 1 commit into
contre95:mainfrom
ImDevinC:fix/fees
Aug 24, 2025
Merged

Properly tracks fees as entered#58
contre95 merged 1 commit into
contre95:mainfrom
ImDevinC:fix/fees

Conversation

@ImDevinC

Copy link
Copy Markdown
Contributor

There's a few issues happening here that this resolves to properly show
fees.

  • For starters, the form values were named differently than what the
    backend was expecting. To make things match as closely possible, I
    renamed the form inputs to bfee (BaseFee) and qfee (QuoteFee)
    which is what the backend is expecting.
  • The database had no fields in the Trade table for either fee. Added
    those. Note: This will require a change to the database, and I don't
    currently see a migration pattern. Is is there one in mind?
  • Also updated the appropriate queries to return the new fee values from
    the database.

Resolves #53

There's a few issues happening here that this resolves to properly show
fees.

- For starters, the form values were named differently  than what the
  backend was expecting. To make things match as closely possible, I
  renamed the form inputs to `bfee` (BaseFee) and `qfee` (QuoteFee)
  which is what the backend is expecting.
- The database had no fields in the `Trade` table for either fee. Added
  those. **Note: This will require a change to the database, and I don't
  currently see a migration pattern. Is is there one in mind?**
- Also updated the appropriate queries to return the new fee values from
  the database.

Resolves #53
return nil, fmt.Errorf("invalid trade type: %s", req.Type)
}
fmt.Println("RecordTradeReq", req)
fmt.Printf("RecordTradeReq: %+v\n", req)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Changed to printf so it prints out the full key/value naming instead of a struct of values

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good, thanks !

@contre95 contre95 merged commit a029768 into contre95:main Aug 24, 2025
1 check passed
@contre95

Copy link
Copy Markdown
Owner

@ImDevinC thanks for the PR.

There's currently no migration plan for the Database. I wouldn't know how to do this without an engine for sqlite.

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.

Fees paid is always 0

2 participants