fixed the response code when copying the file from shares to personal space

[2403905]

Related Issue #9482

[fschade]

LGTM, one thing, since there was no red ci, i expect we have not tested this code path in the past... could you add some tests for httpDownloadRes.StatusCode == http.StatusForbidden.

just in case, why do you return nil instead of the err?

THANKS 🤗

[phil-davis]

From an API test point-of-view, I think test coverage is related to issue owncloud/ocis#8428
API tests have not been added yet for this case. And I think that we have been asked not to add test scenarios that fail (for example, due to a known reported bug like this). So there won't be an API test scenario that starts passing.

To verify the fix at the API level, someone would need to add a test scenario to the ocis API tests, then make that test run here (an extra PR from here that uses the new branch in ocis as the source for the tests).

But maybe you are just referring to unit tests? @fschade I am not sure what level of test you mean.

[fschade]

From an API test point-of-view, I think test coverage is related to issue owncloud/ocis#8428 API tests have not been added yet for this case. And I think that we have been asked not to add test scenarios that fail (for example, due to a known reported bug like this). So there won't be an API test scenario that starts passing.

To verify the fix at the API level, someone would need to add a test scenario to the ocis API tests, then make that test run here (an extra PR from here that uses the new branch in ocis as the source for the tests).

But maybe you are just referring to unit tests? @fschade I am not sure what level of test you mean.

Yes exactly, not a logic test, more like a behavior test :)
thanks for the detailed explanation

[saw-jan]

We have a PR owncloud/ocis#9437 to cover the behavior but not merged because it was not fixed yet. We will test and report if the fixes worked with the new test scenarios.

[saw-jan]

Also, we do not have secure-view role test scenarios in coreApi tests (test suites running for reva). Secure view tests only runs in ocis ci because we do not run ocis api test suites (tests suites starting with api<Suitename>) in reva.

[2403905]

LGTM, one thing, since there was no red ci, i expect we have not tested this code path in the past... could you add some tests for httpDownloadRes.StatusCode == http.StatusForbidden.

just in case, why do you return nil instead of the err?

THANKS 🤗

We return the nil because the error will be handled as a 500