Bug 311481 - It is possible to create a schedule with empty name
Summary: It is possible to create a schedule with empty name
Status: RESOLVED FIXED
Alias: None
Product: kmymoney
Classification: Applications
Component: general (show other bugs)
Version: git (master)
Platform: MacPorts All
: NOR normal
Target Milestone: ---
Assignee: KMyMoney Devel Mailing List
URL: http://glenbox.free.fr/videos/kmymone...
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-10 20:12 UTC by Marko Käning
Modified: 2019-02-22 07:33 UTC (History)
2 users (show)

See Also:
Latest Commit:
Version Fixed In:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Marko Käning 2012-12-10 20:12:39 UTC
When creating a schedule it is possible to leave the name edit box empty. 



Reproducible: Always

Steps to Reproduce:
1. Create a schedule w/o name
2. Enter then something into the "Pay To" edit field 
3. This makes the OK button become valid, which is NOT wanted!
Actual Results:  
OK button clickable

Expected Results:  
OK should stay disabled
Comment 1 Thomas Baumgart 2012-12-10 20:42:22 UTC
The problem is with the two methods to control the enabled state of the OK button in this case. One is the kMandatoryFieldGroup created in the KEditScheduleDlg ctor and the other the line

  connect(editor, SIGNAL(transactionDataSufficient(bool)), buttonOk, SLOT(setEnabled(bool)));

