Skip to content

Commit 1659403

Browse files
keankean
committed
Fix an issue with subscribing to Jetpack-connected site from search leading to duplicated subscriptions
1 parent cdd01f9 commit 1659403

File tree

9 files changed

+523
-196
lines changed

9 files changed

+523
-196
lines changed

‎Modules/Sources/WordPressKit/ReaderFeed.swift‎

Lines changed: 93 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,56 @@ import Foundation
44
/// Encapsulates details of a single feed returned by the Reader feed search API
55
/// (read/feed?q=query)
66
///
7+
/// The API returns different structures depending on the site type:
8+
/// - WordPress.com sites: Data at root level (URL, title, blog_ID)
9+
/// - Jetpack sites: Data in meta.data.feed
10+
/// - External RSS feeds: Data in meta.data.feed, blog_ID is "0"
11+
///
712
public struct ReaderFeed: Decodable {
8-
public let url: URL?
9-
public let title: String?
10-
public let feedDescription: String?
11-
public let feedID: String?
12-
public let blogID: String?
13-
public let blavatarURL: URL?
13+
/// Feed ID from meta.data.feed
14+
public var feedID: String? {
15+
feed?.feedID
16+
}
1417

15-
private enum CodingKeys: String, CodingKey {
16-
case url = "URL"
17-
case title = "title"
18-
case feedID = "feed_ID"
19-
case blogID = "blog_ID"
20-
case meta = "meta"
18+
/// Site/Feed URL with fallback: data.site → data.feed
19+
/// Prioritizes site URL over feed URL for canonical representation
20+
public var url: URL? {
21+
site?.url ?? feed?.url
22+
}
23+
24+
/// Site/Feed title with fallback: data.site → data.feed
25+
/// Prioritizes site name over feed name
26+
public var title: String? {
27+
site?.name ?? feed?.name
28+
}
29+
30+
/// Feed description with fallback: data.site → data.feed
31+
public var description: String? {
32+
site?.description ?? feed?.description
33+
}
34+
35+
/// Blog ID with fallback: data.feed.blog_ID → data.site.ID
36+
/// Returns nil if "0" (external RSS feeds)
37+
public var blogID: String? {
38+
let id = feed?.blogID ?? site?.id.map(String.init)
39+
return (id == "0") ? nil : id
40+
}
41+
42+
/// Site icon/avatar URL, prioritizing data.site.icon.img over data.feed.image
43+
public var iconURL: URL? {
44+
site?.iconURL ?? feed?.imageURL
45+
}
46+
47+
// MARK: - Decodable
48+
49+
/// Feed data from meta.data.feed
50+
private var feed: FeedData?
51+
52+
/// Site data from meta.data.site
53+
private var site: SiteData?
54+
55+
private enum CodingKeys: CodingKey {
56+
case meta
2157
}
2258

2359
private enum MetaKeys: CodingKey {
@@ -30,61 +66,52 @@ public struct ReaderFeed: Decodable {
3066
}
3167

3268
public init(from decoder: Decoder) throws {
33-
// We have to manually decode the feed from the JSON, for a couple of reasons:
34-
// - Some feeds have no `icon` dictionary
35-
// - Some feeds have no `data` dictionary
36-
// - We want to decode whatever we can get, and not fail if neither of those exist
37-
let rootContainer = try decoder.container(keyedBy: CodingKeys.self)
38-
39-
var feedURL = try? rootContainer.decodeIfPresent(URL.self, forKey: .url)
40-
var title = try? rootContainer.decodeIfPresent(String.self, forKey: .title)
41-
feedID = try? rootContainer.decode(String.self, forKey: .feedID)
42-
blogID = try? rootContainer.decode(String.self, forKey: .blogID)
43-
44-
var feedDescription: String?
45-
var blavatarURL: URL?
46-
47-
// Try to parse both site and feed data from meta.data
48-
do {
49-
let metaContainer = try rootContainer.nestedContainer(keyedBy: MetaKeys.self, forKey: .meta)
50-
let dataContainer = try metaContainer.nestedContainer(keyedBy: DataKeys.self, forKey: .data)
51-
52-
let siteData = try? dataContainer.decode(SiteOrFeedData.self, forKey: .site)
53-
let feedData = try? dataContainer.decode(SiteOrFeedData.self, forKey: .feed)
54-
55-
// Use data from either source, preferring site data when both are available
56-
feedDescription = siteData?.description ?? feedData?.description
57-
blavatarURL = siteData?.iconURL ?? feedData?.iconURL
58-
59-
// Fixes CMM-1002: in some cases, the backend fails to embed certain fields
60-
// directly in the feed object
61-
if feedURL == nil {
62-
feedURL = siteData?.url ?? feedData?.url
63-
}
64-
if title == nil {
65-
title = siteData?.title ?? feedData?.title
66-
}
67-
} catch {
69+
let root = try decoder.container(keyedBy: CodingKeys.self)
70+
if let meta = try? root.nestedContainer(keyedBy: MetaKeys.self, forKey: .meta),
71+
let data = try? meta.nestedContainer(keyedBy: DataKeys.self, forKey: .data) {
72+
self.feed = try? data.decode(FeedData.self, forKey: .feed)
73+
self.site = try? data.decode(SiteData.self, forKey: .site)
6874
}
75+
}
76+
}
77+
78+
// MARK: - Feed Data
6979

70-
self.url = feedURL
71-
self.title = title
72-
self.feedDescription = feedDescription
73-
self.blavatarURL = blavatarURL
80+
/// Represents feed-specific data from meta.data.feed
81+
private struct FeedData: Decodable {
82+
let feedID: String?
83+
let blogID: String?
84+
let name: String?
85+
let url: URL?
86+
let description: String?
87+
let imageURL: URL?
88+
89+
private enum CodingKeys: String, CodingKey {
90+
case feedID = "feed_ID"
91+
case blogID = "blog_ID"
92+
case name = "name"
93+
case url = "URL"
94+
case description = "description"
95+
case imageURL = "image"
7496
}
7597
}
7698

77-
private struct SiteOrFeedData: Decodable {
78-
var title: String?
79-
var description: String?
80-
var iconURL: URL?
81-
var url: URL?
99+
// MARK: - Site Data
100+
101+
/// Represents site-specific data from meta.data.site
102+
private struct SiteData: Decodable {
103+
let id: Int?
104+
let name: String?
105+
let url: URL?
106+
let description: String?
107+
let iconURL: URL?
82108

83-
enum CodingKeys: String, CodingKey {
84-
case description
85-
case icon
109+
private enum CodingKeys: String, CodingKey {
110+
case id = "ID"
111+
case name = "name"
86112
case url = "URL"
87-
case name
113+
case description = "description"
114+
case icon = "icon"
88115
}
89116

90117
private enum IconKeys: CodingKey {
@@ -94,19 +121,16 @@ private struct SiteOrFeedData: Decodable {
94121
init(from decoder: Decoder) throws {
95122
let container = try decoder.container(keyedBy: CodingKeys.self)
96123

97-
title = try? container.decodeIfPresent(String.self, forKey: .name)
98-
description = try? container.decodeIfPresent(String.self, forKey: .description)
124+
id = try? container.decodeIfPresent(Int.self, forKey: .id)
125+
name = try? container.decodeIfPresent(String.self, forKey: .name)
99126
url = try? container.decodeIfPresent(URL.self, forKey: .url)
127+
description = try? container.decodeIfPresent(String.self, forKey: .description)
100128

101-
// Try to decode the icon URL from the nested icon dictionary
129+
// Decode icon.img if icon dictionary exists
102130
if let iconContainer = try? container.nestedContainer(keyedBy: IconKeys.self, forKey: .icon) {
103131
iconURL = try? iconContainer.decode(URL.self, forKey: .img)
132+
} else {
133+
iconURL = nil
104134
}
105135
}
106136
}
107-
108-
extension ReaderFeed: CustomStringConvertible {
109-
public var description: String {
110-
return "<Feed | URL: \(String(describing: url)), title: \(String(describing: title)), feedID: \(String(describing: feedID)), blogID: \(String(describing: blogID))>"
111-
}
112-
}

‎RELEASE-NOTES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
* [*] Fix overly long related post titles in Reader [#25011]
2424
* [*] Increase number of lines for post tiles in Reader to three [#25019]
2525
* [*] Fix horizontal insets in Reader article view [#25010]
26+
* [*] Fix an issue with duplicated subscriptions for Jetpack-connected sites [#25026]
2627
* [*] Fixed several reader bugs causing posts to load strangely, or not at all [#25016]
2728

2829
26.4

‎Tests/WordPressKitTests/WordPressKitTests/Mock Data/Reader/reader-search-response-01.json‎

Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
{
2+
"feeds": [
3+
{
4+
"subscribe_URL": "https://ma.tt/feed/",
5+
"feed_ID": "188407",
6+
"meta": {
7+
"links": {
8+
"feed": "https://public-api.wordpress.com/rest/v1.1/read/feed/188407",
9+
"site": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865"
10+
},
11+
"data": {
12+
"feed": {
13+
"blog_ID": "1047865",
14+
"feed_ID": "188407",
15+
"blog_owner": {
16+
"ID": 5,
17+
"name": "Matt"
18+
},
19+
"name": "Matt Mullenweg",
20+
"URL": "https://ma.tt/",
21+
"feed_URL": "http://ma.tt/feed",
22+
"subscribers_count": 4520,
23+
"is_following": false,
24+
"last_update": "2025-11-27T07:18:10+00:00",
25+
"last_checked": "2025-11-27T19:04:42+00:00",
26+
"marked_for_refresh": false,
27+
"next_refresh_time": null,
28+
"organization_id": 0,
29+
"subscription_id": null,
30+
"unseen_count": 0,
31+
"meta": {
32+
"links": {
33+
"self": "https://public-api.wordpress.com/rest/v1.1/read/feed/188407",
34+
"site": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865"
35+
}
36+
},
37+
"resolved_feed_url": "https://ma.tt/feed/",
38+
"image": "https://i0.wp.com/ma.tt/files/2024/01/cropped-matt-favicon.png?fit=32%2C32&amp;quality=80&amp;ssl=1",
39+
"description": "Unlucky in Cards"
40+
},
41+
"site": {
42+
"ID": 1047865,
43+
"name": "Matt Mullenweg",
44+
"description": "Unlucky in Cards",
45+
"URL": "https://ma.tt",
46+
"jetpack": true,
47+
"jetpack_connection": true,
48+
"post_count": 5600,
49+
"subscribers_count": 4520,
50+
"lang": "en-US",
51+
"icon": {
52+
"img": "https://ma.tt/files/2024/01/cropped-matt-favicon.png",
53+
"ico": "https://ma.tt/files/2024/01/cropped-matt-favicon.png?w=16"
54+
},
55+
"logo": {
56+
"id": 0,
57+
"sizes": [],
58+
"url": ""
59+
},
60+
"visible": true,
61+
"is_private": false,
62+
"is_coming_soon": false,
63+
"is_following": false,
64+
"organization_id": 0,
65+
"meta": {
66+
"links": {
67+
"self": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865",
68+
"help": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865/help",
69+
"posts": "https://public-api.wordpress.com/rest/v1.1/read/sites/1047865/posts/",
70+
"comments": "https://public-api.wordpress.com/rest/v1.1/sites/1047865/comments/",
71+
"xmlrpc": "https://ma.tt/blog/xmlrpc.php"
72+
}
73+
},
74+
"launch_status": false,
75+
"site_migration": {
76+
"is_complete": false,
77+
"in_progress": false
78+
},
79+
"is_fse_active": false,
80+
"is_fse_eligible": false,
81+
"is_core_site_editor_enabled": false,
82+
"is_wpcom_atomic": false,
83+
"is_wpcom_staging_site": false,
84+
"is_deleted": false,
85+
"is_a4a_client": false,
86+
"is_a4a_dev_site": false,
87+
"is_wpcom_flex": false,
88+
"capabilities": {
89+
"edit_pages": false,
90+
"edit_posts": false,
91+
"edit_others_posts": false,
92+
"edit_theme_options": false,
93+
"list_users": false,
94+
"manage_categories": false,
95+
"manage_options": false,
96+
"publish_posts": false,
97+
"upload_files": false,
98+
"view_stats": false
99+
},
100+
"is_multi_author": true,
101+
"feed_ID": 188407,
102+
"feed_URL": "http://ma.tt/feed",
103+
"header_image": false,
104+
"owner": {
105+
"ID": 5,
106+
"login": "matt",
107+
"name": "Matt",
108+
"first_name": "Matt",
109+
"last_name": "Mullenweg",
110+
"nice_name": "matt",
111+
"URL": "https://matt.blog/",
112+
"avatar_URL": "https://0.gravatar.com/avatar/33252cd1f33526af53580fcb1736172f06e6716f32afdd1be19ec3096d15dea5?s=96&d=retro&r=G",
113+
"profile_URL": "https://gravatar.com/matt",
114+
"ip_address": false,
115+
"site_visible": true,
116+
"has_avatar": true
117+
},
118+
"subscription": {
119+
"delivery_methods": {
120+
"email": null,
121+
"notification": {
122+
"send_posts": false
123+
}
124+
}
125+
},
126+
"is_blocked": false,
127+
"unseen_count": 0
128+
}
129+
}
130+
}
131+
}
132+
]
133+
}

0 commit comments

Comments
 (0)