fix: remove broken owncloud.com/federation link from federation settings - #41608
Conversation
DeepDiver1975
left a comment
There was a problem hiding this comment.
Removes the defunct https://owncloud.com/federation#<cloudID> URL from the federated sharing personal settings panel and replaces the linked badge with a plain <span> that displays the Cloud ID inline.
The fix is clean and correct. A few notes:
message_with_URL still assigned but now contains the Cloud ID instead of a URL: GeneralPersonalPanel.php now passes $cloudID as the second argument to message_with_URL, making it effectively identical to message_without_URL. The Twitter button correctly switched to message_without_URL. Consider whether message_with_URL is still needed at all, or whether it can be removed to reduce dead template variables — not a blocker, but worth a follow-up.
<xmp> tag: The HTML snippet preview still uses the deprecated <xmp> element (pre-existing, not introduced here). Not a regression from this PR.
Changelog entry: changelog/unreleased/40501 is present and well-written. Good.
The core fix (removing the broken external link) is solid and the UX impact is minimal — users lose a broken link and gain a directly visible Cloud ID. Safe to merge.
85fbc3a to
e71a606
Compare
DeepDiver1975
left a comment
There was a problem hiding this comment.
Updated revision — diff is identical to the previously reviewed version. The changelog entry title was tweaked slightly ("Remove broken owncloud.com/federation link" → "Remove owncloud.com/federation link"), but all code changes are unchanged. Previous review comments still apply; no new concerns from this revision.
phil-davis
left a comment
There was a problem hiding this comment.
LGTM now, and tests so far are passing.
DeepDiver1975
left a comment
There was a problem hiding this comment.
Code Review
Overview: Removes a dead link (https://owncloud.com/federation#) from the federated cloud sharing personal settings panel. The external page no longer exists after owncloud.com was restructured.
Changes:
GeneralPersonalPanel.php: Drops$urlvariable, passes$cloudIDdirectly as bothmessage_with_URLandreferencetemplate vars — the Cloud ID is already the meaningful identifier, the defunct URL added no valuesettings-personal-general.php: Twitter share button switches frommessage_with_URLtomessage_without_URL(correct — there's no longer a URL to share); the "Add to your website" badge replaces<a href="...">with<span>and appends the Cloud ID inlineGeneralPersonalPanelTest.php: Adds$mockUser->method('getCloudId')->willReturn('user@cloud.example.com')stub — previously missing, which would have caused a null return fromgetCloudId()and a broken test against the now-removed$urlconstructionchangelog/unreleased/40501: Well-written entry clearly explaining the "Add to your website" breakage
One minor observation: The message_with_URL template variable is now populated with just the Cloud ID (same as message_without_URL), making the two variables redundant. A follow-up cleanup could consolidate them, but it's not a blocker for this fix.
Overall: Clean, minimal fix. The test addition is a welcome improvement. No concerns.
The "Add to your website" feature linked to https://owncloud.com/federation# which stopped working after owncloud.com was restructured. Replace the broken external link with the plain federation Cloud ID so the snippet displayed to users is still useful. Fixes #40501 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
The test's IUser mock never returned a value for getCloudId(), which was previously harmless because the panel concatenated it into a URL string (null → ""). After removing the broken owncloud.com/federation link, cloudId is passed directly as the 'reference' template variable, causing urlencode(null) to throw an ErrorException on PHP 8.3. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Signed-off-by: Thomas Müller <1005065+DeepDiver1975@users.noreply.github.com>
4edd744 to
680989b
Compare
Summary
https://owncloud.com/federation#<cloudID>which redirects to an unrelated owncloud.com marketing page after a site restructuringmessage_without_URL(was incorrectly usingmessage_with_URLwhich embedded the broken URL)Test Plan
<span>with the Cloud ID instead of<a href="https://owncloud.com/federation#...">Fixes #40501
🤖 Generated with Claude Code