-
Notifications
You must be signed in to change notification settings - Fork 750
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix external redirect link not shown to author #2882
base: main
Are you sure you want to change the base?
Conversation
Fixes an issue where a page configured as a redirect and havign a target of anything other than a Page (e.g. Asset, or external URL) does not display the link target to the author viewing the page in edit mode.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2882 +/- ##
============================================
+ Coverage 87.15% 87.18% +0.02%
- Complexity 2692 2697 +5
============================================
Files 235 235
Lines 7188 7196 +8
Branches 1100 1099 -1
============================================
+ Hits 6265 6274 +9
Misses 365 365
+ Partials 558 557 -1 ☔ View full report in Codecov by Sentry. |
Page page = getPageUnderTest(REDIRECT_PAGE_EXTERNAL); | ||
NavigationItem redirectTarget = page.getRedirectTarget(); | ||
assertNotNull(redirectTarget); | ||
assertNull(redirectTarget.getPage()); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
NavigationItem.getPage
NavigationItem redirectTarget = page.getRedirectTarget(); | ||
assertNotNull(redirectTarget); | ||
assertNull(redirectTarget.getPage()); | ||
assertEquals("https://www.adobe.com/", redirectTarget.getURL()); |
Check notice
Code scanning / CodeQL
Deprecated method or constructor invocation Note test
ListItem.getURL
Quality Gate passedIssues Measures |
Fixes an issue where a page configured as a redirect and having a target of anything other than a Page (e.g. Asset, or external URL) does not display the link target to the author viewing the page in edit mode.
This issue occurred because:
v1/RedirectItemImpl
resolved thePage
for the redirect target before handing that page to theLinkManager
.LinkManager#get(Page)
requiresPage
to be not null; however it would be passed a null unless the redirect target could be resolved to aPage
.LinkManager#get(null)
is called (because the redirect target does not point to aPage
), aLink
is returned where the URL for the link is null.Additionally, even if a valid
Link
were returned, it would not be presented to the author because of a typo inpage/v3/redirect.html
:The problem here is that
redirectTarget.linkURL
does not exist because there is no methodNavigationItem#getLinkURL()
.This is probably a simple typo that should have been
redirectTarget.link.URL
Solution
The solution included in this PR resolves the issue by:
redirect.html
.v1/RedirectItemImpl
to callLinkManager#get(Resource)
with aResource
that has the redirect target path as a property, instead of forcing it to be a page and callingLinkManager#get(Page)
.This solution allows the link to be anything: Page, Asset, external, etc.
The path/URL will be shown to the author in all cases except where the redirect target is a page, in which case the page title will continue to be shown.