Group folders fixes, tests, docs

[danil-topchiy]

Based on #9

[matejak]

please rebase against master.

[danil-topchiy]

@matejak rebased. After rebasing noticed one issue here, groupfolders app not listed in get_apps by default and I couldn't enable it without downloading and installing from nextcloud web interface, but web uses client internal api call: /settings/apps/enable.

Do you know any better way how to enable that app, except volume folder with all apps to docker ? May be somehow in docker-compose or by command, couldn't find such way (

[matejak]

I see, the Nextcloud Docker setup doesn't allow some form of on-demand installation and enablement of apps. We can extend our Nextcloud images with a more capable entrypoint script that would manage to do that and upstream that later. Sounds intriguing, I will take a look at that.
In the meantime, I think that sharing a persistent data folder as you have described is the correct way of setting tests up.

[danil-topchiy]

@matej Anyway I had to extend Nextcloud image because couldn't change directory owners with just volume.
I did it as they said in their docker docs:

Copy only the custom apps you use (or simply redownload them from the web interface):
docker cp ./apps/ nextcloud_data:/var/www/html/custom_apps
docker-compose exec app chown -R www-data:www-data /var/www/html/custom_apps

Although have new problem here.
So I see this app is installed, but disabled, in web interface and in list of apps. When I'm trying to enable this app using ocs api like this self.nxc.enable_app('groupfolders') everything looks fine and I see this app in list of enabled apps, but when I'm doing any request to it's api database logger throws exception:

app_1         | 192.168.0.1 - admin [27/Dec/2018:22:44:34 +0000] "POST /ocs/v1.php/cloud/apps/groupfolders?format=json HTTP/1.1" 200 1375 "-" "python-requests/2.20.1"
db_1          | ERROR:  relation "group_folders" does not exist at character 257
db_1          | STATEMENT:  SELECT "f"."folder_id", "mount_point", "quota", "fileid", "storage", "path", "name", "mimetype", "mimepart", "size", "mtime", "storage_mtime", "etag", "encrypted", "parent", "a"."permissions" AS "group_permissions", "c"."permissions" AS "permissions" FROM "group_folders" "f" INNER JOIN "group_folders_groups" "a" ON "f"."folder_id" = "a"."folder_id" LEFT JOIN "filecache" "c" ON ("storage" = $1) AND ("path_hash" = MD5(($2 || "f"."folder_id"))) WHERE "a"."group_id" = $3

and this app becomes disabled again.
Although if I'm enabling it using web interface - everything is fine, I can disable/enable it using OCS api after and everything works as expected.

[danil-topchiy]

I suppose when you enable it using web interface, this new relation is created (as well as some other db migrations probably), and when you trying to do this using ocs api endpoint - it doesn't happen and only turns app on/off.
Cannot find a way how to get around it yet.

[danil-topchiy]

So enabling an app works using occ tool before running tests:
docker-compose exec --user www-data app php occ app:enable groupfolders

[matejak]

Well, we have to find another way to make this working than adding the whole directory into the repository. Not only it is a huge amount of files, but they can change any time, introducing noise.
I think that the solution is to expand the Nextcloud image's /entrypoint.sh script by an ability to install and enable apps according to some env variables, or running the command you have posted in a comment before executing tests.
Please remove those data files, amend your commit and force push, because it makes any review very difficult.

[danil-topchiy]

@matejak agree, done.

I decided that it will be easier than override entrypoint.sh, to just add downloading of custom app to dockerfile and than copy and enable it before running tests using docker-compose exec:
docker-compose exec --user www-data app /bin/bash -c "cp -R /tmp/groupfolders /var/www/html/custom_apps/groupfolders && php occ app:enable groupfolders"

[matejak]

Thank you for your pull request, I have done a dirve-by review and it looks good, except a small nitpick.
However, I was not successful in running the tests for group folders, so please double-check that, and while you are at it, also add a simple README.md to the tests folder, so it is clear what to do in order to execute tests and what is going on in the process.
Good work and happy new year!

[danil-topchiy]

@matejak thanks for review!
I think tests failed because you didn't manually enable groupfolders app before running them:

    docker-compose exec --user www-data app /bin/bash -c \
    "cp -R /tmp/groupfolders /var/www/html/custom_apps/groupfolders && php occ app:enable groupfolders"

Added it to README.md in tests as well.

Happy new year!

[matejak]

I think tests failed because you didn't manually enable groupfolders app before running them

This is not the case, I got errors like

    def test_setting_folder_quotas(self):
        # create new group folder
        folder_mount_point = "test_folder_quotas_" + self.get_random_string(length=4)
        res = self.nxc.create_group_folder(folder_mount_point)
        group_folder_id = res['ocs']['data']['id']
    
        # assert quota is unlimited by default
        res = self.nxc.get_group_folder(group_folder_id)
>       assert res['ocs']['data']['quota'] == QUOTE_UNLIMITED
E       AssertionError: assert '-3' == -3

i.e. it looks like there are string vs integer conversion issues. However, I have pulled/rebuilt all images, and it got fixed. But I think that it won't hurt to convert the actual quota to integer.

I have also noticed the QUOTE_UNLIMITED that should be QUOTA_UNLIMITED, so please rename this in the source code.
Other from that, the PR looks good, so after you address these concerns, it is good to be merged, good work!

[scrutinizer-notifier]

The inspection completed: 9 updated code elements

[danil-topchiy]

Added converting of quota in response to integer and fixed typo in variable

[matejak]

Good job, merging!