Bug 71390 - add API for accounts that cannot be disabled
Summary: add API for accounts that cannot be disabled
Status: RESOLVED FIXED
Alias: None
Product: Telepathy
Classification: Unclassified
Component: mission-control (show other bugs)
Version: git master
Hardware: Other All
: medium enhancement
Assignee: Simon McVittie
QA Contact: Telepathy bugs list
URL:
Whiteboard: review?
Keywords: patch
Depends on:
Blocks:
 
Reported: 2013-11-08 14:17 UTC by Simon McVittie
Modified: 2013-11-13 19:39 UTC (History)
2 users (show)

See Also:
i915 platform:
i915 features:


Attachments
[1/2] fakeaccountsservice: be a bit more object-oriented (8.69 KB, patch)
2013-11-08 16:26 UTC, Simon McVittie
Details | Splinter Review
[2/2] Replace McdAccount::always-on with the existing TpStorageRestrictionFlags (26.11 KB, patch)
2013-11-08 16:26 UTC, Simon McVittie
Details | Splinter Review
[3/3] Allow backends, but not D-Bus clients, to delete restricted accounts (7.66 KB, patch)
2013-11-08 16:56 UTC, Simon McVittie
Details | Splinter Review
Fix addition of restrictions to test accounts (942 bytes, patch)
2013-11-13 17:53 UTC, Simon McVittie
Details | Splinter Review

Description Simon McVittie 2013-11-08 14:17:55 UTC
Account.I.Storage has a property for a set of restriction flags.

McdAccount::always-on allows the creation of accounts that can never go offline (ideal for telephony), but now that we've abolished Mcd* as a plugin API, there's no actual way to achieve that.

On Bug #71384 I suggested:

> >   return TP_STORAGE_RESTRICTION_FLAG_CANNOT_SET_PARAMETERS |
> >       TP_STORAGE_RESTRICTION_FLAG_CANNOT_SET_ENABLED |
> >       TP_STORAGE_RESTRICTION_FLAG_CANNOT_SET_PRESENCE |
> >       TP_STORAGE_RESTRICTION_FLAG_CANNOT_SET_SERVICE;
> 
> I think this suggests how we can supersede McdAccount::always-on:
> 
> * if we CANNOT_SET_ENABLED, then Enabled is forcibly set to what the
>   storage backend says it is (instead of TRUE, as always-on does)
> 
> * if we CANNOT_SET_PRESENCE, then AutomaticPresence and ConnectAutomatically
>   are forcibly set to what the storage backend says they are,
>   and RequestedPresence is forcibly set to
>   (ConnectAutomatically ? AutomaticPresence : (OFFLINE, "offline", ""))
Comment 1 Simon McVittie 2013-11-08 16:26:08 UTC
Created attachment 88898 [details] [review]
[1/2] fakeaccountsservice: be a bit more object-oriented
Comment 2 Simon McVittie 2013-11-08 16:26:49 UTC
Created attachment 88899 [details] [review]
[2/2] Replace McdAccount::always-on with the existing  TpStorageRestrictionFlags

I used CANNOT_SET_PRESENCE to control access to ConnectAutomatically
as well as the obvious AutomaticPresence and RequestedPresence, because
RequestedPresence is not under the storage plugin's control - it is
derived from ConnectAutomatically and AutomaticPresence at runtime.

Use MCD_DBUS_PROP_SET_FLAG_ALREADY_IN_STORAGE as a way to bypass the
storage restriction flags, so that storage plugins themselves can
alter enabledness etc. even if they don't allow MC to change it.

The regression test for this initially failed, because toggled_cb()
did not pass that flag to the McdAccount: fix that too.
Comment 3 Simon McVittie 2013-11-08 16:30:46 UTC
We should probably have a restriction flag for the ability to delete accounts, too. Not implementing that right now in case it interferes with Bug #71384, and because it requires telepathy-spec work.

At the moment, mcd_account_delete() disables the account, which means if you can't disable an account, you also can't delete it (probably good); it also means that the 'deleted' signal from the storage plugin can't delete it either (bad).
Comment 4 Simon McVittie 2013-11-08 16:56:52 UTC
Created attachment 88900 [details] [review]
[3/3] Allow backends, but not D-Bus clients, to delete  restricted accounts

I assumed that if TP_STORAGE_RESTRICTION_FLAG_CANNOT_SET_ENABLED
or TP_STORAGE_RESTRICTION_FLAG_CANNOT_SET_PRESENCE, then a hypothetical
CANNOT_DELETE flag would also have been present.
Comment 5 Xavier Claessens 2013-11-08 17:04:17 UTC
I like this, makes much more sense!

+1
Comment 6 Xavier Claessens 2013-11-08 21:56:29 UTC
Pushed those 3 patches for you to master, because I have changes depending on them.
Comment 7 Simon McVittie 2013-11-11 13:00:26 UTC
Thanks. For our future selves' benefit: fixed in git for 5.17.0
Comment 8 Simon McVittie 2013-11-13 17:53:10 UTC
Created attachment 89158 [details] [review]
Fix addition of restrictions to test accounts

---

Xavier, this might fix the test failures you were seeing?
Comment 9 Xavier Claessens 2013-11-13 19:04:07 UTC
+1
Comment 10 Simon McVittie 2013-11-13 19:39:14 UTC
Fixed again, 5.17.0. Sorry about that.

More patches to come on Bug #71390, reinstating some of the stuff I reverted.


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.