Skip to content

WIP: import contacts from remote CardDAV server - #134

Open
Fnux wants to merge 12 commits into
ZeroPhone:develfrom
Fnux:contacts-sync
Open

WIP: import contacts from remote CardDAV server#134
Fnux wants to merge 12 commits into
ZeroPhone:develfrom
Fnux:contacts-sync

Conversation

@Fnux

@Fnux Fnux commented Nov 17, 2018

Copy link
Copy Markdown
Collaborator

This PR addresses parts of #124: it alows to import contacts from a remote CardDAV server using vdirsyncer. I did not add a way to export the local contacts since the user doesn't have a form (for now) to add new entries manually.

I still have to investigate a sexier way to store the application's configuration and write tests for the new features.

@CRImier

CRImier commented Nov 17, 2018

Copy link
Copy Markdown
Member

Re "storing application's configuration" - we have solutions for working with .json config files - storing, recreating configs on error, saving, even primitive migrations. See the howtos regarding that, let me know if anything's unclear.

@codecov

codecov Bot commented Nov 17, 2018

Copy link
Copy Markdown

Codecov Report

Merging #134 into devel will increase coverage by 2.8%.
The diff coverage is 37.61%.

Impacted file tree graph

@@            Coverage Diff            @@
##            devel     #134     +/-   ##
=========================================
+ Coverage   38.51%   41.32%   +2.8%     
=========================================
  Files         233      255     +22     
  Lines       18126    20867   +2741     
=========================================
+ Hits         6982     8623   +1641     
- Misses      11144    12244   +1100
Impacted Files Coverage Δ
helpers/vdirsyncer.py 1.88% <1.88%> (ø)
helpers/xdg.py 100% <100%> (ø)
apps/personal/contacts/address_book.py 54.92% <33.33%> (-20.29%) ⬇️
apps/personal/contacts/vcard_converter.py 33.33% <50%> (-1.15%) ⬇️
apps/personal/contacts/main.py 11.49% <7.46%> (-23.93%) ⬇️
apps/personal/contacts/contact.py 83.33% <83.33%> (ø)
tests/test_drivers.py 66.22% <0%> (-24.69%) ⬇️
apps/example_apps/sandbox/main.py 40.9% <0%> (-22.73%) ⬇️
ui/refresher.py 72.61% <0%> (-8.21%) ⬇️
ui/utils.py 90.67% <0%> (-5.16%) ⬇️
... and 90 more

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 0c9b304...a180045. Read the comment docs.

@CRImier CRImier left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Reviewed the code without running it, for now. Change the i=self.i, o=self.o, Printer and Listbox stuff, and do let me know about the other changes =)

Comment thread apps/personal/contacts/main.py Outdated
Comment thread apps/personal/contacts/main.py Outdated
Comment thread apps/personal/contacts/main.py Outdated
Comment thread apps/personal/contacts/main.py Outdated
Comment thread apps/personal/contacts/main.py Outdated
Comment thread apps/personal/contacts/main.py Outdated
Comment thread apps/personal/contacts/contact.py Outdated
Comment thread apps/personal/contacts/contact.py Outdated
Comment thread apps/personal/contacts/contact.py Outdated
Comment thread apps/personal/contacts/main.py Outdated
@Fnux

Fnux commented Jan 4, 2019

Copy link
Copy Markdown
Collaborator Author

@CRImier I'd like to move the Contact and AddressBook classes under libs/ since they might be used by other apps in the future (Calls, SMS). Does it makes sense to you?

@Fnux

Fnux commented Jan 5, 2019

Copy link
Copy Markdown
Collaborator Author

I still have to clean, document and test the code I moved to lib/. I will resolve the WIP state once it is done.

@Fnux

Fnux commented Jan 6, 2019

Copy link
Copy Markdown
Collaborator Author

I added a paths helper, which I needed for the vdirsyncer and address_book modules.

from vcard_converter import VCardContactConverter
from ui import (NumberedMenu, Listbox, Menu, LoadingIndicator, DialogBox,
PrettyPrinter as Printer, UniversalInput)
from distutils.spawn import find_executable

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Unused import?

@CRImier

CRImier commented Jan 7, 2019

Copy link
Copy Markdown
Member

This works for your usecase, right?

We can use the default config management functions to store the config files, just need to point it to the right folder.

As about testing - doctests for code in libs/ pass, but you can add more; we don't yet have a good way to test apps.

@CRImier

CRImier commented Feb 4, 2019

Copy link
Copy Markdown
Member

Looks good! Will do a couple of fixes now and merge

@CRImier

CRImier commented Feb 5, 2019

Copy link
Copy Markdown
Member

Oh right, read through the code again. Thinking about some kind of changes to config management that'd avoid using the main ZPUI config. For now, I'm making sure that the ZPUI config is used the least amount possible - since it has hardware configuration parameters, it's important for it to not get changed by the users often. + I'm not sure that it makes sense for the vdirsyncer wrapper library to store the config where it currently is, that is, in libs/ - the idea about libraries in libs/ is that they can be used by more than one app, in different ways.

So, I have an idea - read the config from the app (+ allow changing it from the same app), then pass the config variables. Let me know what you think of it. For now, merging the code into a separate branch (contacts-sync) + adding you as a contributor in this repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants