[FIX] Apply read permission to the list of members instead of the main option by corevibe555 · Pull Request #4809 · owncloud/android

corevibe555 · 2026-03-21T15:43:30Z

Problem

Users without the libre.graph/driveItem/permissions/read permission had the Members action hidden from the space menu. However, that screen also contains the permalink section, which became inaccessible to these users.

Solution

Always include Members in the filtered space menu options.
In SpaceMembersFragment, treat the read permission as controlling members and public links sections.

Issue

#4782

CLAassistant · 2026-03-21T15:43:38Z

All committers have signed the CLA.

jesmrec · 2026-03-23T07:59:59Z

Thanks for the contribution @corevibe555 . I'll give you some tips that you will also find in our CONTRIBUTING file:

For the branch names, we use underscore _ to separate the chunks, like corevibe555:fix/apply_read_permission_to_the_list_of_members. Don't worry for that in the current PR, but please take in account for future contributions 😉. We'd appreciate that.
In terms of CHANGELOG, we use calens to keep our changelog updated. The only task you have to do is adding an small file to our unreleased folder, which name is the PR number. There, you'll find some examples. As a guide, here you have the TEMPLATE. Don't forget to use the present perfect passive tense!! let us know about any question about this.
I see two different commiters (root and corevibe555). In order to be able to merge the PR, every commiter must sign the CLA. As you can see in the CLAassistant message, just one is signed, so the CI system ⬇️ is waiting for it (required).
About CI, we use Detekt as static code analyzer. Detekt is marking the PR with a red ❌ (check it below ⬇️ ), because one of the methods you changed exceedes the maximum number of lines. Take a look into it and let us know!
As information, every PR has to pass two different stages: a code review that uses to be done by @joragua , and a QA-testing phase, that i will do once code review is approved. During these phases, we can suggest or ask you for changes (always open to discussion). When both CR and QA passes, the PR is merged. Also interesting to know: our branch update method is rebasing, not merging.

Any question about the process, @joragua or myself will be happy to help you!! Don't hesitate to ask!! 😄

corevibe555 · 2026-03-23T11:36:26Z

@jesmrec Thanks for your comment.
I am working on the feedback.

jesmrec · 2026-03-23T11:37:37Z

@jesmrec Thanks for your kind comment.
I am working on these feedback.
As a first time contributor, I will keep in mind for any future contributions.

cool! let us know if you need some help 🚀

jesmrec · 2026-03-23T13:14:13Z

Thanks @corevibe555 for the quick response. New iteration is much better, but still some tiny details to take care of:

About the calens file, we might not need a calens entry for the current PR, since it's not released yet. Just add the links to the following calens file: https://github.com/owncloud/android/blob/master/changelog/unreleased/4728, that complies all the features and fixes related with the Members option. Sorry, i did notice that first, and thanks to @joragua for the tip.
We also use to reduce the number of commits. For example, this commit could be saved by removing the comment lines from the commit ahead with an interactive rebase (let us know if you need help)
I'd mark the detekt commit as refactor instead of a fix, it fits better.

Summing up, this could be the better approach:

fix: show members menu without permissions/read -> including your fix candidate
chore: add changelog -> add links to 4728 calens file
refactor: resolve Detekt issue -> with the Detekt stuff

with this, we could move to code review.

…AD_PERMISSION check Made-with: Cursor

Made-with: Cursor

corevibe555 · 2026-03-23T13:47:01Z

Thanks for the feedback, I've updated the PR.

jesmrec · 2026-03-23T14:05:17Z

Moving to CR. As soon as we are available, we'll review and give you feedback. Thanks!

corevibe555 · 2026-03-30T09:01:36Z

@joragua Would you please review my PR and give me any feedback so I can complete and pick next issue?

joragua · 2026-03-30T09:14:06Z

Hi @corevibe555! As mentioned in #4782 (comment), I'm missing the logic for space public links. If the user doesn't have the permission, the Members view should be completely empty (only the space header with the permanent link action should be displayed). Could you update the code and implement this, please? Ping me when it's ready and I'll review your PR! 🙌🏻

joragua

Good job @corevibe555! 💯 Somme comments about your code

...udApp/src/main/java/com/owncloud/android/presentation/spaces/members/SpaceMembersFragment.kt

owncloudApp/src/main/res/layout/members_fragment.xml

corevibe555 · 2026-03-30T19:38:39Z

Thank you for the review comments. I've made requested changes.

joragua

One more comment and it's ready to go! @corevibe555

