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

Piggybanks with single quotes in their name get filled from withdrawals #605

Closed
Zsub opened this issue Mar 1, 2017 · 9 comments
Closed

Comments

@Zsub
Copy link
Contributor

Zsub commented Mar 1, 2017

That may not the the most descriptive title ever.

It is possible to get the amount left for piggybanks below zero. I have verified this on the demo site. Steps are:

  1. Fill all existing piggybanks for a savings account, so there is zero money left for new piggybanks.
  2. Create a new piggybank for that savings account with a single quote in its name (such as 'puter, because you have just seen the LEGO Batman movie), with a target amount (such as €100).
  3. Create a new withdrawal from the checking account, for example for €10.
  4. Observe that there is now -€10 available for piggybanks and the 'puter piggybank contains €10.
  5. Rename the piggybank (for example to Computer).
  6. Create another withdrawal, which does not get deposited in the piggybank.

Findings from (ongoing) debugging:

  • It is not possible to reproduce this for piggybanks linked to the same account as the withdrawal.
@Zsub Zsub changed the title It Piggybanks with accents in their name get filled from withdrawals Mar 1, 2017
@Zsub Zsub changed the title Piggybanks with accents in their name get filled from withdrawals Piggybanks with single quotes in their name get filled from withdrawals Mar 1, 2017
@JC5
Copy link
Member

JC5 commented Mar 1, 2017

Wait, in step 3, which account do you take money from?

After step 1 and 2 I have this. Note I removed the quote on purpose to see if it would matter.

screen shot 2017-03-01 at 20 44 40

@Zsub
Copy link
Contributor Author

Zsub commented Mar 1, 2017

Step 3 has to take money from the checking account (so the not-savings account).

@JC5
Copy link
Member

JC5 commented Mar 1, 2017

Found it. Moving money away from the transfer account creates an empty event. It is tied to a piggy bank from which no money can be taken. But the event is created anyway.

screen shot 2017-03-01 at 20 46 52

@JC5
Copy link
Member

JC5 commented Mar 1, 2017

This:

screen shot 2017-03-01 at 20 47 49

Is annoying but possible, yes. Technically it's a bug but I'm not sure if I want to intervene by editing piggies.

@Zsub
Copy link
Contributor Author

Zsub commented Mar 1, 2017

True, but this is not the full extent of the bug yet. It's possible to tie a transaction to a piggy this way, even when no piggy should be involved.

And by 'tie a transaction to a piggy' I mean increase the piggy by the withdrawal amount up to the target savings for that piggy.

@JC5
Copy link
Member

JC5 commented Mar 1, 2017

Yep, you're right. The event should not have been created. Nice catch!

JC5 added a commit that referenced this issue Mar 1, 2017
@Zsub
Copy link
Contributor Author

Zsub commented Mar 1, 2017

It seems to be a persistent bug. I'm not sure yet if/how this is related but who knows.

[2017-03-01 20:27:20] testing.DEBUG: Going to store accounts for type Withdrawal
[2017-03-01 20:27:20] testing.DEBUG: Now in storeWithdrawalAccounts().
[2017-03-01 20:27:20] testing.DEBUG: Source account is #1 ("HALLO")
[2017-03-01 20:27:20] testing.DEBUG: destination account name is "expacct", account is 11
[2017-03-01 20:27:20] testing.DEBUG: Transaction stored with ID: 19
[2017-03-01 20:27:20] testing.DEBUG: Transaction stored with ID: 20
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""what"" with value ""withdrawal"" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""date"" with value "{"date":"2017-03-01 00:00:00.000000","timezone_type":3,"timezone":"UTC"}" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""tags"" with value "[""]" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""currency_id"" with value "1" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""description"" with value ""withdr"" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""amount"" with value "7" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""budget_id"" with value "0" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""category"" with value """" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""source_account_id"" with value "1" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""source_account_name"" with value """" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""destination_account_id"" with value ""1"" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""destination_account_name"" with value ""expacct"" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Could not store meta field ""piggy_bank_id"" with value "1" for journal #10
[2017-03-01 20:27:20] testing.DEBUG: Trying to connect journal 10 to piggy bank 1.
[2017-03-01 20:27:20] testing.DEBUG: Found piggy bank #1: "'PIGGY"
[2017-03-01 20:27:20] testing.DEBUG: Will add/remove 7.000000 to piggy bank #1 ("'PIGGY")
[2017-03-01 20:27:20] testing.DEBUG: Created piggy bank event #9
[2017-03-01 20:27:23] testing.DEBUG: Final format: "-%s%v"
[2017-03-01 20:27:23] testing.DEBUG: Final format: "%s%v"

Strangely, when I removed the single quote from the piggy's name, it works:

[2017-03-01 20:41:06] testing.DEBUG: Going to store accounts for type Withdrawal
[2017-03-01 20:41:06] testing.DEBUG: Now in storeWithdrawalAccounts().
[2017-03-01 20:41:06] testing.DEBUG: Source account is #1 ("HALLO")
[2017-03-01 20:41:06] testing.DEBUG: destination account name is "rrrrrr", account is 12
[2017-03-01 20:41:06] testing.DEBUG: Transaction stored with ID: 21
[2017-03-01 20:41:06] testing.DEBUG: Transaction stored with ID: 22
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""what"" with value ""withdrawal"" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""date"" with value "{"date":"2017-03-01 00:00:00.000000","timezone_type":3,"timezone":"UTC"}" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""tags"" with value "[""]" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""currency_id"" with value "1" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""description"" with value ""5rrrr"" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""amount"" with value "4" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""budget_id"" with value "0" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""category"" with value """" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""source_account_id"" with value "1" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""source_account_name"" with value """" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""destination_account_id"" with value ""1"" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""destination_account_name"" with value ""rrrrrr"" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Could not store meta field ""piggy_bank_id"" with value "0" for journal #11
[2017-03-01 20:41:06] testing.DEBUG: Trying to connect journal 11 to piggy bank 0.
[2017-03-01 20:41:06] testing.ERROR: No such piggy bank!
[2017-03-01 20:41:06] testing.ERROR: No such piggy bank or no repetition on 2017-03-01
[2017-03-01 20:41:09] testing.DEBUG: Final format: "-%s%v"
[2017-03-01 20:41:09] testing.DEBUG: Final format: "%s%v"

Somehow it now finds a different piggy id (namely zero) to connect to the journal.

@Zsub
Copy link
Contributor Author

Zsub commented Mar 1, 2017

After some more digging, I found out that the withdrawal form always submitted a piggy ID other than zero for me. I also noticed that if I temporarily switch to creating a transfer, my piggy with the single quote it selected by default. If I manually select the (none) piggy, and switch back to the withdrawal screen, the transaction processes as expected. A possible cause might be that ' sorts before (, and I'm trying to verify that right now.

@Zsub
Copy link
Contributor Author

Zsub commented Mar 1, 2017

Yup. Found the culprit. It's asort($piggies); on line 148 in SingleController.php, combined with {{ ExpandedForm.select('piggy_bank_id', piggies) }} on line 80 in views/transactions/single/create.twig. This can be solved by setting the selected attribute for the (none) option, so that always is the default independent of whacky user input.

Zsub added a commit to Zsub/firefly-iii that referenced this issue Mar 1, 2017
Fixes firefly-iii#605. By explicitly setting the selected piggybank to the 0-piggy, new transactions will not inadvertently get coupled to a piggybank if the piggy’s name starts with characters that get sorted before `(` (such as `!` or `’`).
@Zsub Zsub closed this as completed Mar 2, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jan 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants