> On Jan. 23, 2013, 12:32 p.m., Alessandro Russo wrote:
> > When I added the tags I followed as example the code for memo and payee
> > fields, so if that line of code was wrong for the memo and payee fields it
> > will be wrong also for the tag field.
>
> Allan Anderson wrote:
> The patch I proposed takes care of the enabling of the OK button issues,
> including those with TAGs, but I think you'll find that the other points I
> raised may still exist.
>
> I'm just about to push this fix, so we'll soon see.
>
> Allan Anderson wrote:
> Erm, there still is a problem with the OK button, to do with the amount
> and memo fields, which I thought I'd fixed as part of the patch. I'll need
> to look again.
>
> So far as Tags are concerned, I've assumed that they would function in
> the same way as the payee field. Enter a new name, then move focus to
> another field. A dialog pops up to ask if a new payee should be created.
> With a new tag, however, when focus is moved to another field, the new name
> is cleared and no popup appears.
>
> Whilst this may be what was intended, isn't it a bit inconsistent?
>
>
> Allan Anderson wrote:
> Sorry, false alarm. I had to break off, and when I returned, I found
> there is not still a problem with the OK button, so far as I can see. I'd
> looked at the code and all the calls to slotUpdateButtonState() in that area
> had been removed. I started to think that it must be getting called from
> elsewhere, but it wasn't.
>
> I then realised that I could no longer produce that problem. So, what I
> had done, I don't now know, but possibly had reverted to a pre-patched
> version. Apologies again.
>
> The Tag issue still is there, and there is a variation. If I create a
> new tag, then select that in a new schedule, then click in another field, the
> selection disappears, both from the line-edit and from the drop-down.
> Clicking the red 'x' restores it to the drop-down, which gets us back to
> square one.
>
>
> Alessandro Russo wrote:
> The intended behavior of tag field is the following: After you start
> typing the tag in the tag field a drop down list show up with all the tag
> names that match the characters typed. When you stop typing and click on
> another field (or press the Tab key) if a tag with that name exists than it
> will be added after the tag field with a little red 'X' on the left and the
> name of the tag; the name of this tag will be removed from the combo box list
> (you can't insert the same tag two times), if you click on the red 'x' the
> tag will be removed from the transaction and it will be re-added to the combo
> box (so that you can re-add it if you change idea). If the name that you
> wrote in the tag field is different from the names of existing tags a new
> dialog will appear (like the payee field), there you can choose to create a
> new tag or to close it. If you create the tag it will be automatically added
> to the transaction.
>
> Allan Anderson wrote:
> Yes, I can see it works that way in Ledger view, but not in Schedules.
> I have created a new tag called 'New Tag', and in the schedule dialog,
> I type a 't', and the drop-down appears showing 'New Tag'. I continue to
> type
> 'ag' and 'New Tag' still shows. If I now type a 'u', the drop-down
> disappears.
> So far, so good. Now, and this is where it differs, I have 'tagu' in the
> line-edit,
> and if I now click in the memo field, or any edit field in the
> transaction tab widget,
> but not in the schedule name box, 'tagu' disappears. Clicking in the
> schedule name box
> appears to cause the tag to be 'accepted'. In no way can I get the
> dialog to appear
> asking if I want to accept the new tag.
>
> I was expecting it to function as for a new payee or category, which it
> does in the ledger,
> but not in the new schedule.
>
> Allan
>
> Alessandro Russo wrote:
> Hum.. so maybe there is a bug .. I haven't used the schedules too much...
> In the weekend I'll make some tests.
>
> Marko Käning wrote:
> BTW, is it a bug or a feature that the editor doesn't allow to edit
> overdue payments of schedules?
>
> Alessandro Russo wrote:
> Ok, i found the problem. Here there is the fix, can you modify your patch
> or should I open a different review request?
> diff --git a/kmymoney/dialogs/keditscheduledlg.cpp
> b/kmymoney/dialogs/keditscheduledlg.cpp
> index 7ddf3c3..f3146f3 100644
> --- a/kmymoney/dialogs/keditscheduledlg.cpp
> +++ b/kmymoney/dialogs/keditscheduledlg.cpp
> @@ -201,6 +201,7 @@ TransactionEditor* KEditScheduleDlg::startEdit(void)
> connect(MyMoneyFile::instance(), SIGNAL(dataChanged()), editor,
> SLOT(slotReloadEditWidgets()));
> // connect(editor,
> SIGNAL(finishEdit(KMyMoneyRegister::SelectedTransactions)), this,
> SLOT(slotLeaveEditMode(KMyMoneyRegister::SelectedTransactions)));
> connect(editor, SIGNAL(createPayee(QString,QString&)), kmymoney,
> SLOT(slotPayeeNew(QString,QString&)));
> + connect(editor, SIGNAL(createTag(QString,QString&)), kmymoney,
> SLOT(slotTagNew(QString,QString&)));
> connect(editor,
> SIGNAL(createCategory(MyMoneyAccount&,MyMoneyAccount)), kmymoney,
> SLOT(slotCategoryNew(MyMoneyAccount&,MyMoneyAccount)));
> connect(editor,
> SIGNAL(createSecurity(MyMoneyAccount&,MyMoneyAccount)), kmymoney,
> SLOT(slotInvestmentNew(MyMoneyAccount&,MyMoneyAccount)));
> connect(MyMoneyFile::instance(), SIGNAL(dataChanged()), editor,
> SLOT(slotReloadEditWidgets()));
> diff --git a/kmymoney/dialogs/kenterscheduledlg.cpp
> b/kmymoney/dialogs/kenterscheduledlg.cpp
> index 9f4432f..04752f0 100644
> --- a/kmymoney/dialogs/kenterscheduledlg.cpp
> +++ b/kmymoney/dialogs/kenterscheduledlg.cpp
> @@ -218,6 +218,7 @@ TransactionEditor* KEnterScheduleDlg::startEdit(void)
> connect(MyMoneyFile::instance(), SIGNAL(dataChanged()), editor,
> SLOT(slotReloadEditWidgets()));
> // connect(editor,
> SIGNAL(finishEdit(KMyMoneyRegister::SelectedTransactions)), this,
> SLOT(slotLeaveEditMode(KMyMoneyRegister::SelectedTransactions)));
> connect(editor, SIGNAL(createPayee(QString,QString&)), kmymoney,
> SLOT(slotPayeeNew(QString,QString&)));
> + connect(editor, SIGNAL(createTag(QString,QString&)), kmymoney,
> SLOT(slotTagNew(QString,QString&)));
> connect(editor,
> SIGNAL(createCategory(MyMoneyAccount&,MyMoneyAccount)), kmymoney,
> SLOT(slotCategoryNew(MyMoneyAccount&,MyMoneyAccount)));
> connect(editor,
> SIGNAL(createSecurity(MyMoneyAccount&,MyMoneyAccount)), kmymoney,
> SLOT(slotInvestmentNew(MyMoneyAccount&,MyMoneyAccount)));
> connect(MyMoneyFile::instance(), SIGNAL(dataChanged()), editor,
> SLOT(slotReloadEditWidgets()));
>
> You simply need to add the following line:
> connect(editor, SIGNAL(createTag(QString,QString&)), kmymoney,
> SLOT(slotTagNew(QString,QString&)));
> after the similar line with createPayee in the files
> dialogs/keditscheduledlg.cpp and dialogs/kenterscheduledlg.cpp
>
> (I fixed the same bug in the Enter Scheduled dialog)
>
> Allan Anderson wrote:
> My patches are committed already, but it doesn't make sense really to
> start a new review,
> having got this far, so I've added your changes here.
>
> If I type in a new tag name and then click in one of the edit fields, the
> dialog now appears
> asking whether to add it. It doesn't appear though if I focus on the
> schedule name, or widgets
> not in the transaction area, or click outside the transaction widgets.
>
> If I am creating a new schedule, and type in all the fields, the entries
> remain visible in the
> line edit boxes. This isn't the case with a new tag. If I type in a new
> tag name and accept it,
> it gets cleared. I have to click the red cross then select it from the
> combo. With the name now
> showing, however, if I click the memo field it goes again. It also is
> possible to get more than
> one red cross visible, together with the added tag name alongside.
>
> If I have added a new tag to a schedule, and then decide to delete that
> tag, and it is the only
> tag, I get a message saying I can't delete the last tag as there needs to
> be one to reassign to.
> I understand that, but if I have a bunch of schedules, how do I find
> which is the one referencing
> that tag, to allow me to remove it? Search transaction shows the tag,
> but selecting it doesn't
> enable Find.
>
> Apologies if I seem 'picky'. I didn't set out to undermine your good
> work, and it is quite a
> big change.
>
>
> Alessandro Russo wrote:
> Hi, no, you don't seem picky, I appreciate your testing.
> The first problem (clicking on the schedule name and in other places) is
> present also for the payee and category/account fields. So we should fix them
> too.
> The second problem is the intended behavior. You can have multiple tags
> so the behavior is this: you have to write (or select in the combo) a tag (or
> write a new tag and click 'yes') when you press tab or change focus your tag
> is 'added' by removing it from the combo box and creating a new label with
> the same name and a 'X' on its left just after the combo box. You can add
> multiple tags in this way. To remove a tag from the transaction/schedule you
> simply have to click on the red X for that tag. I tested it also when adding
> a new tag (by accepting the creation clicking 'yes' in the modal dialog) and
> it works too.
>
> The last problem is a TODO present somewhere in the sources, I used the
> same code for payee that require that every transaction has a payee, for the
> tags maybe isn't needed. You could simply delete the tag and remove it from
> all the transaction. Maybe we could show a dialog offering the possibility
> (not requesting it) to substitute it with another tag, if the user chooses to
> not do it simply remove the tag from all transactions.
>
> About the Search transaction: I couldn't reproduce it. If I select a tag,
> it enables 'Find' and you can search all the transactions with that tag/tags.
> However it can not find any scheduled transaction (this not only for tag
> search but for every search). So the bug is more general.
>
> Allan Anderson wrote:
> > About the Search transaction: I couldn't reproduce it. If I select a
> tag, it enables 'Find' and you can search all the transactions with that
> tag/tags.
> > However it can not find any scheduled transaction (this not only for
> tag search but for every search). So the bug is more general.
>
> I had just created and saved a new schedule having a new tab. I hadn't
> got to the stage of creating a transaction with that tab. As I had completed
> what I was testing, I decided to delete the tab, but couldn't remember which
> schedule I'd added it to. The Schedules list doesn't show tabs, so that's
> why I tried the transaction search. Of course, I now realise there weren't
> any transactions with the tab to find, but couldn't get to that stage as Find
> was disabled.
>
> Whether or not it is a real requirement to show, or find, which schedules
> use a particular tab, I'll leave to you.
>
> One final point. If I create a new schedule and set up all the fields
> except tag, then select a tag and immediately click OK, the tag isn't entered.
>
> Prior to looking at tags, I had been going over new schedule creation
> with a fine tooth comb looking at OK button enabling, and that's how I
> approached tabs, so nothing personal <BG>
>
> Alessandro Russo wrote:
> The 'Find' button is not enabled because you have only 1 tag. If you have
> more than 1 tag you can deselect the other tags and leave selected only the
> tag you are searching for. There is the possibility to search for all the
> transaction without tag but not to search the transaction with a tag when
> this tag is the only one. I think we could change the search dialog as
> follow: remove the check 'Select transactions without tags' and put at the
> top of the tags list a row with the name 'None'. So to replicate the old
> functionality you have to deselect all the tags except for the first one
> ('None'). To search for a specific tag when you have only 1 you have to
> deselect 'None' and select the tag.
>
> For the last fix we need to check, when the 'Ok' button is clicked, if
> there is a tag to be added in the tag field and if so add it.
>
> I don't know if it's needed to add a 'tag' column in the scheduled
> transaction list, do you have so many of them?
> I don't know if it's needed to add a 'tag' column in the scheduled
> transaction list, do you have so many of them?
Nope! I have none. My requirements are pretty simple, and I don't, so far,
have a need for them. But, I do have quite
a few schedules, so wouldn't fancy plodding through them all.
- Allan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/107714/#review26063
-----------------------------------------------------------
On Jan. 12, 2013, 12:07 p.m., Allan Anderson wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/107714/
> -----------------------------------------------------------
>
> (Updated Jan. 12, 2013, 12:07 p.m.)
>
>
> Review request for KMymoney.
>
>
> Description
> -------
>
> The problem as originally reported was that in Schedules view, the OK button
> became enabled even though no schedule name had been entered.
>
> It was found that the button became enabled as soon as a payee was entered.
> It was also found that this happened when an amount was entered.
>
> For "payee", line 753 of transactioneditor.cpp has -
> "connect(payee,SIGNAL(textChanged(QString)),this,SLOT(slotUpdateButtonState()))",
> and slotUpdateButtonState() has -
> "emit transactionDataSufficient(isComplete(reason)",
> and 'This signal is sent out whenever enough data is present to enter the
> transaction into the ledger.'
>
> Similarly, for "amount", at line 826, the same line appears.
>
> As neither of these fields is a mandatory one, I believe they should not
> affect the OK button status. So, as shown in the patch, I have temporarily
> disabled these lines. I have done numerous tests of schedule creation and
> editing, and manual entry and editing of transactions without any problem.
>
> The same area of code in transactioneditor.cpp has several more of these
> possibly unneeded lines, although not affecting schedules. For instance,
> even with these two lines out and with no mandatory fields completed, if a
> payee is selected and the memo, tag field, next due date or status is edited,
> the OK button again is enabled wrongly.
>
> I don't really see any valid reason for 'slotUpdateButtonState()' to be in
> this section. What do the wise men think?
>
>
> This addresses bug 311481.
> http://bugs.kde.org/show_bug.cgi?id=311481
>
>
> Diffs
> -----
>
> kmymoney/dialogs/transactioneditor.cpp 8f6f06b
>
> Diff: http://git.reviewboard.kde.org/r/107714/diff/
>
>
> Testing
> -------
>
> Numerous tests of schedule creation and editing, and manual entry and editing
> of transactions without any problem.
>
>
> Thanks,
>
> Allan Anderson
>
>
_______________________________________________
KMyMoney-devel mailing list
KMyMoney-devel@kde.org
https://mail.kde.org/mailman/listinfo/kmymoney-devel
