Review Request 111308: Disallow changing URI of PersonData after creation

David Edmundson david at davidedmundson.co.uk
Sat Jun 29 14:06:57 UTC 2013


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

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


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
-----

  src/autotests/tests/persondatatests.cpp b09b00fe95b2e55f0f285fb968ccf03634d0e20a 
  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 3665d319bc5e3fa100e34a3d94d7435776d99851 
  src/widgets/persondetailsview.cpp f655eb2ec0a16f00239b679263354632a6183fee 

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/20130629/045b6035/attachment.html>


More information about the KDE-Telepathy mailing list