Review Request 111308: Disallow changing URI of PersonData after creation

[David Edmundson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/111308/
-----------------------------------------------------------

(Updated June 30, 2013, 10:33 p.m.)


Review request for Telepathy, Aleix Pol Gonzalez and Vishesh Handa.


Changes
-------

Update:

Changed PersonData to be a sharedpointer.

In the old code PersonsDetailView deleted the passed PersonData that was last given to it, when you set a new person data. 
If we were to use this in real life we would have crashes everywhere.

For objects shared in multiple places, created and deleted during the runtime of the program sharedptrs are the safest solution.

Due to the above changes, this will still work in QML which has it's own shared pointer solution.


Description
-------

Disallow changing URI of PersonData after creation

Currently setUri and setContactId are public which would cause a lot of bugs if changed.
Instead replace with two clear static methods; loadFromUri and loadFromContactId.

In practice they were never used more than once.

A declarative wrapper using QDeclarativeParserStatus provides the same functionality
in QML. A wrapper is needed anyway as it means we can now put more complex data types in PersonData.


bugs that used to exist which are no longer relevant:
 - every single plugin for PersonsViewer does not respond to uriChanged
 - personsactionsmodel did not respond to uriChanged()
 - contactId would return the last user set with setContactId not the current contacts ID (which is misleading/wrong anyway, as a person can have multiple IDs)
 - isPerson property was not updated when changed
 - PeronDetailsView kept an invalid empty PersonData on construction with a lifespan of the parent.. for no reason.

Given we want people to write plugins, we need to make it easier.
PersonData code is also simpler as it doesn't have to be careful about wiping old resources in setUri.

Downside is you can't have a PersonData on the stack. Bit of a effort for the unit tests.. but not in the real world I don't think.

I believe someone (vishesh?) also suggested this at the Pineda sprint. 


Diffs (updated)
-----

  src/abstractpersonplugin.h 60508b693fd6406b77f4622bff4c4419fb13c50e 
  src/abstractpersonplugin.cpp 3a41f3bcff668994de70d77c77fd5a4a061dadde 
  src/autotests/tests/persondatatests.cpp 623fb5f752cca6d98aa261a72a34b740d74b92e8 
  src/declarative/CMakeLists.txt 9b33b85429c65ce832f37edecebf2ecdf467c476 
  src/declarative/declarativepersondata.h PRE-CREATION 
  src/declarative/declarativepersondata.cpp PRE-CREATION 
  src/declarative/peopleqmlplugin.cpp c991064e011f5115683adb90955c0e3a5dbd156c 
  src/examples/personwidget.cpp bc2eaf2805891a201fbbf8cb20420764652e34c7 
  src/personactionsmodel.cpp 458006213442bcd24bb08216594aed1deb7fb171 
  src/persondata.h 177f2451c5c432d3cb6add454716bd292540bb7f 
  src/persondata.cpp 2341bd4f4215bcc2a671f75a118c488e870c98ac 
  src/personitem.h 91fcd3cc6558625622d17b02805cc4f04382c51d 
  src/personitem.cpp 6086e41afbd028d830c6a3771dad9fd69d3627cc 
  src/personpluginmanager.h 38f13816485e69a4a2acab45b73c978d6448be1a 
  src/personpluginmanager.cpp 487f1bbde1cf122b4c475d8a33bc7514bc9bcc43 
  src/personsmodel.h 4b6a829d5c3b62e7f057fd15c821460b1e8c5df3 
  src/personsmodel.cpp 813d0afd6add7d8c33a9d08729ce9c5acadc421f 
  src/plugins/core/emailplugin.h ebffe6451b0a8cd74d934227e1f771643acab402 
  src/plugins/core/emailplugin.cpp 83babd4aa71627d6172794a6878a15b0738ad15e 
  src/resourcewatcherservice.h 94210ef7ac754eebd324c3d55c391378e1dc07d9 
  src/resourcewatcherservice.cpp 1342c99a597a3203c17aeb187ec2740af1e0ed00 
  src/widgets/persondetailsview.cpp f655eb2ec0a16f00239b679263354632a6183fee 
  src/widgets/plugins/mergecontactswidget.cpp 51e11dc8d2457297336616e6ee211493b0f5fa08 

Diff: http://git.reviewboard.kde.org/r/111308/diff/


Testing
-------


Thanks,

David Edmundson

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-telepathy/attachments/20130630/f5560f2a/attachment.html>