-
Notifications
You must be signed in to change notification settings - Fork 22
export csv for coin ledger #245
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
Conversation
pages/admin/pro/history.js
Outdated
<br /> | ||
<br /> | ||
Let us know if we miss koinlyIDs for your tokens. We will add them to the system. | ||
Let us know if we miss koinlyIDs for your tokens. We will add them to the system. */} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you comment it? the text should be visible when koinly is chosen
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Got it I'll updated this
pages/admin/pro/history.js
Outdated
@@ -252,10 +310,10 @@ export default function History({ queryAddress, selectedCurrency, setSelectedCur | |||
|
|||
res.activities[i].timestampExport = new Date(res.activities[i].timestamp * 1000).toISOString() | |||
|
|||
res.activities[i].sentAmount = sending ? res.activities[i].amountNumber : 0 | |||
res.activities[i].sentAmount = sending ? res.activities[i].amountNumber : "" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is a dangerous chnage that can brake csv for koinly.. was that tested?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Export CSV file for CoindLedger platform => SOLVED @ihomp could you check this? export-csv-coinledger.mp4 |
pages/admin/pro/history.js
Outdated
res.activities[i].receivedCurrency = !sending ? scvCurrency : '' | ||
|
||
res.activities[i].netWorthCurrency = selectedCurrency.toUpperCase() | ||
|
||
//sanitize memos for CSV | ||
res.activities[i].memo = res.activities[i].memo?.replace(/"/g, "'") || '' | ||
|
||
// For CoinLedger platform | ||
res.activities[i].coinLedgerTxType = res.activities[i].direction === 'sent' ? 'Withdrawal' : res.activities[i].direction === 'received' ? 'Deposit' : res.activities[i].txType |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we should not rely on "direction", it should rely on the "amount".
- if positive amount -> Deposit
- if negative balance -> Withdrawl
Are there any other options available?
Because if only fee was paid it's not widtrawal. it's network fee, it should be an option for that.
then if the negative balance is less or equal the fee - then it's "Fee" or equvalent
the "direction" should not be used for the balance changes types |
pages/admin/pro/history.js
Outdated
@@ -29,7 +29,9 @@ import Image from 'next/image' | |||
import { CSVLink } from 'react-csv' | |||
import DownloadIcon from '../../../public/images/download.svg' | |||
import { koinly } from '../../../utils/koinly' | |||
|
|||
import { format } from 'date-fns' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do we need 'date-fns' in here?
JS functions not enough to format a date?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The project doesn't have date-fns installed, and I don't see the package is added in the package.json and yarn.lock
it's nice sometimes to avoid installing new packages, and just replace them with a function.
pages/admin/pro/history.js
Outdated
CoinLedger: (timestamp) => { | ||
// Format: MM/DD/YYYY HH:MM:SS in UTC | ||
const date = new Date(timestamp * 1000); | ||
return format(new Date(date.getTime() + date.getTimezoneOffset() * 60000), 'MM/dd/yyyy HH:mm:ss'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would this work from unix seconds to format they need in utc?
function formatUnixToUTC(unixSeconds) {
const date = new Date(unixSeconds * 1000); // Convert to milliseconds
const pad = (n) => n.toString().padStart(2, '0');
const mm = pad(date.getUTCMonth() + 1);
const dd = pad(date.getUTCDate());
const yyyy = date.getUTCFullYear();
const hh = pad(date.getUTCHours());
const min = pad(date.getUTCMinutes());
const ss = pad(date.getUTCSeconds());
return `${mm}/${dd}/${yyyy} ${hh}:${min}:${ss}`;
}
pages/admin/pro/history.js
Outdated
@@ -222,7 +280,7 @@ export default function History({ queryAddress, selectedCurrency, setSelectedCur | |||
let sending = res.activities[i].amountInFiats?.[selectedCurrency]?.[0] === '-' | |||
res.activities[i].index = options?.marker ? activities.length + 1 + i : i + 1 | |||
res.activities[i].amountExport = amountFormat(res.activities[i].amount) | |||
res.activities[i].amountNumber = res.activities[i].amount?.value || res.activities[i].amount / 1000000 | |||
res.activities[i].amountNumber = res.activities[i].amount?.value || (parseFloat(res.activities[i].amount) / 1000000) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
didn't it work without parseFloat? it is string, but it's always integer
@ihomp Are there any other options available? then if the negative balance is less or equal the fee - then it's "Fee" or equvalent => CoinLedger also support Fee Tx type |
So, why did you remove the "Fee" option in your last commit? |
pages/admin/pro/history.js
Outdated
data={processDataForExport(filteredActivities || [], platformCSVExport)} | ||
headers={ | ||
platformCSVHeaders.find( | ||
(header) => header.platform.toLowerCase() === platformCSVExport.toLocaleLowerCase() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toLocaleLowerCase?
did you mean the toLowerCase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll update this right now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Anna15170221 the code looks good for me now.
If CSV imports works well for both platforms, we can merge it and give to some users to test.
No description provided.