github-mirror/discourse
2
0
Fork 0
mirror of https://github.com/discourse/discourse.git synced 2024-11-25 09:42:07 +08:00
Code Issues Actions 6 Packages Projects Releases Wiki Activity

FIX: Mobile app notification urls w/ subfolder (#12282)

Browse Source 
The urls that we generate for mobile post notifications don't take into
account the subfolder url if a site happens to have one configured. When
this happens when you tap on a new mobile notification it takes you to
a url that doesn't work because it is missing the subfolder portion.

I honestly think this should be handled in the Post model like we do
with the Topic model. `Post.url` should know how to handle subfolder
installs, but that seemed like a very risky change because there are
lots of other places in the codebase where we tack on the base_path and
I didn't want to risk duplicating it.

I also found a small typo in the topics controller spec.
This commit is contained in:
Blake Erickson 2021-03-04 07:07:37 -07:00 committed by GitHub
parent 4a41f72f09
commit 2123561125
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 7 additions and 6 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/jobs/regular/push_notification.rb
View File

@ -4,7 +4,7 @@ module Jobs
class PushNotification < ::Jobs::Base class PushNotification < ::Jobs::Base
def execute(args) def execute(args)
notification = args["payload"] notification = args["payload"]
notification["url"] = UrlHelper.absolute_without_cdn(notification["post_url"]) notification["url"] = UrlHelper.absolute_without_cdn(Discourse.base_path + notification["post_url"])
notification.delete("post_url") notification.delete("post_url")
payload = { payload = {

2
spec/requests/topics_controller_spec.rb
View File

@ -1645,7 +1645,7 @@ RSpec.describe TopicsController do
end end
end end
it 'correctly renders canoicals' do it 'correctly renders canonicals' do
get "/t/#{topic.id}", params: { slug: topic.slug } get "/t/#{topic.id}", params: { slug: topic.slug }
expect(response.status).to eq(200) expect(response.status).to eq(200)

9
spec/services/post_alerter_spec.rb
View File

@ -768,6 +768,7 @@ describe PostAlerter do
{ status: 200, body: "OK" } { status: 200, body: "OK" }
end end
set_subfolder "/subpath"
payload = { payload = {
"secret_key" => SiteSetting.push_api_secret_key, "secret_key" => SiteSetting.push_api_secret_key,
"url" => Discourse.base_url, "url" => Discourse.base_url,
@ -781,7 +782,7 @@ describe PostAlerter do
'topic_id' => topic.id, 'topic_id' => topic.id,
'excerpt' => 'Hello @eviltrout ❤', 'excerpt' => 'Hello @eviltrout ❤',
'username' => user.username, 'username' => user.username,
'url' => UrlHelper.absolute(mention_post.url), 'url' => UrlHelper.absolute(Discourse.base_path + mention_post.url),
'client_id' => 'xxx0' 'client_id' => 'xxx0'
}, },
{ {
@ -791,7 +792,7 @@ describe PostAlerter do
'topic_id' => topic.id, 'topic_id' => topic.id,
'excerpt' => 'Hello @eviltrout ❤', 'excerpt' => 'Hello @eviltrout ❤',
'username' => user.username, 'username' => user.username,
'url' => UrlHelper.absolute(mention_post.url), 'url' => UrlHelper.absolute(Discourse.base_path + mention_post.url),
'client_id' => 'xxx1' 'client_id' => 'xxx1'
} }
] ]
@ -815,7 +816,7 @@ describe PostAlerter do
"post_number" => new_post.post_number, "post_number" => new_post.post_number,
"username" => new_post.user.username, "username" => new_post.user.username,
"excerpt" => new_post.raw, "excerpt" => new_post.raw,
"url" => UrlHelper.absolute(new_post.url) "url" => UrlHelper.absolute(Discourse.base_path + new_post.url)
} }
payload["notifications"][0].merge! changes payload["notifications"][0].merge! changes
@ -829,7 +830,7 @@ describe PostAlerter do
"post_number" => new_post.post_number, "post_number" => new_post.post_number,
"username" => new_post.user.username, "username" => new_post.user.username,
"excerpt" => new_post.raw, "excerpt" => new_post.raw,
"url" => UrlHelper.absolute(new_post.url) "url" => UrlHelper.absolute(Discourse.base_path + new_post.url)
} }
payload["notifications"][0].merge! changes payload["notifications"][0].merge! changes