D12837: Split replaceAccount from addAccountToCache

This revision was automatically updated to reflect the committed changes. Closed by commit R128:ff88e24e4380: Split replaceAccount from addAccountToCache (authored by valeriymalov). REPOSITORY R128 User Manager CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D12837?vs=34027&id=34304 R

D12837: Split replaceAccount from addAccountToCache

ngraham added a comment. The next stable release is 5.12.6, which is scheduled for 2018-06-27 (actually after 5.13!). So I think you should be fine. https://community.kde.org/Schedules/Plasma_5 REPOSITORY R128 User Manager BRANCH master REVISION DETAIL https://phabricator.kde.org

D12837: Split replaceAccount from addAccountToCache

valeriymalov added a comment. I meant it more in terms of "not landing it right before next stable release" :v But if it's okay to land now I can land it right now REPOSITORY R128 User Manager BRANCH master REVISION DETAIL https://phabricator.kde.org/D12837 To: valeriymalov, #plasma,

D12837: Split replaceAccount from addAccountToCache

ngraham added a comment. In D12837#263632 , @valeriymalov wrote: > In D12837#262817 , @ngraham wrote: > > > Any chance we could land this on the stable branch? > > > Well, it does apply cleanl

D12837: Split replaceAccount from addAccountToCache

valeriymalov added a comment. In D12837#262817 , @ngraham wrote: > Any chance we could land this on the stable branch? Well, it does apply cleanly to Plasma/5.12 branch and it still works, but I don't really know if there's a special proc

D12837: Split replaceAccount from addAccountToCache

ngraham added a comment. Any chance we could land this on the stable branch? REPOSITORY R128 User Manager BRANCH master REVISION DETAIL https://phabricator.kde.org/D12837 To: valeriymalov, #plasma, davidedmundson Cc: davidedmundson, ngraham, rdieter, plasma-devel, ragreen, Pitel, Zren

D12837: Split replaceAccount from addAccountToCache

valeriymalov added inline comments. INLINE COMMENTS > davidedmundson wrote in accountmodel.cpp:361 > On second look, I might have misunderstood. > > This isn't a replace, it's just making sure the current user is at the top? Yes. Replace code was added to `addAccountToCache` at some point becau

D12837: Split replaceAccount from addAccountToCache

davidedmundson added inline comments. INLINE COMMENTS > accountmodel.cpp:361 > if (uid == getuid()) { > addAccountToCache(path, acc, 0); > return; On second look, I might have misunderstood. This isn't a replace, it's just making sure the current user is at the top? REPO

D12837: Split replaceAccount from addAccountToCache

valeriymalov added a comment. > We have addAccountToCache which has an argument pos, which determines if we're adding or replacing. I've actually removed this bit, addAccountToCache now only adds accounts (either inserts at pos or appends) Though I agree that current code is probabl

D12837: Split replaceAccount from addAccountToCache

davidedmundson requested changes to this revision. davidedmundson added a comment. This revision now requires changes to proceed. It might work, but it's overly messy. We have addAccountToCache which has an argument pos, which determines if we're adding or replacing. Then we have a s

D12837: Split replaceAccount from addAccountToCache

ngraham added a comment. Bulletproof steps to reproduce for me: 1. System Settings > Account Details > User Manager > Create a new user 2. Give the new user admin rights 3. Log out 4. Log in as that the new user 5. Go to System Settings > Account Details > User Manager The use

D12837: Split replaceAccount from addAccountToCache

valeriymalov created this revision. valeriymalov added a reviewer: Plasma. Restricted Application added a project: Plasma. Restricted Application added a subscriber: plasma-devel. valeriymalov requested review of this revision. REVISION SUMMARY We were accidentally overwriting first account in t