Skip to content

feat(files_sharing): Add toggle to control display area for federated shares - #52511

Merged
nfebe merged 4 commits into
masterfrom
feat/no-issue/show-remote-shares-as-internal-config
May 6, 2025
Merged

feat(files_sharing): Add toggle to control display area for federated shares#52511
nfebe merged 4 commits into
masterfrom
feat/no-issue/show-remote-shares-as-internal-config

Conversation

@nfebe

@nfebenfebe commented Apr 28, 2025

Copy link
Copy Markdown
Contributor

Add command to control display area for federated shares following : #52423

Usage: occ config:app:set --value false --type boolean files_sharing show_federated_shares_as_internal

Screencast

external-shares-control.mp4

@nfebenfebe added feature: sharing pending documentation This pull request needs an associated documentation update labels Apr 28, 2025
@nfebe
nfebe requested review from a team as code ownersApril 28, 2025 17:25
@nfebe
nfebe requested review from skjnldsv and szaimen and removed request for a teamApril 28, 2025 17:25
@github-project-automationgithub-project-automationBot moved this to 🏗️ In progress in 📁 Files teamApr 28, 2025
Comment threadapps/files_sharing/lib/AppInfo/Application.php Outdated
@szaimen
szaimen removed their request for review April 29, 2025 08:26
Comment threadapps/files_sharing/lib/AppInfo/Application.php Outdated
@kesselb

Copy link
Copy Markdown
Contributor

Hi,

With sharing.federation.show_remote_shares_as_internal = true, is it possible to add such a share by entering the user's name in the internal shares section? I currently don't have a GS setup to validate this myself.

Given that federation and GS are quite complex topics with various configuration options that can influence behavior, I think we should do another round of planning to adjust the new sharing sidebar to meet expectations. I'm concerned that a simple flag to move shares from one list to another might not be sufficient.

@AndyScherzinger

AndyScherzinger commented Apr 30, 2025

Copy link
Copy Markdown
Member

With sharing.federation.show_remote_shares_as_internal = true, is it possible to add such a share by entering the user's name in the internal shares section? I currently don't have a GS setup to validate this myself.

