import Time in function Import from CSV in case it is available #4555
Conversation
|
Normal time format is |
|
Hi @pfalcon, yes you are right. In my usecase if I export the depot, then PP generates this:
This is now possible to read back with PP. |
pyBlockDev
force-pushed
the
csvimport_time
branch
from
ad8324c to
877d093
Compare
|
Hello @pyBlockDev Regards |
Nirus2000
added
the
! Good first pull request !
|
Hi @Nirus2000, thanks for your hint. I have added the testcases. Additional I have rechanged the code, used the existing structure and tried to simplify it. |
| public void testTransferTransactionDateContainsTimeAndTimeColumn() | ||
| { | ||
| Client client = new Client(); | ||
| Security security = new Security(); | ||
| security.setTickerSymbol("SAP.DE"); | ||
| client.addSecurity(security); | ||
|
|
||
| CSVExtractor extractor = new CSVPortfolioTransactionExtractor(client); | ||
|
|
||
| List<Exception> errors = new ArrayList<>(); | ||
| List<Item> results = extractor.extract(0, | ||
| Arrays.<String[]>asList(new String[] { "2013-01-01T14:05", "10:07", "DE0007164600", "SAP", "", | ||
| "SAP SE", "100", "EUR", "11", "10", "", "", "", "1,2", "TRANSFER_IN", | ||
| "Notiz" }), | ||
| buildField2Column(extractor), errors); | ||
|
|
||
| assertThat(errors, empty()); | ||
| assertThat(results.size(), is(1)); | ||
| new AssertImportActions().check(results, CurrencyUnit.EUR); | ||
|
|
||
| PortfolioTransferEntry entry = (PortfolioTransferEntry) results.stream() | ||
| .filter(PortfolioTransferItem.class::isInstance).findAny() | ||
| .orElseThrow(IllegalArgumentException::new).getSubject(); | ||
|
|
||
| PortfolioTransaction source = entry.getSourceTransaction(); | ||
| assertThat(source.getType(), is(PortfolioTransaction.Type.TRANSFER_OUT)); | ||
| assertThat(source.getMonetaryAmount(), is(Money.of(CurrencyUnit.EUR, 100_00))); | ||
| assertThat(source.getNote(), is("Notiz")); | ||
| assertThat(source.getDateTime(), is(LocalDateTime.parse("2013-01-01T10:07"))); | ||
| assertThat(source.getShares(), is(Values.Share.factorize(1.2))); | ||
| assertThat(source.getSecurity(), is(security)); | ||
| // security transfers do not support fees and taxes at the moment | ||
| assertThat(source.getUnitSum(Unit.Type.FEE), is(Money.of("EUR", 0))); | ||
| assertThat(source.getUnitSum(Unit.Type.TAX), is(Money.of("EUR", 0))); | ||
|
|
||
| PortfolioTransaction target = entry.getTargetTransaction(); | ||
| assertThat(target.getType(), is(PortfolioTransaction.Type.TRANSFER_IN)); | ||
| assertThat(target.getUnitSum(Unit.Type.FEE), is(Money.of("EUR", 0))); | ||
| assertThat(target.getUnitSum(Unit.Type.TAX), is(Money.of("EUR", 0))); | ||
| } |
There was a problem hiding this comment.
Arrays.<String[]>asList(new String[] { "2013-01-01T14:05", "10:07", "DE0007164600", "SAP", "",
"SAP SE", "100", "EUR", "11", "10", "", "", "", "1,2", "TRANSFER_IN",
"Notiz" }),
Test result
assertThat(source.getDateTime(), is(LocalDateTime.parse("2013-01-01T10:07")));
assertThat(target.getDateTime(), is(LocalDateTime.parse("2013-01-01T10:07")));
you set the time to "T14:05" but in the result is "10:07"
There was a problem hiding this comment.
Yes that is right, the reason is the Time column has a higher priority then the (Date+T) column in the PR implementaion. I thought about the possibility that somebody
- defines a Time in the date column with the "THH:MM" notation and
- also in the Time column.
In my opinion this is an error in the csv file, but I thought maybe it is better to handle this with a priority of the Time column and the current beahvior of the PP. Because at the moment in PP if the Time column is defined then this is used.
The PR has the following new behavior:
- if the Date column contains the "THH:mm" text and no Time column is defined, then the time is used from the Date
- if the Date column contains the "THH:mm" text and Time column is defined, then the time is used from the Time.
Closes #4554