Add support for checking a selected online quote in related settings dialog
AbandonedPublic

Authored by habacker on Jun 15 2018, 11:15 AM.

Details

Reviewers
tbaumgart
Summary

To support this function, it was necessary to add detailed error results
to the WebPriceQuote class. In addition, some corrections to the output
of status and error messages were necessary.

CCBUG:388449

Test Plan

compiled and test on linux

Diff Detail

Repository
R261 KMyMoney
Branch
4.8
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 68
Build 68: arc lint + arc unit
habacker requested review of this revision.Jun 15 2018, 11:15 AM
habacker created this revision.
tbaumgart requested changes to this revision.Jun 15 2018, 12:20 PM

Can you provide a screen shot of the enhanced dialog?

kmymoney/converter/webpricequote.cpp
267

This might not work (or-ing multiple errors) if you do not define them as bit-values but plain enums. Since you evaluate the bits in KSettingsOnlineQuotes::setupIcons(), I suggest you use

class Errors {
  public:
    enum Type {
      None =       0x00000000,
      Data =       0x00000001,
      Date =       0x00000002,
      DateFormat = 0x00000004,
      Price =      0x00000008,
      Script =     0x00000010,
      Source =     0x00000020,
      Symbol =     0x00000040,
      Success =    0x00000080,
      URL =        0x00000100
    };

as defintion for the enum.

kmymoney/dialogs/settings/ksettingsonlinequotes.cpp
261

Can we have named constants (enums) for the states please?

311

I would write this function as:

void KSettingsOnlineQuotes::slotLogQuote(const QString &id, const QString &symbol, const QString &date, double price)
{
  slotLogStatus(QString("%1 %2 %3 %4").arg(id, symbol, date).arg(price));
}

It avoid code duplication. The optimizer should take care of the alleged overhead here very well.

kmymoney/dialogs/settings/ksettingsonlinequotesdecl.ui
36

A hint to the forum at https://forum.kde.org/viewforum.php?f=69 might also be valuable.

84

Does this really describe the date format?

This revision now requires changes to proceed.Jun 15 2018, 12:20 PM
habacker marked an inline comment as done.Jun 15 2018, 5:25 PM

check result after pressing 'check source' from a working source


check result after pressing 'check source' from a non functional source (missing price and date)

Please note that the screenshot also shows a "Show page' button to be able to show the source url for GBP or BTC/GBP depending if the source uses one or two parameter. The related code will be in the next update of the patch

kmymoney/converter/webpricequote.cpp
267

There is no issue here, because class Errors stores the enum values of type 'Type' in a QList<Type>, see webpriceqoute.h:140ff. The statement errors |= Errors:Symbol. is managed by class Errors operator |= and simply add the enum value to the internal list.

kmymoney/dialogs/settings/ksettingsonlinequotes.cpp
261

sure or I can use two methos clearIcons() and setupIcons(Errors errors)

311

will change the implementation

kmymoney/dialogs/settings/ksettingsonlinequotesdecl.ui
36

I will check that

84

I will check that too

habacker updated this revision to Diff 36217.Jun 16 2018, 8:09 AM
habacker marked an inline comment as done.
  • fixes issues
  • add show page button for furthe assistance
  • add delete button to remove obsolate entries
tbaumgart requested changes to this revision.Jun 16 2018, 9:49 AM
tbaumgart added inline comments.
kmymoney/converter/webpricequote.cpp
56

I see some potential problem here that once noError is used in a method call as default and an error occurs, noError will keep the error for as long as the application lives. Use a temporary in the case of default value for parameters to avoid this, e.g.

bool launch(const QString& _symbol, const QString& _id, const QString& _source = QString(), Errors &_errors = Errors());

as you do with QStrings already.

267

The fun of operator overloading. I did not spot this but you're right. Case closed.

This revision now requires changes to proceed.Jun 16 2018, 9:49 AM
habacker updated this revision to Diff 36242.Jun 16 2018, 8:30 PM
habacker marked 11 inline comments as done.Jun 17 2018, 10:49 AM

Just a question:

kmymoney/dialogs/settings/ksettingsonlinequotes.cpp
299

If a URL contains'%2', I can trust that this is a query for a currency and otherwise a query for a security or similar? If that is the case, I cannot use "GBP" for the second case, but must use something else - Do you have a suggestion?

tbaumgart suggested to add this to alkimia library instead

see D16973 for an alkimia based followup

habacker abandoned this revision.Jan 14 2019, 10:14 PM

This feature has been integrated into alkimia instead.