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 activity logging for favorites in dav #48612

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

grnd-alt
Copy link
Member

@grnd-alt grnd-alt commented Oct 8, 2024

Summary

Dav api did not log favorite added/removed events.

Checklist

@grnd-alt grnd-alt force-pushed the fix/activity-log-for-favorites-in-dav branch from cde368c to e747f11 Compare October 8, 2024 10:56
@szaimen szaimen requested review from artonge and removed request for szaimen October 8, 2024 11:32
@szaimen szaimen added enhancement 3. to review Waiting for reviews labels Oct 8, 2024
@szaimen szaimen added this to the Nextcloud 31 milestone Oct 8, 2024
@grnd-alt grnd-alt force-pushed the fix/activity-log-for-favorites-in-dav branch 2 times, most recently from 19fea9b to 3a2b521 Compare October 8, 2024 12:52
Signed-off-by: grnd-alt <salimbelakkaf@outlook.de>
@grnd-alt grnd-alt force-pushed the fix/activity-log-for-favorites-in-dav branch from 3a2b521 to fbd6419 Compare October 8, 2024 12:55
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

Would it not be cleaner and make more sense to emit just the event and handle the activity in an event listener?

Then only one place for the activity is required and one would not mix different apps (files app activity in dav app).

Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

I don't get why this is need. Isn't TagService already doing that?

protected function addActivity($addToFavorite, $fileId, $path) {
$user = $this->userSession->getUser();
if (!$user instanceof IUser) {
return;
}
if ($addToFavorite) {
$event = new NodeAddedToFavorite($user, $fileId, $path);
} else {
$event = new NodeRemovedFromFavorite($user, $fileId, $path);
}
$this->dispatcher->dispatchTyped($event);
$event = $this->activityManager->generateEvent();
try {
$event->setApp('files')
->setObject('files', $fileId, $path)
->setType('favorite')
->setAuthor($user->getUID())
->setAffectedUser($user->getUID())
->setTimestamp(time())
->setSubject(
$addToFavorite ? FavoriteProvider::SUBJECT_ADDED : FavoriteProvider::SUBJECT_REMOVED,
['id' => $fileId, 'path' => $path]
);
$this->activityManager->publish($event);
} catch (\InvalidArgumentException $e) {
} catch (\BadMethodCallException $e) {
}
}
}

EDIT: reading the issue

@@ -277,9 +280,11 @@ public function __construct(IRequest $request, string $baseUri) {
$this->server->addPlugin(
new QuotaPlugin($view));
}

// if (!$user instanceOf IUser )
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be removed?

Comment on lines 288 to 294
if ((int)$favState === 1 || $favState === 'true') {
$this->addActivity(true, $node->getId(), $path);
$this->getTagger()->tagAs($node->getId(), self::TAG_FAVORITE);
} else {
$this->addActivity(false, $node->getId(), $path);
$this->getTagger()->unTag($node->getId(), self::TAG_FAVORITE);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe use those methods instead:

server/lib/private/Tags.php

Lines 481 to 497 in ff9ea47

public function addToFavorites($objid) {
if (!$this->userHasTag(ITags::TAG_FAVORITE, $this->user)) {
$this->add(ITags::TAG_FAVORITE);
}
return $this->tagAs($objid, ITags::TAG_FAVORITE);
}
/**
* Remove an object from favorites
*
* @param int $objid The id of the object
* @return boolean
*/
public function removeFromFavorites($objid) {
return $this->unTag($objid, ITags::TAG_FAVORITE);
}

And then emit the event in those methods?

Copy link
Contributor

Choose a reason for hiding this comment

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

  • addActivity from TagService can be copied to Tags.
  • Then TagService can be deleted.
  • And then
    $this->tagService->updateFileTags($path, $tags);
    } catch (\OCP\Files\NotFoundException $e) {
    can be ported to use the methods mentioned above.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review Waiting for reviews enhancement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Missing activity for favorites via WebDAV
4 participants