cozy / cozy-data-system

Deprecated - Data Layer for Cozy V2 Platform
https://cozy.io
GNU Affero General Public License v3.0
24 stars 21 forks source link

Add sharing support #214

Closed paultranvan closed 8 years ago

paultranvan commented 8 years ago

Implements sharing capabilities for the data-system, as described here.

The logic is split in 2 files :

Note this code can only work if the proxy and home are able to receive and interpret sharing requests. But this will come later :)

nono commented 8 years ago

Can you add some unit tests?

paultranvan commented 8 years ago

@nono, yep, @Ljinod is on it :)

Le 14/03/2016 13:47, Bruno Michel a écrit :

Can you add some unit tests?

— Reply to this email directly or view it on GitHub https://github.com/cozy/cozy-data-system/pull/214#issuecomment-196295468.

frankrousseau commented 8 years ago

@Gara64 @nono Can we merge that one?

nono commented 8 years ago

@frankrousseau nope, it's not ready

@Gara64 can you look at linting? Travis builds show some errors with it

@Ljinod do you need some help for the unit tests?

Ljinod commented 8 years ago

@nono I'm okay for now and I should have the tests for controllers/sharing soon

paultranvan commented 8 years ago

@nono I fixed the linting and added @Ljinod's tests units. I still need to fix 3 tests that failed though.

nono commented 8 years ago

@Gara64 Good news!

paultranvan commented 8 years ago

@nono Seems ok now :)

nono commented 8 years ago

@frankrousseau do you want to review this PR too?

aenario commented 8 years ago

A few remarks :

The unit approach makes for great speed, but the http approach ensure everything works (for instance here, you dont test the correct ID is passed to db.delete, only that db.delete has been called).

Not sure which approach is best, may be we neeed both, but it's the reason you have to stub a lot of things with all the XXX Ugly parts (which could be replaced by https://github.com/howardabrams/node-mocks-http#usage)

frankrousseau commented 8 years ago

Yep, I would like to review this one before merging.

paultranvan commented 8 years ago

@aenario About your three first remarks : I agree and I changed accordingly. I let @Ljinod answers for the unit tests.

frankrousseau commented 8 years ago

I made some comments about code styles. After that I vote for a merge of this PR.