FEATURE: opt-in guidance on topics for users without access (#7852)

Co-Authored-By: majakomel <maja.komel@gmail.com>
Co-Authored-By: Robin Ward <robin.ward@gmail.com>

app/assets/javascripts/discourse/components/topic-join-group-notice.js.es6

@@ -0,0 +1,17 @@
+import { default as computed } from "ember-addons/ember-computed-decorators";
+
+export default Ember.Component.extend({
+  classNames: ["topic-notice"],
+
+  @computed("model.group.{full_name,name,allow_membership_requests}")
+  accessViaGroupText(group) {
+    const name = group.full_name || group.name;
+    const suffix = group.allow_membership_requests ? "request" : "join";
+    return I18n.t(`topic.group_${suffix}`, { name });
+  },
+
+  @computed("model.group.allow_membership_requests")
+  accessViaGroupButtonText(allowRequest) {
+    return `groups.${allowRequest ? "request" : "join"}`;
+  }
+});

app/assets/javascripts/discourse/controllers/topic.js.es6

@@ -941,6 +941,46 @@ export default Ember.Controller.extend(bufferedProperty("model"), {
       }
     },
 
+    joinGroup() {
+      const groupId = this.get("model.group.id");
+      if (groupId) {
+        if (this.get("model.group.allow_membership_requests")) {
+          const groupName = this.get("model.group.name");
+          return ajax(`/groups/${groupName}/request_membership`, {
+            type: "POST",
+            data: {
+              topic_id: this.get("model.id")
+            }
+          })
+            .then(() => {
+              bootbox.alert(
+                I18n.t("topic.group_request_sent", {
+                  group_name: this.get("model.group.full_name")
+                }),
+                () =>
+                  this.previousURL
+                    ? DiscourseURL.routeTo(this.previousURL)
+                    : DiscourseURL.routeTo("/")
+              );
+            })
+            .catch(popupAjaxError);
+        } else {
+          const topic = this.model;
+          return ajax(`/groups/${groupId}/members`, {
+            type: "PUT",
+            data: { user_id: this.get("currentUser.id") }
+          })
+            .then(() =>
+              topic.reload().then(() => {
+                topic.set("view_hidden", false);
+                topic.postStream.refresh();
+              })
+            )
+            .catch(popupAjaxError);
+        }
+      }
+    },
+
     replyAsNewTopic(post, quotedText) {
       const composerController = this.composer;
 

app/assets/javascripts/discourse/models/topic.js.es6

@@ -481,12 +481,12 @@ const Topic = RestModel.extend({
 
   // Update our attributes from a JSON result
   updateFromJson(json) {
+    const keys = Object.keys(json);
+    if (!json.view_hidden) {
       this.details.updateFromJson(json.details);
 
-    const keys = Object.keys(json);
-    keys.removeObject("details");
-    keys.removeObject("post_stream");
-
+      keys.removeObjects(["details", "post_stream"]);
+    }
     keys.forEach(key => this.set(key, json[key]));
   },
 

app/assets/javascripts/discourse/routes/topic-from-params.js.es6

@@ -35,6 +35,11 @@ export default Discourse.Route.extend({
         // TODO we are seeing errors where closest post is null and this is exploding
         // we need better handling and logging for this condition.
 
+        // there are no closestPost for hidden topics
+        if (topic.view_hidden) {
+          return;
+        }
+
         // The post we requested might not exist. Let's find the closest post
         const closestPost = postStream.closestPostForPostNumber(
           params.nearPost || 1
@@ -76,5 +81,14 @@ export default Discourse.Route.extend({
           console.log("Could not view topic", e);
         }
       });
+  },
+
+  actions: {
+    willTransition() {
+      this.controllerFor("topic").set(
+        "previousURL",
+        document.location.pathname
+      );
+    }
   }
 });

app/assets/javascripts/discourse/templates/components/topic-join-group-notice.hbs

@@ -0,0 +1,5 @@
+{{accessViaGroupText}}
+{{d-button action=action
+    class="btn-primary topic-join-group"
+    icon="user-plus"
+    label=accessViaGroupButtonText}}

app/assets/javascripts/discourse/templates/topic.hbs

@@ -1,4 +1,7 @@
 {{#discourse-topic multiSelect=multiSelect enteredAt=enteredAt topic=model hasScrolled=hasScrolled}}
+  {{#if model.view_hidden}}
+    {{topic-join-group-notice model=model action=(action "joinGroup")}}
+  {{else}}
     {{#if model}}
       {{add-category-tag-classes category=model.category tags=model.tags}}
       <div class="container">
@@ -361,4 +364,5 @@
     {{#if embedQuoteButton}}
       {{quote-button quoteState=quoteState selectText=(action "selectText")}}
     {{/if}}
+  {{/if}}
 {{/discourse-topic}}

app/assets/stylesheets/desktop/topic.scss

@@ -52,7 +52,8 @@
   display: inline;
 }
 
-.topic-error {
+.topic-error,
+.topic-notice {
   padding: 18px;
   width: 60%;
   margin-left: auto;
@@ -61,7 +62,8 @@
   text-align: center;
   line-height: $line-height-medium;
 
-  .topic-retry {
+  .topic-retry,
+  .topic-join-group {
     display: block;
     margin-top: 28px;
     margin-left: auto;

app/assets/stylesheets/mobile/topic.scss

@@ -147,7 +147,8 @@
   }
 }
 
-.topic-error {
+.topic-error,
+.topic-notice {
   padding: 18px;
   width: 90%;
   margin-left: auto;
@@ -155,7 +156,8 @@
   font-size: $font-up-4;
   line-height: $line-height-medium;
 
-  .topic-retry {
+  .topic-retry,
+  .topic-join-group {
     display: block;
     margin-top: 20px;
     margin-left: auto;

app/controllers/groups_controller.rb

@@ -420,12 +420,18 @@ class GroupsController < ApplicationController
   end
 
   def request_membership
-    params.require(:reason)
+    params.require(:reason) if params[:topic_id].blank?
 
     group = find_group(:id)
 
+    if params[:topic_id] && topic = Topic.find_by_id(params[:topic_id])
+      reason = I18n.t("groups.view_hidden_topic_request_reason", group_name: group.name, topic_url: topic.url)
+    end
+
+    reason ||= params[:reason]
+
     begin
-      GroupRequest.create!(group: group, user: current_user, reason: params[:reason])
+      GroupRequest.create!(group: group, user: current_user, reason: reason)
     rescue ActiveRecord::RecordNotUnique => e
       return render json: failed_json.merge(error: I18n.t("groups.errors.already_requested_membership")), status: 409
     end
@@ -438,7 +444,7 @@ class GroupsController < ApplicationController
     )
 
     raw = <<~EOF
-      #{params[:reason]}
+      #{reason}
 
       ---
       <a href="#{Discourse.base_uri}/g/#{group.name}/requests">

app/controllers/topics_controller.rb

@@ -131,6 +131,9 @@ class TopicsController < ApplicationController
     perform_show_response
 
   rescue Discourse::InvalidAccess => ex
+    if !guardian.can_see_topic?(ex.obj) && guardian.can_get_access_to_topic?(ex.obj)
+      return perform_hidden_topic_show_response(ex.obj)
+    end
 
     if current_user
       # If the user can't see the topic, clean up notifications for it.
@@ -950,6 +953,19 @@ class TopicsController < ApplicationController
     end
   end
 
+  def perform_hidden_topic_show_response(topic)
+    respond_to do |format|
+      format.html do
+        @topic_view = nil
+        render :show
+      end
+
+      format.json do
+        render_serialized(topic, HiddenTopicViewSerializer, root: false)
+      end
+    end
+  end
+
   def render_topic_changes(dest_topic)
     if dest_topic.present?
       render json: { success: true, url: dest_topic.relative_url }

app/models/topic.rb

@@ -1384,6 +1384,15 @@ class Topic < ActiveRecord::Base
     end
   end
 
+  def access_topic_via_group
+    Group
+      .joins(:category_groups)
+      .where("category_groups.category_id = ?", self.category_id)
+      .where("groups.public_admission OR groups.allow_membership_requests")
+      .order(:allow_membership_requests)
+      .first
+  end
+
   private
 
   def invite_to_private_message(invited_by, target_user, guardian)

app/serializers/hidden_topic_view_serializer.rb

@@ -0,0 +1,15 @@
+# frozen_string_literal: true
+
+class HiddenTopicViewSerializer < ApplicationSerializer
+  attributes :view_hidden?
+
+  has_one :group, serializer: BasicGroupSerializer, root: false, embed: :objects
+
+  def view_hidden?
+    true
+  end
+
+  def group
+    object.access_topic_via_group
+  end
+end

app/views/topics/show.html.erb

@@ -1,10 +1,11 @@
-<h1 class="crawler-topic-title">
+<% if @topic_view %>
+  <h1 class="crawler-topic-title">
     <%= render_topic_title(@topic_view.topic) %>
-</h1>
+  </h1>
 
-<% @breadcrumbs = categories_breadcrumb(@topic_view.topic)
+  <% @breadcrumbs = categories_breadcrumb(@topic_view.topic)
       if @breadcrumbs.present? %>
-<div id='breadcrumbs'>
+  <div id='breadcrumbs'>
     <% @breadcrumbs.each_with_index do |c,i| %>
       <div id="breadcrumb-<%=i%>" itemscope itemtype="http://data-vocabulary.org/Breadcrumb"
         <%-if (i+1) < @breadcrumbs.length%>
@@ -16,10 +17,10 @@
         </a>
       </div>
     <% end %>
-</div>
-<% end %>
+  </div>
+  <% end %>
 
-<% if SiteSetting.tagging_enabled %>
+  <% if SiteSetting.tagging_enabled %>
     <% @tags = @topic_view.topic.tags %>
     <% if @tags.present? %>
       <div class='crawler-tags-list' itemscope itemtype='http://schema.org/DiscussionForumPosting'>
@@ -32,13 +33,13 @@
         <% end %>
       </div>
     <% end %>
-<% end %>
+  <% end %>
 
-<%= server_plugin_outlet "topic_header" %>
+  <%= server_plugin_outlet "topic_header" %>
 
-<%- if include_crawler_content? %>
+  <%- if include_crawler_content? %>
 
-<% @topic_view.posts.each do |post| %>
+  <% @topic_view.posts.each do |post| %>
     <div itemscope itemtype='http://schema.org/DiscussionForumPosting' class='topic-body crawler-post'>
       <% if (u = post.user) %>
         <div class='crawler-post-meta'>
@@ -103,9 +104,9 @@
 
       <% end %>
     </div>
-<% end %>
+  <% end %>
 
-<% if @topic_view.prev_page || @topic_view.next_page %>
+  <% if @topic_view.prev_page || @topic_view.next_page %>
     <div role='navigation' itemscope itemtype='http://schema.org/SiteNavigationElement' class="topic-body crawler-post">
       <% if @topic_view.prev_page %>
         <span itemprop='name'><%= link_to t(:prev_page), @topic_view.prev_page_path, rel: 'prev', itemprop: 'url' %></span>
@@ -114,11 +115,11 @@
         <span itemprop='name'><b><%= link_to t(:next_page), @topic_view.next_page_path, rel: 'next', itemprop: 'url' %></b></span>
       <% end %>
     </div>
-<% end %>
+  <% end %>
 
-<% end %>
+  <% end %>
 
-<% content_for :head do %>
+  <% content_for :head do %>
     <%= auto_discovery_link_tag(@topic_view, {action: :feed, slug: @topic_view.topic.slug, topic_id: @topic_view.topic.id}, title: t('rss_posts_in_topic', topic: @topic_view.title), type: 'application/rss+xml') %>
     <%= raw crawlable_meta_data(title: @topic_view.title, description: @topic_view.summary(strip_images: true), image: @topic_view.image_url, read_time: @topic_view.read_time, like_count: @topic_view.like_count, ignore_canonical: true, published_time: @topic_view.published_time) %>
 
@@ -130,12 +131,13 @@
         <link rel="next" href="<%= @topic_view.next_page_path -%>">
       <% end %>
     <% end %>
-<% end %>
+  <% end %>
 
-<% content_for(:title) { @title || "#{gsub_emoji_to_unicode(@topic_view.page_title)} - #{SiteSetting.title}" } %>
+  <% content_for(:title) { @title || "#{gsub_emoji_to_unicode(@topic_view.page_title)} - #{SiteSetting.title}" } %>
 
-<% if @topic_view.print %>
+  <% if @topic_view.print %>
     <% content_for :after_body do %>
         <%= preload_script('print-page') %>
     <% end %>
+  <% end %>
 <% end %>

config/locales/client.en.yml

@@ -1967,6 +1967,9 @@ en:
       toggle_information: "toggle topic details"
       read_more_in_category: "Want to read more? Browse other topics in {{catLink}} or {{latestLink}}."
       read_more: "Want to read more? {{catLink}} or {{latestLink}}."
+      group_request: "You need to request membership to the `{{name}}` group to see this topic"
+      group_join: "You need join the `{{name}}` group to see this topic"
+      group_request_sent: "Your group membership request has been sent. You will be informed when it's accepted."
 
       # keys ending with _MF use message format, see https://meta.discourse.org/t/message-format-support-for-localization/7035 for details
       read_more_MF: "There {

config/locales/server.en.yml

@@ -381,6 +381,7 @@ en:
     request_membership_pm:
       title: "Membership Request for @%{group_name}"
       handle: "handle membership request"
+    view_hidden_topic_request_reason: "I would like to join the group '%{group_name}', so I may access [this topic](%{topic_url})"
 
   education:
     until_posts:

lib/guardian/topic_guardian.rb

@@ -159,6 +159,10 @@ module TopicGuardian
     can_see_topic?(topic, false)
   end
 
+  def can_get_access_to_topic?(topic)
+    topic&.access_topic_via_group.present? && authenticated?
+  end
+
   def filter_allowed_categories(records)
     unless is_admin?
       allowed_ids = allowed_category_ids

spec/models/topic_spec.rb

@@ -2414,4 +2414,31 @@ describe Topic do
       expect { topic.reset_bumped_at }.not_to change { topic.bumped_at }
     end
   end
+
+  describe "#access_topic_via_group" do
+    let(:open_group) { Fabricate(:group, public_admission: true) }
+    let(:request_group) do
+      Fabricate(:group).tap do |g|
+        g.add_owner(user)
+        g.allow_membership_requests = true
+        g.save!
+      end
+    end
+    let(:category) { Fabricate(:category) }
+    let(:topic) { Fabricate(:topic, category: category) }
+
+    it "returns a group that is open or accepts membership requests and has access to the topic" do
+      expect(topic.access_topic_via_group).to eq(nil)
+
+      category.set_permissions(request_group => :full)
+      category.save!
+
+      expect(topic.access_topic_via_group).to eq(request_group)
+
+      category.set_permissions(request_group => :full, open_group => :full)
+      category.save!
+
+      expect(topic.access_topic_via_group).to eq(open_group)
+    end
+  end
 end

spec/requests/topics_controller_spec.rb

@@ -1279,6 +1279,7 @@ RSpec.describe TopicsController do
     context 'permission errors' do
       fab!(:allowed_user) { Fabricate(:user) }
       let(:allowed_group) { Fabricate(:group) }
+      let(:accessible_group) { Fabricate(:group, public_admission: true) }
       let(:secure_category) do
         c = Fabricate(:category)
         c.permissions = [[allowed_group, :full]]
@@ -1287,6 +1288,12 @@ RSpec.describe TopicsController do
         allowed_user.save
         c
       end
+      let(:accessible_category) do
+        Fabricate(:category).tap do |c|
+          c.set_permissions(accessible_group => :full)
+          c.save!
+        end
+      end
       let(:normal_topic) { Fabricate(:topic) }
       let(:secure_topic) { Fabricate(:topic, category: secure_category) }
       let(:private_topic) { Fabricate(:private_message_topic, user: allowed_user) }
@@ -1294,6 +1301,7 @@ RSpec.describe TopicsController do
       let(:deleted_secure_topic) { Fabricate(:topic, category: secure_category, deleted_at: 1.day.ago) }
       let(:deleted_private_topic) { Fabricate(:private_message_topic, user: allowed_user, deleted_at: 1.day.ago) }
       let(:nonexist_topic_id) { Topic.last.id + 10000 }
+      let(:secure_accessible_topic) { Fabricate(:topic, category: accessible_category) }
 
       shared_examples "various scenarios" do |expected|
         expected.each do |key, value|
@@ -1314,7 +1322,8 @@ RSpec.describe TopicsController do
           deleted_topic: 410,
           deleted_secure_topic: 403,
           deleted_private_topic: 403,
-          nonexist: 404
+          nonexist: 404,
+          secure_accessible_topic: 403
         }
         include_examples "various scenarios", expected
       end
@@ -1330,7 +1339,8 @@ RSpec.describe TopicsController do
           deleted_topic: 302,
           deleted_secure_topic: 302,
           deleted_private_topic: 302,
-          nonexist: 302
+          nonexist: 302,
+          secure_accessible_topic: 302
         }
         include_examples "various scenarios", expected
       end
@@ -1347,7 +1357,8 @@ RSpec.describe TopicsController do
           deleted_topic: 410,
           deleted_secure_topic: 403,
           deleted_private_topic: 403,
-          nonexist: 404
+          nonexist: 404,
+          secure_accessible_topic: 200
         }
         include_examples "various scenarios", expected
       end
@@ -1364,7 +1375,8 @@ RSpec.describe TopicsController do
           deleted_topic: 410,
           deleted_secure_topic: 410,
           deleted_private_topic: 410,
-          nonexist: 404
+          nonexist: 404,
+          secure_accessible_topic: 200
         }
         include_examples "various scenarios", expected
       end
@@ -1381,7 +1393,8 @@ RSpec.describe TopicsController do
           deleted_topic: 200,
           deleted_secure_topic: 403,
           deleted_private_topic: 403,
-          nonexist: 404
+          nonexist: 404,
+          secure_accessible_topic: 200
         }
         include_examples "various scenarios", expected
       end
@@ -1398,7 +1411,8 @@ RSpec.describe TopicsController do
           deleted_topic: 200,
           deleted_secure_topic: 200,
           deleted_private_topic: 200,
-          nonexist: 404
+          nonexist: 404,
+          secure_accessible_topic: 200
         }
         include_examples "various scenarios", expected
       end