From 2123561125242a3c048958a56bd8bebe83cd577f Mon Sep 17 00:00:00 2001 From: Blake Erickson Date: Thu, 4 Mar 2021 07:07:37 -0700 Subject: [PATCH] FIX: Mobile app notification urls w/ subfolder (#12282) 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. --- app/jobs/regular/push_notification.rb | 2 +- spec/requests/topics_controller_spec.rb | 2 +- spec/services/post_alerter_spec.rb | 9 +++++---- 3 files changed, 7 insertions(+), 6 deletions(-) diff --git a/app/jobs/regular/push_notification.rb b/app/jobs/regular/push_notification.rb index 6773bf4585a..16026323e74 100644 --- a/app/jobs/regular/push_notification.rb +++ b/app/jobs/regular/push_notification.rb @@ -4,7 +4,7 @@ module Jobs class PushNotification < ::Jobs::Base def execute(args) 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") payload = { diff --git a/spec/requests/topics_controller_spec.rb b/spec/requests/topics_controller_spec.rb index 4714d4d44b8..74749f3240f 100644 --- a/spec/requests/topics_controller_spec.rb +++ b/spec/requests/topics_controller_spec.rb @@ -1645,7 +1645,7 @@ RSpec.describe TopicsController do end end - it 'correctly renders canoicals' do + it 'correctly renders canonicals' do get "/t/#{topic.id}", params: { slug: topic.slug } expect(response.status).to eq(200) diff --git a/spec/services/post_alerter_spec.rb b/spec/services/post_alerter_spec.rb index 1ee1ac93b15..c70d47815f0 100644 --- a/spec/services/post_alerter_spec.rb +++ b/spec/services/post_alerter_spec.rb @@ -768,6 +768,7 @@ describe PostAlerter do { status: 200, body: "OK" } end + set_subfolder "/subpath" payload = { "secret_key" => SiteSetting.push_api_secret_key, "url" => Discourse.base_url, @@ -781,7 +782,7 @@ describe PostAlerter do 'topic_id' => topic.id, 'excerpt' => 'Hello @eviltrout ❤', 'username' => user.username, - 'url' => UrlHelper.absolute(mention_post.url), + 'url' => UrlHelper.absolute(Discourse.base_path + mention_post.url), 'client_id' => 'xxx0' }, { @@ -791,7 +792,7 @@ describe PostAlerter do 'topic_id' => topic.id, 'excerpt' => 'Hello @eviltrout ❤', 'username' => user.username, - 'url' => UrlHelper.absolute(mention_post.url), + 'url' => UrlHelper.absolute(Discourse.base_path + mention_post.url), 'client_id' => 'xxx1' } ] @@ -815,7 +816,7 @@ describe PostAlerter do "post_number" => new_post.post_number, "username" => new_post.user.username, "excerpt" => new_post.raw, - "url" => UrlHelper.absolute(new_post.url) + "url" => UrlHelper.absolute(Discourse.base_path + new_post.url) } payload["notifications"][0].merge! changes @@ -829,7 +830,7 @@ describe PostAlerter do "post_number" => new_post.post_number, "username" => new_post.user.username, "excerpt" => new_post.raw, - "url" => UrlHelper.absolute(new_post.url) + "url" => UrlHelper.absolute(Discourse.base_path + new_post.url) } payload["notifications"][0].merge! changes