It has been inadvertently omitted from the last patch. Old read methods are still around but only for an easy comparison with new methods.
Details
Details
- Reviewers
tbaumgart - Group Reviewers
KMyMoney - Commits
- R261:ea34b91bc46c: Move MyMoneySplit reading and writing to XML storage
Diff Detail
Diff Detail
- Repository
- R261 KMyMoney
- Lint
Automatic diff as part of commit; lint not applicable. - Unit
Automatic diff as part of commit; unit tests not applicable.
Comment Actions
I needed to apply the following patch to get correct results. Reason: you must not set the entry date of a scheduled transaction to a valid value, due to the fact that
// some old versions did not remove the entry date and post date fields // in the schedule. So if this is the case, we deal with a very old transaction // and can't use the post date field as next due date. Hence, we wipe it out here
Since you set the entry date prior to this check, such an old entry is assumed and things go wrong. The patch takes care of this.
diff --git a/kmymoney/plugins/xml/mymoneystoragexml.cpp b/kmymoney/plugins/xml/mymoneystoragexml.cpp index 0e5ec0b4..e938e814 100644 --- a/kmymoney/plugins/xml/mymoneystoragexml.cpp +++ b/kmymoney/plugins/xml/mymoneystoragexml.cpp @@ -152,7 +152,7 @@ private: static void writeBaseXML(const QString &id, QDomDocument &document, QDomElement &el); static void addToKeyValueContainer(MyMoneyKeyValueContainer &container, const QDomElement &node); static void writeKeyValueContainer(const MyMoneyKeyValueContainer &container, QDomDocument &document, QDomElement &parent); - static MyMoneyTransaction readTransaction(const QDomElement &node); + static MyMoneyTransaction readTransaction(const QDomElement &node, bool assignEntryDateIfEmpty = true); static void writeTransaction(const MyMoneyTransaction &transaction, QDomDocument &document, QDomElement &parent); static MyMoneySplit readSplit(const QDomElement &node); static void writeSplit(const MyMoneySplit &_split, QDomDocument &document, QDomElement &parent); @@ -514,7 +514,7 @@ void MyMoneyXmlContentHandler::writeKeyValueContainer(const MyMoneyKeyValueConta } } -MyMoneyTransaction MyMoneyXmlContentHandler::readTransaction(const QDomElement &node) +MyMoneyTransaction MyMoneyXmlContentHandler::readTransaction(const QDomElement &node, bool assignEntryDateIfEmpty) { if (nodeName(Node::Transaction) != node.tagName()) throw MYMONEYEXCEPTION_CSTRING("Node was not TRANSACTION"); @@ -525,7 +525,7 @@ MyMoneyTransaction MyMoneyXmlContentHandler::readTransaction(const QDomElement & transaction.setPostDate(QDate::fromString(node.attribute(attributeName(Attribute::Transaction::PostDate)), Qt::ISODate)); auto entryDate = QDate::fromString(node.attribute(attributeName(Attribute::Transaction::EntryDate)),Qt::ISODate); - if (entryDate == QDate()) + if (!entryDate.isValid() && assignEntryDateIfEmpty) entryDate = QDate::currentDate(); transaction.setEntryDate(entryDate); transaction.setBankID(node.attribute(attributeName(Attribute::Transaction::BankID))); @@ -557,7 +557,7 @@ MyMoneyTransaction MyMoneyXmlContentHandler::readTransaction(const QDomElement & } } else if (c.tagName() == nodeName(Node::KeyValuePairs)) { - addToKeyValueContainer(transaction, c.toElement()); + addToKeyValueContainer(transaction, c.toElement()); } child = child.nextSibling(); @@ -1197,7 +1197,7 @@ MyMoneySchedule MyMoneyXmlContentHandler::readSchedule(const QDomElement &node) if (nodeList.count() == 0) throw MYMONEYEXCEPTION_CSTRING("SCHEDULED_TX has no TRANSACTION node"); - auto transaction = readTransaction(nodeList.item(0).toElement()); + auto transaction = readTransaction(nodeList.item(0).toElement(), false); // some old versions did not remove the entry date and post date fields // in the schedule. So if this is the case, we deal with a very old transaction
| kmymoney/mymoney/mymoneysplit.cpp | ||
|---|---|---|
| 51 | Is this needed? It should be initialized in the MyMoneySplitPrivate ctor which is the case. So in my POV this is superfluous. I might be mistaken, in that case a short comment why it is needed would help. | |
| kmymoney/plugins/xml/mymoneystoragexml.cpp | ||
| 545 | Can you add a /// @todo remove after final test or similar doxygen comment here and in other locations where you check the new code against the old? | |
| kmymoney/plugins/xml/mymoneystoragexml.cpp | ||
|---|---|---|
| 545 | Only in one place. | |
Comment Actions
Looks OK to me. Made some tests in comparing the output of the master version and this version which do not show differences.