fix change permission on mail shares of folders

[blizzz]

1. Create a mail share on a folder
2. Change "can edit"

As found in #2398 (comment)

Please test and review @schiessle @rullzer @ChristophWurst

[mention-bot]

@blizzz, thanks for your PR! By analyzing the history of the files in this pull request, we identified @rullzer, @ChristophWurst and @schiessle to be potential reviewers.

[schiessle]

button works again but the permissions for "can edit" are wrong, they should be 15 (create, read, update, delete)

[blizzz]

@schiessle please check again. Could there be any corner cases were we should not set it to 15?

[rullzer]

Yes for files. Files dont' get DELETE or CREATE permissions

[blizzz]

@schiessle @rullzer should be settled now I hope.

[schiessle]

works great! 👍

[skjnldsv]

👍

[rullzer]

Why do we do this? Any reason why we don't have the regual checkboxes for mail shares as well (create/update/delete/share) ???

[schiessle]

Why do we do this? Any reason why we don't have the regual checkboxes for mail shares as well (create/update/delete?) ???

Because at the end it is a link share and the public link view can't really distinguish between this permissions. So we chose to use the same permissions we have for public links (either 1 or 15), at least for now.

[schiessle]

@blizzz js unit tests fail, can you have a look? Thanks!

[blizzz]

👀

[blizzz]

PhantomJS 2.1.1 (Linux 0.0.0) ERROR
  TypeError: undefined is not a function (evaluating 'this.data.state()')
  at apps/files/js/file-upload.js:157
PhantomJS 2.1.1 (Linux 0.0.0): Executed 835 of 837 (skipped 2) ERROR (18.498 secs / 17.433 secs)
PhantomJS 2.1.1 (Linux 0.0.0) ERROR
  TypeError: undefined is not a function (evaluating 'this.data.state()')
at apps/files/js/file-upload.js:157

how is that related? the output is as useful as a matchstick in the ocean.

[schiessle]

how is that related? the output is as useful as a matchstick in the ocean.

good question. I also can't reproduce it locally. Maybe just a rebase and let Drone try it once more? 😉 (can't judge if this is critical or not)

[blizzz]

I restarted the tests, but it fails on the same place.

[blizzz]

And no related changes in master since.

[blizzz]

On another this test succeeds, so it has something to do with the changes, somewhow. A stacktrace could be helpful, but somehow the suite does not print it out?

I get the same error. autotest-js.sh is your friend. Investigating.

[blizzz]

I get the same error. Investigating.

… some fruitless attempts to get the stack trace later tests succeed without further changes. 🏴

 PhantomJS 2.1.1 (Linux 0.0.0): Executed 835 of 837 (skipped 2) SUCCESS (25.776 secs / 24.039 secs)
root@jessica:/drone/src/github.com/nextcloud/server# git status
HEAD detached at FETCH_HEAD
nothing to commit, working directory clean
root@jessica:/drone/src/github.com/nextcloud/server# git log
commit b9bc708ffad5e940a09aaef4de922bf252913047
Author: blizzz <[email protected]>
Date:   Tue Dec 6 16:10:06 2016 +0000

    Merge c2062a93a3eb57c01609a89775c8551b92cc9013 into 0b0f27358f67382f4c45f469db50de8f17f72bce

Cannot restart from the UI anymore. So, merging master pushing again.

[codecov-io]

Current coverage is 57.21% (diff: 100%)

Merging #2530 into master will decrease coverage by <.01%

@@             master      #2530   diff @@
==========================================
  Files          1202       1202          
  Lines         72470      72477     +7   
  Methods        7365       7365          
  Messages          0          0          
  Branches       1228       1229     +1   
==========================================
+ Hits          41463      41465     +2   
- Misses        31007      31012     +5   
  Partials          0          0          

Powered by Codecov. Last update b190153...7e2b866

[blizzz]

Test succeeded now without actual changes 🃏 waiting for the rest and then 🚢

[blizzz]

LGTM not working somehow, but approvals from @schiessle and @skjnldsv (#2530 (comment) && #2530 (comment)) thus merging.