scottohara / loot

An implementation of some of the core MS Money features in Ruby on Rails
MIT License
4 stars 3 forks source link

Error entering split transaction where a split has no subcategory #177

Closed scottohara closed 4 years ago

scottohara commented 4 years ago

Repro steps

Enter a new transaction as follows:

Loot_-_Account_Transactions

On save, console error shows:

TypeError: Cannot read property 'id' of undefined
    at bt.primaryAccountSelected (edit.ts:338)
    at fn (eval at compile (angular.js:15651), <anonymous>:4:178)
    at i (angular.js:27475)
    at l.$eval (angular.js:18542)
    at l.$apply (angular.js:18641)
    at HTMLInputElement.<anonymous> (angular.js:27480)
    at HTMLInputElement.dispatch (jquery.js:5183)
    at HTMLInputElement.v.handle (jquery.js:4991)

...and the POST /transactions request returns a 500 Internal Server Error with the following:

G::NotNullViolation: ERROR:  null value in column "category_id" violates not-null constraint
DETAIL:  Failing row contains (53027, null, 2020-02-23 00:24:59.626293, 2020-02-23 00:24:59.626293).
scottohara commented 4 years ago

Unable to reproduce in dev using the repo example above?

scottohara commented 4 years ago

Based on the 500 Internal Server Error response, the only non-nullable column in the database called category_id exists in the transaction_categories table.

This suggests that the error originates from line 20 in the SubTransaction::create_from_json class method:

https://github.com/scottohara/loot/blob/61d120a454033c26dcd2240fee0cf647be9e51c3/app/models/sub_transaction.rb#L15-L22

My guess is that when the error occurs in production, the subcategory of the 3rd split ("Home Repair") was not actually nil (despite the typeahead being blank and showing the Subcategory placeholder text).

Perhaps it's a zero-length string? The TransactionEditController.isString() function in the front-end only returns true when the value is both typeof string and non-zero length. So a zero-length string wouldn't style the field as a warning (to indicate a new subcategory will be created).

On the server, assuming json['subcategory'] is an empty string, this expression at line 17 would attempt to create a new subcategory:

Category.find_or_new json['subcategory'], category unless json['subcategory'].nil?

The Category model has validates :name, presence: true so a zero-length string for name would generate an invalid category instance.

When SplitTransaction#save! is eventually called, way down in the model SplitTransaction.transaction_splits.trx.transaction_category.category would fail validation, causing the entire thing to fail.

Proposed fix:

Change the unless json['subcategory'].nil? expression to if json['subcategory'].present?; to account for the value being either nil or an empty string.

There are 2 other occurrences of the same code in BasicTransaction::create_from_json and BasicTransaction#update_from_json, so these should be updated as well.