Skip to content
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

Transaction validation: date range #91

Merged
merged 1 commit into from
Sep 8, 2016

Conversation

Entea
Copy link
Contributor

@Entea Entea commented Sep 6, 2016

Implements #85

@coveralls
Copy link

Coverage Status

Coverage increased (+0.3%) to 68.288% when pulling 8863c2c on Entea:transaction_validation_date_range into 0f0ddf9 on hrj:master.

val format = DateTimeFormatter.ISO_LOCAL_DATE

val from = Try(LocalDate.parse(config.getString("from"), format)).map(Some(_)).getOrElse(None)
val to = Try(LocalDate.parse(config.getString("to"), format)).map(Some(_)).getOrElse(None)
Copy link
Owner

Choose a reason for hiding this comment

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

For parsing the constraint dates, you could use the same date-parser as used in the .ledger parser. So the date format will match the one in the .ledger.

@hrj hrj added this to the Version 0.3.5 milestone Sep 7, 2016
@hrj hrj added the enhancement label Sep 7, 2016
@hrj
Copy link
Owner

hrj commented Sep 7, 2016

Looks great, thanks!

Apart from the review comments, one more thing remains: we need to add a "suppress error" directive, for posting opening balances.

@Entea Entea force-pushed the transaction_validation_date_range branch from ca03b71 to 1e87401 Compare September 7, 2016 18:51
@Entea
Copy link
Contributor Author

Entea commented Sep 7, 2016

@hrj how exactly this directive should look like? An entry in config file? Or is it about the #93

@Entea Entea force-pushed the transaction_validation_date_range branch from 1e87401 to f5d47ea Compare September 7, 2016 18:58
val to = dateTo.getOrElse(LocalDate.MAX)
val date = LocalDate.of(post.date.year, post.date.month, post.date.day)
val from = dateFrom.getOrElse(Date.MIN)
val to = dateTo.getOrElse(Date.MAX)
Copy link
Owner

@hrj hrj Sep 7, 2016

Choose a reason for hiding this comment

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

While MIN and MAX works in most cases, it is hackish. It would be better to use something like this (pseudo code):

dateFrom.map(_ compare post.date).getOrElse(true)

@hrj
Copy link
Owner

hrj commented Sep 7, 2016

@Entea Yeah, we can follow it up later in #93

The idea is to let the user specify a reserved tag that forcefully accepts a transaction even if it is out of the date-range.

@Entea Entea force-pushed the transaction_validation_date_range branch from d741b02 to 8273dff Compare September 8, 2016 07:12
@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 68.661% when pulling 8273dff on Entea:transaction_validation_date_range into d570296 on hrj:master.

@Entea
Copy link
Contributor Author

Entea commented Sep 8, 2016

@hrj Removed the MAX, MIN, thanks for the hint! Squashed the commits into one, should be ready for merging :) Had to downgrade scalatest to 2.2.6 though.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.7%) to 68.661% when pulling 8273dff on Entea:transaction_validation_date_range into d570296 on hrj:master.

@hrj hrj merged commit 9b3ff51 into hrj:master Sep 8, 2016
@hrj
Copy link
Owner

hrj commented Sep 8, 2016

💯 for taking care of everything :)

@hrj hrj modified the milestones: Version 0.3.5, Version 0.3.1 Sep 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants