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.

Sign up for GitHub

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

Jump to bottom

Retrieving shared contacts/users from database to dropdown menu #14731

Closed
wants to merge 31 commits into from
Closed

Retrieving shared contacts/users from database to dropdown menu #14731

wants to merge 31 commits into from

Conversation

sananirajabov
Copy link

The problem is when you click users dropdown menu, first 25 users appear, to fix this problem I implemented code when you share something with user, this user then appear in dropdown menu, and there is no limit how many users will appear.

fix #5097

Signed-off-by: Sanani Rajabov [email protected]

@MorrisJobke MorrisJobke added this to the Nextcloud 17 milestone Mar 20, 2019
ChristophWurst
ChristophWurst requested changes Mar 20, 2019
View reviewed changes
Copy link
Member

@ChristophWurst ChristophWurst left a comment

Choose a reason for hiding this comment

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

lib/private/Contacts/ContactsMenu/ContactsStore.php 100644 → 100755

please revert the permission changes

@ChristophWurst
Copy link
Member

I think the contacts from shares should be extracted to a new class

@sananirajabov

This comment has been minimized.

Sign in to view
@sananirajabov

This comment has been minimized.

Sign in to view
@sananirajabov
Copy link
Author

Now I think it works well, bring shared contacts to dropdown menu and when search happens it search in whole database.

@sananirajabov

This comment has been minimized.

Sign in to view
ChristophWurst
ChristophWurst requested changes Apr 11, 2019
View reviewed changes
lib/private/Contacts/ContactsMenu/ContactsStore.php Outdated
$queryBuilder = $this->conn->getQueryBuilder();

// Getting shared users
$sharedContactsQuery = $queryBuilder->selectDistinct("share_with")
Copy link
Member

Choose a reason for hiding this comment

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

this, in combination with the expensive iteration below won't scale for larger instances with thousands of shares. there should be a limit

Copy link
Author

Choose a reason for hiding this comment

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

Yes you are right, Actually I had not thought about it. How about limit to 100 shared users? is it ok?

Copy link
Member

Choose a reason for hiding this comment

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

Sounds good!

Copy link
Author

Choose a reason for hiding this comment

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

I had to limit users to 25 entries because in the same directory Manager class limit users to 25 entries,

$topEntries = array_slice($sortedEntries, 0, 25);

I did not want to change to 100.

kesselb and others added 11 commits April 17, 2019 16:20
@kesselb @sananirajabov
Extend regex to match DAVx5
66a61ca 
https://gitlab.com/bitfireAT/davx5-ose/commit/7ce739c271f9b4f9b3cd7c16c526a774a7d99bdc
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: sananirajabov <[email protected]>
@kesselb @sananirajabov
Clipboard -> ClipboardJS
9c9688c 
#12915
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: sananirajabov <[email protected]>
@kesselb @sananirajabov
Make app password description editable
f75239f 
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: sananirajabov <[email protected]>
@kesselb @sananirajabov
Add canRename & name to tests
3f5cc87 
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: sananirajabov <[email protected]>
@kesselb @sananirajabov
Add interface INamedToken
ebc6ccd 
Remove $token instanceof DefaultToken || $token instanceof PublicKeyToken

Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: sananirajabov <[email protected]>
@kesselb @sananirajabov
Improve usability
4d38e37 
- Trigger rename by doubleclick on token name
- Use action rename to open and close
- Line height stays the same

Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: sananirajabov <[email protected]>
@kesselb @sananirajabov
Don't update token if current session token
43cba67 
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: sananirajabov <[email protected]>
@kesselb @sananirajabov
Mock name for integration test
e41074b 
Signed-off-by: Daniel Kesselberg <[email protected]>
Signed-off-by: sananirajabov <[email protected]>
@nickvergessen @sananirajabov
Use the new method instead of the deprecated wrapper
53e31c6 
Signed-off-by: Joas Schilling <[email protected]>
Signed-off-by: sananirajabov <[email protected]>
@nextcloud-bot @sananirajabov
[tx-robot] updated from transifex
77db0cc 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
Show shared users on first click to dropdown and show searched users …
7b3f789 
…when user typed name