in KEditScheduleDlg::startEdit. To fix the issue, the results of both methods need to be ANDed, but I currently have no idea how to solve this. Any takers? Reviewboard is waiting for your suggestions.
Comment 2 allan 2012-12-12 22:42:32 UTC
(In reply to comment #1)
> The problem is with the two methods to control the enabled state of the OK
> button in this case. One is the kMandatoryFieldGroup created in the
> KEditScheduleDlg ctor and the other the line
> 
>   connect(editor, SIGNAL(transactionDataSufficient(bool)), buttonOk,
> SLOT(setEnabled(bool)));
> 
> in KEditScheduleDlg::startEdit. To fix the issue, the results of both
> methods need to be ANDed, but I currently have no idea how to solve this.
> Any takers? Reviewboard is waiting for your suggestions.

While not fully understanding what is going on in this area, I notice that on creating a new schedule, without touching any other widgets, selecting a payee immediately enables the OK button.  As against that, I also notice that if an account and category are selected, the OK button is not enabled until a schedule name is entered.

As the payee is not a 'mandatory field', this seemed strange, so I removed the connect(payee, SIGNAL(textChanged(QString)), this, SLOT(slotUpdateButtonState())); on line 753 of transactioneditor.cpp.

This seems to achieve the required result.

Too simplistic?
Comment 3 Thomas Baumgart 2012-12-13 08:24:37 UTC
Yes, too simplistic, because now you can create a 'transaction' with some missing fields. It is a scheduled transaction but what happens if you enter it? This line serves a purpose when entering transactions in the ledger, so you cannot simply remove it.
Comment 4 allan 2012-12-13 10:24:23 UTC
(In reply to comment #3)
> Yes, too simplistic, because now you can create a 'transaction' with some
> missing fields. It is a scheduled transaction but what happens if you enter
> it? This line serves a purpose when entering transactions in the ledger, so
> you cannot simply remove it.

That wasn't really meant to be the full solution.  However, I did do a little testing last night, and have repeated that this morning.

So far, I can't see any diffrence in transaction entry.  Without the change, it's only necessary to enter a value and the transaction is saved, although flagged as *** Unassigned ***.  Entering the account, the transaction may be saved again, without the warning.

I haven't yet looked into investments.
Comment 5 allan 2012-12-13 10:57:34 UTC
(In reply to comment #3)
> ..... but what happens if you enter
> it? This line serves a purpose when entering transactions in the ledger, so
> you cannot simply remove it.

Picking up on this point, the schedule may be entered and shows in the ledger.  No amount is necessary.
Comment 6 allan 2012-12-13 11:15:12 UTC
There is a very similar problem, too.   With the original code, opening a new schedule and entering just an amount also enables the OK button.  Again, 'Amount' is not a mandatory field.

Removing the same code line for 'Amount' - 
connect(value, SIGNAL(textChanged(QString)), this, SLOT(slotUpdateButtonState()));
on line 826 in transactioneditor.cpp again removes that issue.  Schedule and transaction entry still work as before, so far as I can see, with no requirement for amount entry.  It resolves to 0.00.
Comment 7 allan 2012-12-13 11:35:03 UTC
OK, I see it now.  The 'fix' does fix the initial problem, allowing schedule and Transaction entry.  However, If the schedule is edited, say adding or changing the amount, that appears to be accepted, but is not.

So, that's why the line is is there, but it needs not to be there when creating a schedule.
Comment 8 allan 2012-12-13 13:09:13 UTC
(In reply to comment #7)
> OK, I see it now.  The 'fix' does fix the initial problem, allowing schedule
> and Transaction entry.  However, If the schedule is edited, say adding or
> changing the amount, that appears to be accepted, but is not.
> 
> So, that's why the line is is there, but it needs not to be there when
> creating a schedule.

Erm...
Please ignore this comment.  I cannot now fault the behaviour.

I suspect I had edited the schedule entry being entered, and not the actual value in the schedule itself (next payment).  I'll set up some more schedules for tomorrow.
Comment 9 allan 2013-01-10 17:24:03 UTC
Git commit 328c77508aed7fabd1e8d0a239a7e9bb01de56a5 by Allan Anderson.
Committed on 13/12/2012 at 23:22.
Pushed by allananderson into branch 'master'.
Also, the amount field affects the OK button status.
Make minor change to investmentwizardpage.ui, to fix width where a label was getting clipped, and remove two trailing spaces.

M  +1    -2    kmymoney/dialogs/transactioneditor.cpp
M  +4    -4    kmymoney/plugins/csvimport/investmentwizardpage.ui

http://commits.kde.org/kmymoney/328c77508aed7fabd1e8d0a239a7e9bb01de56a5
Comment 10 Marko Käning 2013-01-11 22:38:45 UTC
Sorry, but I have to re-open this issue!

Select a valid tag and the OK button will be enabled even if the schedule name is empty.
Comment 11 allan 2013-01-12 00:26:38 UTC
(In reply to comment #10)
> Sorry, but I have to re-open this issue!
> 
> Select a valid tag and the OK button will be enabled even if the schedule
> name is empty.
Paraphrasing Alvaro, it's a bit of a bag of worms here, still.  I don't see entering just a tag enabling the OK button, but entering a tag and immediately clicking in the memo field does, and it also clears the tag.  In fact, entering text in the tag field, then clicking any input widget in the transaction area, clears the tag, removes it from thr drop-down, and enables the OK button.  With a new schedule, entering two characters in the memo field, then entering an amount, then adding a third memo character enables the OK button.

I got the impression from Alvaro, "That area of code is a big mess. For example, it's what took longer to migrate and stabilize in KDE4.  I'd say it's good enough for now.", he felt a more thorough rework was necessary.

However, I knew there were other gremlins there, and am happy to take it further in the meantime.
Comment 12 allan 2013-01-16 20:50:14 UTC
(In reply to comment #10)
> Sorry, but I have to re-open this issue!
> 
> Select a valid tag and the OK button will be enabled even if the schedule
> name is empty.

Not unexpected!

Might you be able to try the second patch in https://git.reviewboard.kde.org/r/107714/ ?  I haven't been able to fault it, apart from some difficulty with the Tag field, as mentioned in the review and comment 11 here.
Comment 13 allan 2013-01-23 13:17:51 UTC
Git commit a8fa09f5ba1e6eb2a019089c7071a4ccef0cb356 by Allan Anderson.
Committed on 12/01/2013 at 12:51.
Pushed by allananderson into branch 'master'.
slotUpdateButtonState() from remaining non-mandatory widgets.

M  +0    -7    kmymoney/dialogs/transactioneditor.cpp

http://commits.kde.org/kmymoney/a8fa09f5ba1e6eb2a019089c7071a4ccef0cb356
Comment 14 allan 2013-03-06 23:04:28 UTC
Git commit acb4a9b9b039317ee705600381e5849124322a66 by Allan Anderson.
Committed on 06/03/2013 at 23:41.
Pushed by allananderson into branch 'master'.
Related: bug 107714
On entering a new tag, allow dialog to appear asking if I want to accept the
 new tag.

M  +1    -0    kmymoney/dialogs/keditscheduledlg.cpp
M  +1    -0    kmymoney/dialogs/kenterscheduledlg.cpp

http://commits.kde.org/kmymoney/acb4a9b9b039317ee705600381e5849124322a66
Comment 15 allan 2013-06-01 16:31:42 UTC
Git commit 94935cf78e0135e2bb0cceff3c0dbde8c1c07e6d by Allan Anderson.
Committed on 06/03/2013 at 23:41.
Pushed by allananderson into branch 'master'.
Related: bug 107714, bug 314955, bug 316111
On entering a new tag, allow dialog to appear asking if I want to accept the
 new tag.

M  +3    -0    kmymoney/dialogs/investactivities.cpp
M  +2    -1    kmymoney/dialogs/keditscheduledlg.cpp
M  +44   -10   kmymoney/dialogs/transactioneditor.cpp
M  +39   -7    kmymoney/widgets/kguiutils.cpp
M  +3    -2    kmymoney/widgets/register.cpp

http://commits.kde.org/kmymoney/94935cf78e0135e2bb0cceff3c0dbde8c1c07e6d