Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add UserConfigChangedEvent to fire whenever a user config value is changed #42039

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

akhil1508
Copy link
Contributor


private function triggerUserValueChange($userId, $appId, $key, $value, $oldValue = null) {
if (\OC::$server instanceof \OCP\IServerContainer) {
$dispatcher = \OC::$server->get(IEventDispatcher::class);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cannot use DI for this as it is used in base.php over here

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that the right link? The Allconfig constructor does not call setUserValue.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@come-nc I believe I meant to say:

  1. AllConfig is instantiated in base.php as new \OC\AllConfig(new \OC\SystemConfig(self::$config))
  2. If I add IEventListener $eventListener in the constructor for DI in this class (the usual way), it could be problematic. I remember it causing errors when I first wrote this code, but maybe I should re-test? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes please, the event listener is available early so I would be surprised if this is called even earlier. Maybe for login time 🤔
If some value change call do not trigger the event we need to document that.

@skjnldsv skjnldsv added the 2. developing Work in progress label Feb 21, 2024
@susnux susnux added this to the Nextcloud 30 milestone Apr 18, 2024
This was referenced Jul 30, 2024
This was referenced Aug 5, 2024
@skjnldsv skjnldsv mentioned this pull request Aug 13, 2024
@skjnldsv skjnldsv removed this from the Nextcloud 30 milestone Aug 14, 2024
@akhil1508 akhil1508 force-pushed the dev/user-preference-event branch 4 times, most recently from 037ad05 to b040664 Compare September 16, 2024 12:58
@akhil1508 akhil1508 changed the title Draft: Add UserConfigChangedEvent Add UserConfigChangedEvent to fire whenever a user config value is changed Sep 16, 2024
@skjnldsv
Copy link
Member

skjnldsv commented Sep 16, 2024

Hey @akhil1508
Thanks for the rebase! Let me ask for reviews, and see how we can help you move this forward 👍

@skjnldsv skjnldsv requested review from artonge, come-nc, a team and ArtificialOwl and removed request for a team September 16, 2024 12:59
@skjnldsv skjnldsv added enhancement 3. to review Waiting for reviews feature: users and groups and removed 2. developing Work in progress labels Sep 16, 2024
@skjnldsv skjnldsv added this to the Nextcloud 31 milestone Sep 16, 2024
lib/composer/composer/autoload_static.php Outdated Show resolved Hide resolved
lib/public/User/Events/UserConfigChangedEvent.php Outdated Show resolved Hide resolved

private function triggerUserValueChange($userId, $appId, $key, $value, $oldValue = null) {
if (\OC::$server instanceof \OCP\IServerContainer) {
$dispatcher = \OC::$server->get(IEventDispatcher::class);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is that the right link? The Allconfig constructor does not call setUserValue.

@akhil1508 akhil1508 force-pushed the dev/user-preference-event branch from 0b331d0 to 57d68a3 Compare September 17, 2024 13:36
Signed-off-by: Akhil <akhil@e.email>
@akhil1508 akhil1508 force-pushed the dev/user-preference-event branch from 57d68a3 to 3040493 Compare September 17, 2024 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants