search

dvankley / firefly-plaid-connector-2

Connector to pull Plaid financial data into the Firefly finance tool
GNU General Public License v3.0
94 stars 14 forks source link

Firefly Transaction Formatting Enhancements #116

Closed nprzy closed 3 weeks ago

nprzy commented 3 weeks ago

This PR enhances the Firefly transactions written by firefly-plaid-connector-2 in the following ways:

  1. Populate the Firefly externalUrl, latitude, and longitude fields, if the corresponding Plaid fields were present.
  2. Prefer Plaid's authorizedDatetime over the datetime field. If authorizedDatetime is not available, we will fallback to datetime. And datetime will also still be preserved as the Firefly processDate field. My motivation for this comes from the Plaid documentation, which states: "The authorized_date, when available, is generally preferable to use over the date field for posted transactions, as it will generally represent the date the user actually made the transaction."
  3. Resolves https://github.com/dvankley/firefly-plaid-connector-2/issues/109 by allowing for customization of the Firefly description string. The README is updated with explanation of the formatting and examples. The behavior of each example from the README is also demonstrated via unit tests. I left the default configuration set to the current format, but I do think it might make sense to update the default. I have been using the expression "originalDescription ?: merchantName ?: name" in my stack for the last few weeks and think it would make a sane default.
dvankley commented 3 weeks ago

@nprzy maybe the default commented out value of descriptionExpression in application.yml should be your recommended value of "transaction.originalDescription ?: transaction.merchantName ?: transaction.name" so users are more likely to adopt that value than the legacy fallback?

nprzy commented 3 weeks ago

maybe the default commented out value of descriptionExpression in application.yml should be your recommended value of "transaction.originalDescription ?: transaction.merchantName ?: transaction.name" so users are more likely to adopt that value than the legacy fallback?

I personally find that having the commented-out value reflect the default is helpful in understanding the impact of uncommenting and customizing a configuration. I would bias towards a written recommendation for that value in the comments instead. I can open a PR for it, but it might be a few weeks.

dvankley commented 3 weeks ago

I personally find that having the commented-out value reflect the default is helpful in understanding the impact of uncommenting and customizing a configuration.

Indeed.

I would bias towards a written recommendation for that value in the comments instead.

I buy that. I can make that change easily enough.