Skip to content

Commit

Permalink
Bug 777949 - Accounts implicitly created in ledger attempt creation t…
Browse files Browse the repository at this point in the history
…wice

Guard against recursively calling the account doesn’t exist query or creation
dialog if one is already in the account creation dialog.

The underlying problem is that creating the dialog forces a UI update that
in turn sets the cell value and checks for the existence of the account.
In basic view the cell being displayed (“transfer”) isn’t the one being
changed (“account”) so the account check isn’t invoked, but in
multi-split view the “account” cell *is* displayed so the check is invoked
again.
  • Loading branch information
jralls committed Mar 10, 2017
1 parent 97598c4 commit bc50f3d
Showing 1 changed file with 4 additions and 2 deletions.
6 changes: 4 additions & 2 deletions src/register/ledger-core/split-register.c
Original file line number Diff line number Diff line change
Expand Up @@ -1833,6 +1833,7 @@ gnc_split_register_get_account_by_name (SplitRegister *reg, BasicCell * bcell,
char *account_name;
ComboCell *cell = (ComboCell *) bcell;
Account *account;
static gboolean creating_account = FALSE;

if (!name || (strlen(name) == 0))
return NULL;
Expand All @@ -1842,15 +1843,16 @@ gnc_split_register_get_account_by_name (SplitRegister *reg, BasicCell * bcell,
if (!account)
account = gnc_account_lookup_by_code(gnc_get_current_root_account(), name);

if (!account)
if (!account && !creating_account)
{
/* Ask if they want to create a new one. */
if (!gnc_verify_dialog (gnc_split_register_get_parent (reg),
TRUE, missing, name))
return NULL;

creating_account = TRUE;
/* User said yes, they want to create a new account. */
account = gnc_ui_new_accounts_from_name_window (name);
creating_account = FALSE;
if (!account)
return NULL;
}
Expand Down

2 comments on commit bc50f3d

@limitedAtonement
Copy link
Contributor

Choose a reason for hiding this comment

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

I think gnucash only uses one thread at a time, right? Since this is GUI code, I think that's not likely to change, so this being not thread-safe should be okay. Is there any notation we should add to this effect?

I'm happy to see this fixed, and happy to see it's such a straightforward fix.

@jralls
Copy link
Member Author

@jralls jralls commented on bc50f3d May 9, 2017

Choose a reason for hiding this comment

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

The register is certainly single-threaded now. This is really "controller" (in the MVC sense) so its theoretically possible to call it from somewhere besides the GUI thread, but the risk is very low.

Please sign in to comment.