Yes, that is the goal (or how it has been before the latest change). For a GS setup with this config flag (and it ultimately will do more than just this PR). In the end federated shared (in a GS setup/scenario) shall behave (from a user perspective) like local shares (while this will have its limitations or in other word won't be possible 100% given technical limitations).

So another (the second aspect to come later) is to hide the (remote) info on the username on the share-item in the sidebar because "should behave local".

@kesselb This PR is mandatory for the next release (!).

@nfebe
nfebeforce-pushed the feat/no-issue/show-remote-shares-as-internal-config branch from 4336802 to 5c2b084CompareMay 1, 2025 14:42
@nfebe
nfebe requested a review from come-nc as a code ownerMay 1, 2025 14:42
@nfebe
nfebeforce-pushed the feat/no-issue/show-remote-shares-as-internal-config branch from 5c2b084 to d4e6a96CompareMay 1, 2025 14:50
@nfebenfebe changed the title feat(files_sharing): Add config to control display area for federated…feat(files_sharing): Add command to control display area for federated sharesMay 1, 2025
@nfebenfebe self-assigned this May 1, 2025
@nfebe
nfebe requested a review from kesselbMay 1, 2025 15:01
@nfebe
nfebeforce-pushed the feat/no-issue/show-remote-shares-as-internal-config branch from d4e6a96 to fa60feeCompareMay 1, 2025 15:03
@nfebe
nfebe enabled auto-merge May 1, 2025 15:04

@AndyScherzingerAndyScherzinger left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🦈

Comment threadapps/files_sharing/lib/Command/ShowFederatedSharesAsInternal.php Outdated
Comment threadapps/files_sharing/lib/Command/ShowFederatedSharesAsInternal.php Outdated
Comment threadapps/files_sharing/src/services/ConfigService.ts Outdated
Comment threadapps/files_sharing/lib/AppInfo/Application.php Outdated
@kesselb

Copy link
Copy Markdown
Contributor

Iirc we treat federation different with global scale anyways so we could just check if global scale is enabled and if it is then we just treat any remote user as internal at least for the sharing sidebar maybe this could be an option so we don't need any UI option.1

Currently, the administrator has to set the configuration value to revert to the old behavior.

I assume Ferdinand was referring to the following code. With GS enabled, the sharees API will query the lookup server regardless of the given share type, and therefore also return users from other nodes in the GS compound.

While I don't fully understand how onlyInternalFederation (gs.federation true/false) influences this, we could use isGlobalScaleEnabled as the default value to revert to the old behavior for affected customers without requiring any additional actions on their end. However, we should keep the option to override it for those who might need it.

// In global scale mode we always search the lookup server
$this->result['lookupEnabled'] = Server::get(GlobalScaleIConfig::class)->isGlobalScaleEnabled();

Example:

$this->initialState->provideInitialState(
'showFederatedSharesAsInternal',
$this->appConfig->getAppValueBool('show_federated_shares_as_internal', $this->globalScaleConfig->isGlobalScaleEnabled())
);

Footnotes

  1. https://github.com/nextcloud/server/pull/52423#issuecomment-2834542402

@nfebe
nfebeforce-pushed the feat/no-issue/show-remote-shares-as-internal-config branch 3 times, most recently from 70ada64 to 084ed3cCompareMay 6, 2025 12:06
@nfebe

nfebe commented May 6, 2025

Copy link
Copy Markdown
ContributorAuthor

/compile

@nfebe
nfebeforce-pushed the feat/no-issue/show-remote-shares-as-internal-config branch 3 times, most recently from 2629602 to 438f722CompareMay 6, 2025 12:30
@nfebe

nfebe commented May 6, 2025

Copy link
Copy Markdown
ContributorAuthor

/compile

@ArtificialOwl

Copy link
Copy Markdown
Member

II would move ConfigLexicon.php to lib/ to keep consistency with other apps (as /lib/AppInfo/ was not accepted ;p)

nfebeand others added 3 commits May 6, 2025 14:28
Signed-off-by: Maxence Lange <maxence@artificial-owl.com>
…g input
- Fix autoloading for new `ConfigLexicon`
- Ensure that sharing input in sharing tab respect `show-federated-shares-as-internal`:
This is important, because when federated shares are shown as internal the users should add them from the internal shares section
Signed-off-by: nfebe <fenn25.fn@gmail.com>
@nfebe
nfebeforce-pushed the feat/no-issue/show-remote-shares-as-internal-config branch from e439bdb to f471bd2CompareMay 6, 2025 13:28
@nfebe

nfebe commented May 6, 2025

Copy link
Copy Markdown
ContributorAuthor

/compile

Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
@nfebe
nfebe merged commit 5985793 into masterMay 6, 2025
@nfebe
nfebe deleted the feat/no-issue/show-remote-shares-as-internal-config branch May 6, 2025 14:20
@AndyScherzingerAndyScherzinger added this to the Nextcloud 32 milestone May 6, 2025
@AndyScherzingerAndyScherzinger moved this from 🏗️ In progress to ☑️ Done in 📁 Files teamMay 6, 2025
@maximelehericy

Copy link
Copy Markdown

@nfebe is the setting documented elsewhere than here ?

@nfebe

Copy link
Copy Markdown
ContributorAuthor

@nfebe is the setting documented elsewhere than here ?
nextcloud/documentation#13316

@nfebenfebe removed the pending documentation This pull request needs an associated documentation update label Jun 25, 2025
@skjnldsv

Copy link
Copy Markdown
Member

Sorry, but that sounds a bit extreme to add a custom config for this, no?

@skjnldsvskjnldsv mentioned this pull request Aug 19, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: ☑️ Done

Development

Successfully merging this pull request may close these issues.

7 participants

@nfebe@kesselb@AndyScherzinger@ArtificialOwl@maximelehericy@skjnldsv@nextcloud-command