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 a function to get the unread count for multiple objects in one go #23792

Merged
merged 2 commits into from
Nov 4, 2020

Conversation

nickvergessen
Copy link
Member

Signed-off-by: Joas Schilling coding@schilljs.com

@rullzer
Copy link
Member

rullzer commented Nov 2, 2020

  1. Create a file foo
  2. Share foo with user1
  3. Leave a comment on foo
  4. Run this query (so find out the fileid of foo)

Don't get back the expected results...

@nickvergessen
Copy link
Member Author

Just fixed that in the commit before

@rullzer rullzer force-pushed the techdebt/noid/group-unread-comment-count-query branch from 0bf1131 to 0d8d2c9 Compare November 2, 2020 20:12
@rullzer
Copy link
Member

rullzer commented Nov 2, 2020

Just fixed that in the commit before

I tested that
But a5d6f1d ;) The orX was closed to soon

@rullzer
Copy link
Member

rullzer commented Nov 2, 2020

All working now.
Should be a lot faster!

@rullzer rullzer added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Nov 2, 2020
@rullzer rullzer marked this pull request as ready for review November 2, 2020 20:13
@MorrisJobke
Copy link
Member

With this and master I get the exact same 2 times 10 requests (10 against oc_comments and 10 against oc_comments_read_markers) 😢

I basically shared 10 files and left a comment on each of them. Then did a PROPFIND.

@MorrisJobke
Copy link
Member

With this and master I get the exact same 2 times 10 requests (10 against oc_comments and 10 against oc_comments_read_markers) 😢

I basically shared 10 files and left a comment on each of them. Then did a PROPFIND.

Just checked again and it works 👍

Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Tested and works

Signed-off-by: Joas Schilling <coding@schilljs.com>
@rullzer rullzer force-pushed the techdebt/noid/group-unread-comment-count-query branch from 6a94fae to 533a14d Compare November 4, 2020 10:01
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@rullzer rullzer force-pushed the techdebt/noid/group-unread-comment-count-query branch from 533a14d to 8bd39fc Compare November 4, 2020 15:37
@MorrisJobke
Copy link
Member

I don't know why Psalm triggers this only in that PR here:

/home/runner/work/server/server/lib/private/Comments/Manager.php:778:50:error - InvalidArgument: Argument 4 of OCP\DB\QueryBuilder\IQueryBuilder::leftJoin expects null|string, OCP\DB\QueryBuilder\ICompositeExpression provided

But other code in the same file also use it and thus I would see it as unrelated and needs to be fixed in a separate PR.

cc @rullzer @kesselb

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