Move ibanbicdata to a plugin
ClosedPublic

Authored by wojnilowicz on May 31 2018, 6:11 PM.

Details

Summary

This patch makes Qt5::Sql dependency optional instead of required.
IBAN/BIC data provides the same functionality to KMM, but now is used only if available. Alas the feature did not work properly even before the patch, so it's disabled by default.
Changes:

  1. new KMyMoneyPlugin::DataPlugin

It returns information after providing keyword and information's type.
It could be used not only for IBAN/BIC but also for new online quotes system.

  1. all loaded plugins available in shared kmm_plugin

We already have some parts of KMM where plugins are needed e.g. accounts view or online jobs outbox. Instead of building chain of passing plugin pointers we could have centralized plugins location. In that way we could even use one plugin in another one.

  1. ibanbic and nationalaccount moved to kmm_mymoney

Their definitions aren't optional and are required during a read of XML or DB file, so by not making them loadable on demand removes unnecessary step.
The move was necessary also because of dependency hell of little libraries that were interdependent (circular dependency) and couldn't be set in any other configuration.

  1. decimated kmm_payeeidentifier_loader and made it shared

It was part of dependency hell. Let's hope that after this there will be no multiple definition issue with different compilers.

Test Plan

All tests pass. Added and saved a payee identifier and online task.

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.
wojnilowicz requested review of this revision.May 31 2018, 6:11 PM
wojnilowicz created this revision.
wojnilowicz updated this revision to Diff 35391.Jun 2 2018, 9:57 AM

Added some missing methods.

tbaumgart requested changes to this revision.Jun 2 2018, 12:54 PM

Works for me so far. Here's what I have tested:

  1. Loading XML file
  2. Make a change
  3. Save as XML file
  4. Quit and restart
  5. Load the XML file saved in step 3
  6. Verify that payeeidentifier are still present
  7. Verify that online job information is still present

This looks good so far.

  1. Load XML file
  2. Save as database
  3. Quit and restart (loads the database)
  4. Verify that payeeidentifier are still present
  5. Verify that online job information is still present

Except step 5. everything seems to work. I have to mention, that 5. also does not work on master, so it is not this patch which breaks the feature. Nevertheless, this is a bug that needs to be fixed before we can be sure that this change works as expected with databases.

kmymoney/mymoney/CMakeLists.txt
49

s/muss/must/

kmymoney/mymoney/payeeidentifier/ibanbic/ibanbic.h
253

please remove if not needed (anymore)

kmymoney/plugins/sql/kmymoneystorageplugin.h
21

please remove if not needed anymore

This revision now requires changes to proceed.Jun 2 2018, 12:54 PM
wojnilowicz added a comment.EditedJun 2 2018, 1:39 PM

Works for me so far. Here's what I have tested:

  1. Loading XML file
  2. Make a change
  3. Save as XML file
  4. Quit and restart
  5. Load the XML file saved in step 3
  6. Verify that payeeidentifier are still present
  7. Verify that online job information is still present

    This looks good so far.
  8. Load XML file
  9. Save as database
  10. Quit and restart (loads the database)
  11. Verify that payeeidentifier are still present
  12. Verify that online job information is still present

    Except step 5. everything seems to work. I have to mention, that 5. also does not work on master, so it is not this patch which breaks the feature. Nevertheless, this is a bug that needs to be fixed before we can be sure that this change works as expected with databases.

Works for me so far. Here's what I have tested:

  1. Loading XML file
  2. Make a change
  3. Save as XML file
  4. Quit and restart
  5. Load the XML file saved in step 3
  6. Verify that payeeidentifier are still present
  7. Verify that online job information is still present

    This looks good so far.
  8. Load XML file
  9. Save as database
  10. Quit and restart (loads the database)
  11. Verify that payeeidentifier are still present
  12. Verify that online job information is still present

    Except step 5. everything seems to work. I have to mention, that 5. also does not work on master, so it is not this patch which breaks the feature. Nevertheless, this is a bug that needs to be fixed before we can be sure that this change works as expected with databases.

I don't agree that not saving online jobs in sql should hold this patch but accidentally I fixed it and will commit soon. BTW. It didn't work in 4.8 neither.

This revision was not accepted when it landed; it landed in state Needs Revision.Jun 2 2018, 2:19 PM
This revision was automatically updated to reflect the committed changes.