Shared albums should not consider the require_link property.

[ildyria]

Fixes #1478

/**
 *  - the user is the admin, or
 *  - the user is the owner, or
 *  - the album does not require a direct link and is shared with the user, or
 *  - the album does not require a direct link, is public and has no password set, or
 *  - the album does not require a direct link, is public and has been unlocked
 */

After further thoughts, this is pretty counter intuitive. We should have fixed that one a long time ago. :)

[ildyria]

I am not sure if I missed some other conditions.

[codecov]

Codecov Report

Merging #1480 (aa23932) into master (e18c16b) will decrease coverage by 0.76%.
The diff coverage is 100.00%.

[nagmat84]

You missed the conditions in line 82 and 360.

Moreover, I am in strong favor of tests which checks the new behavior. At the moment, we seem to have reached a point which is rather bug-free with respect to security issues (i.e. accidentally leaking a photo which should stay hidden). Any change in the SQL queries might break that and hence, tests would be wonderful.

Or we could just wait until #1462 is ready. As I detailed out in #1478 there might actually be other users who rely on exactly this feature, i.e. that albums can also be hidden from users with whom they are shared. (Although, I lack the imagination of a meaningful use-case for that.)

I am still wondering if we should make "hidden" a per-share option. @kamil4 raised the same question in #1357 (comment), 3rd bullet point. This being said, I am not sure, if we should change the behavior now or simply wait until #1462. At least, the implementation works as designed and intended.

[ildyria]

As per requested finished the PR changes and added tests. :)

[ildyria]

IDK what happened O.o but this is definitively not something I wanted to close. :'D

[nagmat84]

LGTM, but untested. I only checked all the boolean conditions in the query (which was difficult enough 😆) and could not spot any remaining error.

@ildyria I assume you have tested it yourself and found no regression?

[ildyria]

@ildyria I assume you have tested it yourself and found no regression?

Yes.

[ildyria]

Also the tests you added are making it pretty tight to create regressions :)