owncloudApp/src/main/res/layout/members_fragment.xml

fix: remove updateToolbarTitle() as this is not in the requirement fix: keep simple UX refactor: remove updateMembersLoadingProgressVisibility refactor: resolve code review comment feedback fix: remove redundant properties

joragua

LGTM! Moving it to QA 😄

…embers

jesmrec · 2026-04-01T12:49:36Z

@corevibe555 your contribution has several conflicts with the current version on the master branch. We have tried to solve them, but the merge commit that was created on your branch by github was not useful because your master was not updated with ours. Could yo remove this commit?

We'll assist you with the conflict solving.

corevibe555 changed the title ~~[FIX]: keep Members entry visible; restrict member list by read permission (#4782)~~ [FIX] Keep Members entry visible; restrict member list by read permission (#4782) Mar 21, 2026

corevibe555 force-pushed the fix/apply-read-permission-to-the-list-of-members branch from d0c5a29 to e2c26e7 Compare March 22, 2026 23:17

joragua linked an issue Mar 23, 2026 that may be closed by this pull request

[BUG] Apply read permission to the list of members instead of the main option #4782

Open

corevibe555 force-pushed the fix/apply-read-permission-to-the-list-of-members branch 2 times, most recently from 6de7461 to 1470da1 Compare March 23, 2026 12:07

corevibe555 added 3 commits March 23, 2026 14:45

fix: show members menu without permissions/read by removing DRIVES_RE…

e33a62b

…AD_PERMISSION check Made-with: Cursor

chore: add changelog

a4c0d22

Made-with: Cursor

refactor: resolve Detekt issue

bf812a9

Made-with: Cursor

corevibe555 force-pushed the fix/apply-read-permission-to-the-list-of-members branch from 1470da1 to bf812a9 Compare March 23, 2026 13:45

jesmrec requested a review from joragua March 23, 2026 14:05

corevibe555 force-pushed the fix/apply-read-permission-to-the-list-of-members branch from 3640413 to bf812a9 Compare March 24, 2026 16:56

corevibe555 force-pushed the fix/apply-read-permission-to-the-list-of-members branch 3 times, most recently from 758ed0b to 0667c49 Compare March 30, 2026 10:30

joragua changed the title ~~[FIX] Keep Members entry visible; restrict member list by read permission (#4782)~~ [FIX] Apply read permission to the list of members instead of the main option Mar 30, 2026

joragua requested changes Mar 30, 2026

View reviewed changes

corevibe555 force-pushed the fix/apply-read-permission-to-the-list-of-members branch from 0667c49 to 920ad53 Compare March 30, 2026 19:33

joragua requested changes Mar 31, 2026

View reviewed changes

owncloudApp/src/main/res/layout/members_fragment.xml Outdated Show resolved Hide resolved

fix: hide public links when there is no read permission

5870a78

fix: remove updateToolbarTitle() as this is not in the requirement fix: keep simple UX refactor: remove updateMembersLoadingProgressVisibility refactor: resolve code review comment feedback fix: remove redundant properties

corevibe555 force-pushed the fix/apply-read-permission-to-the-list-of-members branch from 920ad53 to 5870a78 Compare March 31, 2026 10:49

joragua approved these changes Mar 31, 2026

View reviewed changes

Merge branch 'master' into fix/apply-read-permission-to-the-list-of-m…

41db2bc

…embers

Conversation

corevibe555 commented Mar 21, 2026 • edited by joragua Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Solution

Issue

Uh oh!

CLAassistant commented Mar 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jesmrec commented Mar 23, 2026

Uh oh!

corevibe555 commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jesmrec commented Mar 23, 2026

Uh oh!

jesmrec commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

corevibe555 commented Mar 23, 2026

Uh oh!

jesmrec commented Mar 23, 2026

Uh oh!

corevibe555 commented Mar 30, 2026

Uh oh!

joragua commented Mar 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

joragua left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

corevibe555 commented Mar 30, 2026

Uh oh!

joragua left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

joragua left a comment

Choose a reason for hiding this comment

Uh oh!

jesmrec commented Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

corevibe555 commented Mar 21, 2026 •

edited by joragua

Loading

CLAassistant commented Mar 21, 2026 •

edited

Loading

corevibe555 commented Mar 23, 2026 •

edited

Loading

jesmrec commented Mar 23, 2026 •

edited

Loading

joragua commented Mar 30, 2026 •

edited

Loading

jesmrec commented Apr 1, 2026 •

edited

Loading