Signed-off-by: sananirajabov <[email protected]>
sananirajabov and others added 10 commits April 17, 2019 16:20
@sananirajabov
Remove whitespace at line 306
7b5e439 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
Remove whitespaces
51dfc24 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
Permission 755 to 644
cce883b 
Signed-off-by: sananirajabov <[email protected]>
@ChristophWurst @sananirajabov
Update lib/private/Contacts/ContactsMenu/ContactsStore.php
320687b 
Co-Authored-By: sananirajabov <[email protected]>
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
Added IDBConnection to constructor
a09df14 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
Getting QueryBuilder from connection
f88f6db 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
imported OCP/IDBConnection
423b727 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
Limit shared users to 25 entries
5c81610 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
Revert "imported OCP/IDBConnection"
58278fc 
This reverts commit 423b727.

Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
conflicted
fe9869e 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
Copy link
Author

sananirajabov commented Apr 17, 2019 •
edited
Loading

Hi, I had DCO problems, and when entered details to look what is the problem I saw it is sign off problem and also offered two lines git command and I applied then DCO got successful but appeared some problem and I do not know how to solve these problems and conflicts, it shows I signed off someone's commit but I do not know how it happened. If it is possible could you help me? Or it is good idea close this pull request and create new one?

@kesselb
Copy link
Contributor

kesselb commented Apr 17, 2019 •
edited
Loading

Rebase your branch with nextcloud/server:master

image

I usually do it with phpstorm like this. origin is https://github.com/nextcloud/server (because i have write permission to nextcloud/server). You might add a new remote to your repository "upstream" which points to https://github.com/nextcloud/server and rebase onto refs/remotes/upstream/master

@sananirajabov
Copy link
Author

Rebase your branch with nextcloud/server:master

image

I usually do it with phpstorm like this. origin is https://github.com/nextcloud/server (because i have write permission to nextcloud/server). You might add a new remote to your repository "upstream" which points to https://github.com/nextcloud/server and rebase onto refs/remotes/upstream/master

Thanks a lot, I solved problem.

@sananirajabov
Contact Store after merge branches
4a152a8 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
ContactsStore
481af60 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
revert to 644
571cb82 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov
revert to 644
9889be0 
Signed-off-by: sananirajabov <[email protected]>
@sananirajabov sananirajabov reopened this Apr 28, 2019
@MorrisJobke MorrisJobke mentioned this pull request Jul 15, 2019
Merged
28 tasks
This was referenced Aug 18, 2019
Merged
Merged
@rullzer rullzer mentioned this pull request Aug 29, 2019
Merged
16 tasks
@gary-kim
Copy link
Member

gary-kim commented Sep 1, 2019

@sananirajabov Are you still working on this?

@sananirajabov
Copy link
Author

@sananirajabov Are you still working on this?

No, I am not working on this issue. I fixed issue but now it has performance issue when there are lots of records in database.

@rullzer rullzer modified the milestones: Nextcloud 17, Nextcloud 18 Sep 5, 2019
@rullzer
Copy link
Member

rullzer commented Sep 5, 2019

Master is no 18 development. If this needs to go into 17 please follow the normal backport procedures after merging.

@rullzer rullzer mentioned this pull request Dec 11, 2019
Merged
43 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ChristophWurst ChristophWurst ChristophWurst requested changes

@rullzer rullzer Awaiting requested review from rullzer

Assignees
No one assigned
Labels
3. to review Waiting for reviews enhancement feature: contacts menu
Projects
None yet
Milestone
Nextcloud 18.0.0
Development

Successfully merging this pull request may close these issues.

scope of the contacts menu is too broad
8 participants
@sananirajabov @ChristophWurst @kesselb @gary-kim @rullzer @MorrisJobke @nickvergessen @nextcloud-bot