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 contacts menu integration #173

Merged
merged 9 commits into from
Apr 25, 2017
Merged

Conversation

ChristophWurst
Copy link
Member

@ChristophWurst ChristophWurst commented Mar 20, 2017

WIP - requires nextcloud/server#3233

TODO

Signed-off-by: Christoph Wurst <[email protected]>
@ChristophWurst ChristophWurst added 2. developing Work in progress enhancement New feature or request labels Mar 20, 2017
@codecov-io
Copy link

codecov-io commented Mar 20, 2017

Codecov Report

Merging #173 into master will decrease coverage by 0.41%.
The diff coverage is 33.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #173      +/-   ##
==========================================
- Coverage      16%   15.58%   -0.42%     
==========================================
  Files          50       52       +2     
  Lines        1050     1142      +92     
==========================================
+ Hits          168      178      +10     
- Misses        882      964      +82
Impacted Files Coverage Δ
...s/components/contactList/contactList_controller.js 1.76% <0%> (+0.55%) ⬆️
js/main.js 83.33% <50%> (-16.67%) ⬇️
js/services/contact_service.js 0.74% <0%> (-0.27%) ⬇️
...s/components/addressBook/addressBook_controller.js 2.12% <0%> (-0.15%) ⬇️
...onents/contactDetails/contactDetails_controller.js 2% <0%> (ø) ⬆️
js/components/groupList/groupList_controller.js 6.66% <0%> (ø) ⬆️
js/filters/counterFormatter_filter.js 100% <0%> (ø)
js/components/select_directive.js 11.11% <0%> (ø)
js/models/contact_model.js 41% <0%> (+0.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 186eb31...737d63f. Read the comment docs.

@jancborchardt jancborchardt force-pushed the contactsmenu-integration branch from 3fd93f2 to bbb0fcd Compare March 29, 2017 14:21
@jancborchardt
Copy link
Member

@ChristophWurst is this already up for first review or too much in-development state? :)

@jancborchardt jancborchardt added the high High priority label Mar 29, 2017
@jancborchardt jancborchardt added this to the 1.5.4 milestone Mar 29, 2017
@ChristophWurst
Copy link
Member Author

still WIP

@ChristophWurst ChristophWurst force-pushed the contactsmenu-integration branch from bbb0fcd to 7df5447 Compare April 3, 2017 16:06
@ChristophWurst ChristophWurst mentioned this pull request Apr 3, 2017
8 tasks
@jancborchardt
Copy link
Member

jancborchardt commented Apr 7, 2017

A few bugs so far:

  • The list in the Contacts menu shouldn’t show yourself at all. nextcloud/server@cc3edca
  • Neither the »i« Details icon for internal users. :) (At least not yet where we don’t have a detail page for them.) 07f1bb1
  • When clicking on a details icon for a contact, it opens the Contacts app and loads the first contact as usually when opening the app. It should load that specific contact though.

cc @nextcloud/contacts help appreciated. :)

@nickvergessen nickvergessen removed their request for review April 10, 2017 07:40
@ChristophWurst
Copy link
Member Author

Hide other contacts, e.g. from the system address book nextcloud/server#3233 (comment)

@nextcloud/contacts any idea how I can distinguish between vcards from the system address book and the user address book? The data structures looks similar unfortunately, there's nothing specific to contacts from the contacts app it seems 🤔

@nickvergessen
Copy link
Member

If you have access to the URI, it should contain system/system

@ChristophWurst
Copy link
Member Author

If you have access to the URI, it should contain system/system

Thanks. Apparently the URI for system contacts is their UID. However, I've found what I was looking for: isLocalSystemBook is set for system contacts 😂

Signed-off-by: Christoph Wurst <[email protected]>
Signed-off-by: Christoph Wurst <[email protected]>
@@ -60,6 +57,11 @@ public function process(IEntry $entry) {
return;
}

if ($entry->getProperty('isLocalSystemBook') === true) {
// Cannot show details -> ignore
return;
Copy link
Member

Choose a reason for hiding this comment

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

Will this still show their email address though? Because that can be set in the personal details. :) The only thing which should be hidden is the details icon, not the whole entry.

Copy link
Member Author

Choose a reason for hiding this comment

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

email actions are added by the server, also for internal users

Copy link
Member Author

Choose a reason for hiding this comment

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

it doesn't hide the entry. it just doesn't add any action 😉

@ChristophWurst
Copy link
Member Author

When clicking on a details icon for a contact, it opens the Contacts app and loads the first contact as usually when opening the app. It should load that specific contact though.

@skjnldsv you mentioned that it's easy to add that. Could you please tell me where I have to adjust the code? I presume it starts with a dedicated route in main.js. But how do I get the gid of that contacts to be able to redirect to /gid/uid? Is that even possible?

Signed-off-by: Christoph Wurst <[email protected]>
Signed-off-by: Christoph Wurst <[email protected]>
@ChristophWurst
Copy link
Member Author

@skjnldsv you mentioned that it's easy to add that. Could you please tell me where I have to adjust the code? I presume it starts with a dedicated route in main.js. But how do I get the gid of that contacts to be able to redirect to /gid/uid? Is that even possible?

Kinda figured out how to possibly do that: 026d509
Still, that route doesn't seem to work when I'm already in the contacts app. It then always shows the first entry.
cc @nextcloud/contacts

@irgendwie
Copy link
Member

irgendwie commented Apr 24, 2017

@ChristophWurst found the reason (https://github.com/nextcloud/contacts/blob/contactsmenu-integration/js/components/contactList/contactList_controller.js#L142) and fixed it with a commit.

Will make some changes to the url design in a follow-up pr ref. owncloud/contacts#45 (comment)

@ChristophWurst
Copy link
Member Author

@irgendwie thanks a lot \o/

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Can’t see any immediate issues 👍

@ChristophWurst ChristophWurst added 3. to review Waiting for reviews and removed 2. developing Work in progress labels Apr 25, 2017
@jancborchardt jancborchardt merged commit 482d57a into master Apr 25, 2017
@jancborchardt jancborchardt deleted the contactsmenu-integration branch April 25, 2017 23:31
@skjnldsv skjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Aug 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
4. to release Ready to be released and/or waiting for tests to finish enhancement New feature or request high High priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants