Skip to content

create export csv for cryptotax #263

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

juddydev
Copy link
Contributor

  1. Example output:
    crypto-tax-csv.csv

  2. Test import
    image

@ihomp ihomp requested review from ihomp and Copilot April 30, 2025 13:06
Copy link
Member

@ihomp ihomp left a comment

Choose a reason for hiding this comment

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

We need a new PR agains the latest code base

Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds support for exporting CSV data for a new "CryptoTax" platform and adjusts the header keys and data mappings accordingly.

  • Added a new date formatter for the CryptoTax timestamp format.
  • Updated CSV header configurations to include CryptoTax specific columns and adjusted currency mappings.
  • Modified activity processing to handle CryptoTax fields alongside existing platforms.
Comments suppressed due to low confidence (2)

pages/admin/pro/history.js:363

  • [nitpick] The update now sets sentCurrency using 'currency' rather than 'scvCurrency', while the Koinly fields still use 'scvCurrency'. Confirm that this change is intentional to avoid inconsistency in currency mapping.
res.activities[i].sentCurrency = sending ? currency : ''

pages/admin/pro/history.js:366

  • [nitpick] Similar to sentCurrency, receivedCurrency now uses 'currency' instead of 'scvCurrency'. Verify that this change aligns with the intended usage for both display and Koinly CSV export.
res.activities[i].receivedCurrency = !sending ? currency : ''

Comment on lines 172 to 182
{ label: 'Quote Currency (Optional)', key: '' },
{ label: 'Quote Amount (Optional)', key: '' },
{ label: 'Fee Currency (Optional)', key: 'cryptoTaxFeeCurrencyCode' },
{ label: 'Fee Amount (Optional)', key: 'cryptoTaxFeeNumber' },
{ label: 'From (Optional)', key: '' },
{ label: 'To (Optional)', key: '' },
{ label: 'Blockchain (Optional)', key: '' },
{ label: 'ID (Optional)', key: 'hash' },
{ label: 'Description (Optional)', key: 'memo' },
{ label: 'Reference Price Per Unit (Optional)', key: '' },
{ label: 'Reference Price Currency (Optional)', key: '' }
Copy link
Preview

Copilot AI Apr 30, 2025

Choose a reason for hiding this comment

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

The header configuration includes fields with empty key values. Consider providing explicit key names or removing these entries if they are not meant to map to data properties.

Suggested change
{ label: 'Quote Currency (Optional)', key: '' },
{ label: 'Quote Amount (Optional)', key: '' },
{ label: 'Fee Currency (Optional)', key: 'cryptoTaxFeeCurrencyCode' },
{ label: 'Fee Amount (Optional)', key: 'cryptoTaxFeeNumber' },
{ label: 'From (Optional)', key: '' },
{ label: 'To (Optional)', key: '' },
{ label: 'Blockchain (Optional)', key: '' },
{ label: 'ID (Optional)', key: 'hash' },
{ label: 'Description (Optional)', key: 'memo' },
{ label: 'Reference Price Per Unit (Optional)', key: '' },
{ label: 'Reference Price Currency (Optional)', key: '' }
// { label: 'Quote Currency (Optional)', key: '' }, // Removed as it does not map to any data property
// { label: 'Quote Amount (Optional)', key: '' }, // Removed as it does not map to any data property
{ label: 'Fee Currency (Optional)', key: 'cryptoTaxFeeCurrencyCode' },
{ label: 'Fee Amount (Optional)', key: 'cryptoTaxFeeNumber' },
// { label: 'From (Optional)', key: '' }, // Removed as it does not map to any data property
// { label: 'To (Optional)', key: '' }, // Removed as it does not map to any data property
// { label: 'Blockchain (Optional)', key: '' }, // Removed as it does not map to any data property
{ label: 'ID (Optional)', key: 'hash' },
{ label: 'Description (Optional)', key: 'memo' },
// { label: 'Reference Price Per Unit (Optional)', key: '' }, // Removed as it does not map to any data property
// { label: 'Reference Price Currency (Optional)', key: '' } // Removed as it does not map to any data property

Copilot uses AI. Check for mistakes.

@juddydev juddydev requested a review from ihomp May 5, 2025 03:41
Copy link
Member

@ihomp ihomp left a comment

Choose a reason for hiding this comment

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

Requires fixes

@@ -116,6 +121,12 @@ const processDataForExport = (activities, platform) => {
: Math.abs(activity.amountNumber) <= activity.txFeeNumber
? 'Other Fee'
: 'Deposit'
} else if (platform === 'CryptoTax') {
processedActivity.cryptoTaxTxType = !sending
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't it be here processedActivity.type instead of processedActivity.cryptoTaxTxType?

because below you use type in headers
And that would also be good for consistency

platform: 'CryptoTax',
headers: [
{ label: 'Timestamp (UTC)', key: 'timestampExport' },
{ label: 'Type', key: 'type' },
Copy link
Member

Choose a reason for hiding this comment

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

it's type here

@@ -375,6 +406,23 @@ export default function History({ queryAddress, selectedCurrency, setSelectedCur

//sanitize memos for CSV
res.activities[i].memo = res.activities[i].memo?.replace(/"/g, "'") || ''

// For CryptoTax platform
Copy link
Member

Choose a reason for hiding this comment

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

As it's specific for a platform, we shouldn't do it for everyone.
The code needs to be moved into the processDataForExport function

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.

2 participants