DEV: Use cookie instead of URL param to show original content (#273)

Currently when the "Show Original" button on the right side of the page is clicked, we append the param `?show=original` and trigger a refresh to show the original user-written post content.

With this commit, we will be using cookies (`discourse-translator-show-original`) instead. This cookie is applied site-wide, not per-topic.

Author: nattsw

assets/javascripts/discourse/components/show-original-content.gjs

@@ -4,6 +4,10 @@ import { action } from "@ember/object";
 import { service } from "@ember/service";
 import DButton from "discourse/components/d-button";
 import concatClass from "discourse/helpers/concat-class";
+import cookie, { removeCookie } from "discourse/lib/cookie";
+
+const SHOW_ORIGINAL_COOKIE = "discourse-translator-show-original";
+const SHOW_ORIGINAL_COOKIE_EXPIRY = 30;
 
 export default class ShowOriginalContent extends Component {
   static shouldRender(args) {
@@ -15,38 +19,42 @@ export default class ShowOriginalContent extends Component {
 
   @service router;
 
-  @tracked isTranslated = true;
+  @tracked showingOriginal = false;
 
   constructor() {
     super(...arguments);
-    this.isTranslated = !new URLSearchParams(window.location.search).has(
-      "show"
-    );
+    this.showingOriginal = cookie(SHOW_ORIGINAL_COOKIE);
   }
 
   @action
   async showOriginal() {
-    const params = new URLSearchParams(window.location.search);
-    if (this.isTranslated) {
-      params.append("show", "original");
+    if (this.showingOriginal) {
+      removeCookie(SHOW_ORIGINAL_COOKIE, { path: "/" });
     } else {
-      params.delete("show");
+      cookie(SHOW_ORIGINAL_COOKIE, true, {
+        path: "/",
+        expires: SHOW_ORIGINAL_COOKIE_EXPIRY,
+      });
     }
-    window.location.search = params.toString();
+
+    this.router.refresh();
   }
 
   get title() {
-    return this.isTranslated
-      ? "translator.content_translated"
-      : "translator.content_not_translated";
+    return this.showingOriginal
+      ? "translator.content_not_translated"
+      : "translator.content_translated";
   }
 
   <template>
     <div class="discourse-translator_toggle-original">
       <DButton
         @icon="language"
         @title={{this.title}}
-        class={{concatClass "btn btn-default" (if this.isTranslated "active")}}
+        class={{concatClass
+          "btn btn-default"
+          (unless this.showingOriginal "active")
+        }}
         @action={{this.showOriginal}}
       />
     </div>

lib/discourse_translator/inline_translation.rb

@@ -10,15 +10,21 @@ def self.effective_locale
       end
     end
 
+    SHOW_ORIGINAL_COOKIE = "discourse-translator-show-original"
+
     def inject(plugin)
+      plugin.register_anonymous_cache_key :showoriginal do
+        @request.cookies[SHOW_ORIGINAL_COOKIE].present? ? "1" : "0"
+      end
+
       # since locales are eager loaded but translations may not,
       # always return early if topic and posts are in the user's effective_locale.
       # this prevents the need to load translations.
 
       plugin.register_modifier(:basic_post_serializer_cooked) do |cooked, serializer|
         if !SiteSetting.experimental_inline_translation ||
              serializer.object.locale_matches?(InlineTranslation.effective_locale) ||
-             serializer.scope&.request&.params&.[]("show") == "original"
+             serializer.scope&.request&.cookies&.key?(SHOW_ORIGINAL_COOKIE)
           cooked
         else
           serializer.object.translation_for(InlineTranslation.effective_locale).presence
@@ -28,7 +34,7 @@ def inject(plugin)
       plugin.register_modifier(:topic_serializer_fancy_title) do |fancy_title, serializer|
         if !SiteSetting.experimental_inline_translation ||
              serializer.object.locale_matches?(InlineTranslation.effective_locale) ||
-             serializer.scope&.request&.params&.[]("show") == "original"
+             serializer.scope&.request&.cookies&.key?(SHOW_ORIGINAL_COOKIE)
           fancy_title
         else
           serializer
@@ -42,7 +48,7 @@ def inject(plugin)
       plugin.register_modifier(:topic_view_serializer_fancy_title) do |fancy_title, serializer|
         if !SiteSetting.experimental_inline_translation ||
              serializer.object.topic.locale_matches?(InlineTranslation.effective_locale) ||
-             serializer.scope&.request&.params&.[]("show") == "original"
+             serializer.scope&.request&.cookies&.key?(SHOW_ORIGINAL_COOKIE)
           fancy_title
         else
           serializer

spec/serializers/basic_topic_serializer_spec.rb

@@ -25,8 +25,8 @@
       SiteSetting.automatic_translation_target_languages = "ja"
     end
 
-    def serialize_topic(guardian_user: user, params: {})
-      env = { "action_dispatch.request.parameters" => params, "REQUEST_METHOD" => "GET" }
+    def serialize_topic(guardian_user: user, cookie: "")
+      env = create_request_env.merge("HTTP_COOKIE" => cookie)
       request = ActionDispatch::Request.new(env)
       guardian = Guardian.new(guardian_user, request)
       BasicTopicSerializer.new(topic, scope: guardian)
@@ -41,7 +41,11 @@ def serialize_topic(guardian_user: user, params: {})
 
     it "does not replace fancy_title with translation when show_original param is present" do
       topic.set_translation("ja", jap_title)
-      expect(serialize_topic(params: { "show" => "original" }).fancy_title).to eq(topic.fancy_title)
+      expect(
+        serialize_topic(
+          cookie: DiscourseTranslator::InlineTranslation::SHOW_ORIGINAL_COOKIE,
+        ).fancy_title,
+      ).to eq(topic.fancy_title)
     end
 
     it "does not replace fancy_title with translation when no translation exists" do

spec/serializers/post_serializer_spec.rb

@@ -114,8 +114,8 @@
   end
 
   describe "#cooked" do
-    def serialize_post(guardian_user: user, params: {})
-      env = { "action_dispatch.request.parameters" => params, "REQUEST_METHOD" => "GET" }
+    def serialize_post(guardian_user: user, cookie: "")
+      env = create_request_env.merge("HTTP_COOKIE" => cookie)
       request = ActionDispatch::Request.new(env)
       guardian = Guardian.new(guardian_user, request)
       PostSerializer.new(post, scope: guardian)
@@ -131,14 +131,16 @@ def serialize_post(guardian_user: user, params: {})
       expect(serialize_post.cooked).to eq(post.cooked)
     end
 
-    it "does not return translated_cooked when show=original param is present" do
+    it "does not return translated_cooked when show original translation cookie is present" do
       I18n.locale = "ja"
       SiteSetting.automatic_translation_backfill_rate = 1
       SiteSetting.automatic_translation_target_languages = "ja"
       post.set_translation("ja", "こんにちは")
 
-      expect(serialize_post(params: { "show" => "original" }).cooked).to eq(post.cooked)
-      expect(serialize_post(params: { "show" => "derp" }).cooked).to eq("こんにちは")
+      expect(
+        serialize_post(cookie: DiscourseTranslator::InlineTranslation::SHOW_ORIGINAL_COOKIE).cooked,
+      ).to eq(post.cooked)
+      expect(serialize_post(cookie: "derp").cooked).to eq("こんにちは")
     end
 
     it "does not return translated_cooked when post is already in correct locale" do

spec/system/full_page_translation_spec.rb

@@ -14,6 +14,7 @@
   end
 
   let(:topic_page) { PageObjects::Pages::Topic.new }
+  let(:topic_list) { PageObjects::Components::TopicList.new }
 
   before do
     # topic translation setup
@@ -53,14 +54,21 @@
       visit("/")
       visit("/t/#{topic.id}")
       expect(topic_page.has_topic_title?("孫子兵法からの人生戦略")).to eq(true)
+    end
+
+    it "shows original content when 'Show Original' is selected" do
+      sign_in(japanese_user)
 
+      visit("/")
+      topic_list.visit_topic_with_title("孫子兵法からの人生戦略")
+
+      expect(topic_page.has_topic_title?("孫子兵法からの人生戦略")).to eq(true)
       page.find(".discourse-translator_toggle-original button").click
-      expect(page).to have_current_path(/.*show=original.*/)
 
       expect(topic_page.has_topic_title?("Life strategies from The Art of War")).to eq(true)
-      expect(find(topic_page.post_by_number_selector(1))).to have_content(
-        "The masterpiece isn’t just about military strategy",
-      )
+
+      visit("/")
+      topic_list.visit_topic_with_title("Life strategies from The Art of War")
     end
   end
 end