Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -348,12 +348,18 @@
.startActivities();
}

public static void viewReaderPostDetailInNewStack(Context context, long blogId, long postId, Uri uri) {
public static void viewReaderPostDetailInNewStack(
Context context,
long blogId,
long postId,
boolean isFeed,
Uri uri

Check notice

Code scanning / Android Lint

Nullable/NonNull annotation missing on method parameter Note

Missing null annotation
) {
Intent mainActivityIntent = getMainActivityInNewStack(context)
.putExtra(WPMainActivity.ARG_OPEN_PAGE, WPMainActivity.ARG_READER);
Intent viewPostIntent = ReaderActivityLauncher.buildReaderPostDetailIntent(
context,
false,
isFeed,
blogId,
postId,
null,
Expand Down Expand Up @@ -1200,7 +1206,7 @@
Activity activity,
SiteModel site,
PostImmutableModel post,
RemotePreviewType remotePreviewType

Check notice

Code scanning / Android Lint

Nullable/NonNull annotation missing on method parameter Note

Missing null annotation
) {
browsePostOrPageEx(activity, site, post, remotePreviewType);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ class DeepLinkNavigator
activity,
navigateAction.blogId,
navigateAction.postId,
navigateAction.isFeed,
navigateAction.uri.uri
)
OpenNotifications -> ActivityLauncher.viewNotificationsInNewStack(activity)
Expand Down Expand Up @@ -124,7 +125,12 @@ class DeepLinkNavigator
data class OpenEditorForSite(val site: SiteModel) : NavigateAction()
object OpenReader : NavigateAction()
data class OpenInReader(val uri: UriWrapper) : NavigateAction()
data class ViewPostInReader(val blogId: Long, val postId: Long, val uri: UriWrapper) : NavigateAction()
data class ViewPostInReader(
val blogId: Long,
val postId: Long,
val isFeed: Boolean,
val uri: UriWrapper
) : NavigateAction()
object OpenEditor : NavigateAction()
data class OpenStatsForSiteAndTimeframe(val site: SiteModel, val statsTimeframe: StatsTimeframe) :
NavigateAction()
Expand Down
Copy link
Member Author

Choose a reason for hiding this comment

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

From what I understand of DEEP_LINK_HOST_READ and DEEP_LINK_HOST_VIEWPOST, these changes are sound and do not introduce regressions. However, I am less familiar with this code and welcome additional review and testing.

Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,14 @@ class ReaderLinkHandler

override fun buildNavigateAction(uri: UriWrapper): NavigateAction {
return when (uri.host) {
DEEP_LINK_HOST_READ -> OpenReader
DEEP_LINK_HOST_READ -> buildReadNavigateAction(uri)
DEEP_LINK_HOST_VIEWPOST -> {
val blogId = uri.getQueryParameter(BLOG_ID)?.toLongOrNull()
val postId = uri.getQueryParameter(POST_ID)?.toLongOrNull()
if (blogId != null && postId != null) {
analyticsUtilsWrapper.trackWithBlogPostDetails(READER_VIEWPOST_INTERCEPTED, blogId, postId)
ViewPostInReader(blogId, postId, uri)
// viewpost deep links always use blog IDs, not feed IDs
ViewPostInReader(blogId, postId, isFeed = false, uri)
} else {
_toast.value = Event(R.string.error_generic)
OpenReader
Expand All @@ -60,9 +61,56 @@ class ReaderLinkHandler
}
}

/**
* Builds the navigate action for URIs with "read" host.
* Handles paths like /blogs/{blogId}/posts/{postId} or /feeds/{feedId}/posts/{feedItemId}
*/
private fun buildReadNavigateAction(uri: UriWrapper): NavigateAction {
val segments = uri.pathSegments
if (!isValidReadPath(segments)) {
return OpenReader
}
val blogId = segments[BLOG_ID_PATH_POSITION].toLongOrNull()
val postId = segments[POST_ID_PATH_POSITION].toLongOrNull()
val isFeed = segments[0] == FEEDS_PATH
return if (blogId != null && postId != null) {
analyticsUtilsWrapper.trackWithBlogPostDetails(READER_VIEWPOST_INTERCEPTED, blogId, postId)
ViewPostInReader(blogId, postId, isFeed, uri)
} else {
OpenReader
}
}

/**
* Checks if the path segments represent a valid read path.
* Valid paths: /blogs/{blogId}/posts/{postId} or /feeds/{feedId}/posts/{feedItemId}
*/
private fun isValidReadPath(segments: List<String>): Boolean {
if (segments.size < MIN_READ_PATH_SEGMENTS) return false
val isBlogsOrFeeds = segments[0] == BLOGS_PATH || segments[0] == FEEDS_PATH
val hasPostsSegment = segments[POSTS_SEGMENT_POSITION] == POSTS_PATH
return isBlogsOrFeeds && hasPostsSegment
}

/**
* Builds a stripped URL for analytics from URIs with "read" host.
* Replaces actual IDs with placeholders.
*/
private fun buildStrippedReadUrl(uri: UriWrapper): String {
val segments = uri.pathSegments
return buildString {
append("$APPLINK_SCHEME$DEEP_LINK_HOST_READ")
if (isValidReadPath(segments)) {
append("/${segments[0]}/$BLOG_ID/$POSTS_PATH/$POST_ID")
}
}
}

/**
* URLs handled here
* `wordpress://read`
* `wordpress://read/blogs/{blogId}/posts/{postId}`
* `wordpress://read/feeds/{feedId}/posts/{feedItemId}`
* `wordpress://viewpost?blogId={blogId}&postId={postId}`
* wordpress.com/read/feeds/feedId/posts/feedItemId
* wordpress.com/read/blogs/feedId/posts/feedItemId
Expand All @@ -72,7 +120,7 @@ class ReaderLinkHandler
*/
override fun stripUrl(uri: UriWrapper): String {
return when (uri.host) {
DEEP_LINK_HOST_READ -> "$APPLINK_SCHEME$DEEP_LINK_HOST_READ"
DEEP_LINK_HOST_READ -> buildStrippedReadUrl(uri)
DEEP_LINK_HOST_VIEWPOST -> {
val hasBlogId = uri.getQueryParameter(BLOG_ID) != null
val hasPostId = uri.getQueryParameter(POST_ID) != null
Expand Down Expand Up @@ -142,9 +190,17 @@ class ReaderLinkHandler
private const val BLOG_ID = "blogId"
private const val POST_ID = "postId"
private const val FEED_ID = "feedId"
private const val BLOGS_PATH = "blogs"
private const val FEEDS_PATH = "feeds"
private const val POSTS_PATH = "posts"
private const val CUSTOM_DOMAIN_POSITION = 3
private const val BLOGS_FEEDS_PATH_POSITION = 1
private const val POSTS_PATH_POSITION = 3
private const val DATE_URL_SEGMENTS = 3
// Path segment positions for read deep links: /blogs/{blogId}/posts/{postId}
private const val MIN_READ_PATH_SEGMENTS = 4
private const val BLOG_ID_PATH_POSITION = 1
private const val POSTS_SEGMENT_POSITION = 2
private const val POST_ID_PATH_POSITION = 3
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,44 @@ class ReaderLinkHandlerTest : BaseUnitTest() {
assertThat(navigateAction).isEqualTo(OpenReader)
}

@Test
fun `URI with read host and blogs path opens post in reader with isFeed false`() {
val uri = buildUri("read", "blogs", blogId.toString(), "posts", postId.toString())

val navigateAction = readerLinkHandler.buildNavigateAction(uri)

assertThat(navigateAction).isEqualTo(ViewPostInReader(blogId, postId, isFeed = false, uri))
verify(analyticsUtilsWrapper).trackWithBlogPostDetails(READER_VIEWPOST_INTERCEPTED, blogId, postId)
}

@Test
fun `URI with read host and feeds path opens post in reader with isFeed true`() {
val uri = buildUri("read", "feeds", feedId.toString(), "posts", postId.toString())

val navigateAction = readerLinkHandler.buildNavigateAction(uri)

assertThat(navigateAction).isEqualTo(ViewPostInReader(feedId, postId, isFeed = true, uri))
verify(analyticsUtilsWrapper).trackWithBlogPostDetails(READER_VIEWPOST_INTERCEPTED, feedId, postId)
}

@Test
fun `URI with read host and invalid blog ID opens reader`() {
val uri = buildUri("read", "blogs", "invalid", "posts", postId.toString())

val navigateAction = readerLinkHandler.buildNavigateAction(uri)

assertThat(navigateAction).isEqualTo(OpenReader)
}

@Test
fun `URI with read host and incomplete path opens reader`() {
val uri = buildUri("read", "blogs", blogId.toString())

val navigateAction = readerLinkHandler.buildNavigateAction(uri)

assertThat(navigateAction).isEqualTo(OpenReader)
}

@Test
fun `URI with viewpost host without query params opens reader`() {
val uri = buildUri(host = "viewpost")
Expand All @@ -104,7 +142,7 @@ class ReaderLinkHandlerTest : BaseUnitTest() {
}

@Test
fun `URI with viewpost host with query params opens post in reader`() {
fun `URI with viewpost host with query params opens post in reader with isFeed false`() {
val uri = buildUri(
host = "viewpost",
queryParam1 = "blogId" to blogId.toString(),
Expand All @@ -113,7 +151,7 @@ class ReaderLinkHandlerTest : BaseUnitTest() {

val navigateAction = readerLinkHandler.buildNavigateAction(uri)

assertThat(navigateAction).isEqualTo(ViewPostInReader(blogId, postId, uri))
assertThat(navigateAction).isEqualTo(ViewPostInReader(blogId, postId, isFeed = false, uri))
verify(analyticsUtilsWrapper).trackWithBlogPostDetails(READER_VIEWPOST_INTERCEPTED, blogId, postId)
}

Expand All @@ -135,6 +173,24 @@ class ReaderLinkHandlerTest : BaseUnitTest() {
assertThat(strippedUrl).isEqualTo("wordpress://read")
}

@Test
fun `correctly strips READ applink with blogs path`() {
val uri = buildUri("read", "blogs", blogId.toString(), "posts", postId.toString())

val strippedUrl = readerLinkHandler.stripUrl(uri)

assertThat(strippedUrl).isEqualTo("wordpress://read/blogs/blogId/posts/postId")
}

@Test
fun `correctly strips READ applink with feeds path`() {
val uri = buildUri("read", "feeds", feedId.toString(), "posts", postId.toString())

val strippedUrl = readerLinkHandler.stripUrl(uri)

assertThat(strippedUrl).isEqualTo("wordpress://read/feeds/blogId/posts/postId")
}

@Test
fun `correctly strips VIEWPOST applink with all params`() {
val uri = buildUri(
Expand Down