From 9da3a7f436b4c6b64e65a425ffcc27964019241c Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Wed, 17 Jun 2020 12:42:20 +0200 Subject: [PATCH] FEATURE: allows published pages to be public (#10053) --- .../discourse/app/controllers/publish-page.js | 17 ++++-- .../app/templates/modal/publish-page.hbs | 32 +++++++++-- .../discourse/app/templates/topic.hbs | 3 + app/assets/stylesheets/common/base/modal.scss | 49 +++++++++++----- app/assets/stylesheets/common/base/topic.scss | 9 +++ app/controllers/published_pages_controller.rb | 37 +++++++++++- app/models/published_page.rb | 4 +- app/serializers/published_page_serializer.rb | 2 +- config/locales/client.en.yml | 3 + config/locales/server.en.yml | 1 + config/site_settings.yml | 2 + ...701_add_public_field_to_published_pages.rb | 7 +++ spec/fabricators/published_page_fabricator.rb | 3 +- .../published_pages_controller_spec.rb | 57 ++++++++++++++++++- 14 files changed, 196 insertions(+), 30 deletions(-) create mode 100644 db/migrate/20201006172701_add_public_field_to_published_pages.rb diff --git a/app/assets/javascripts/discourse/app/controllers/publish-page.js b/app/assets/javascripts/discourse/app/controllers/publish-page.js index f6c864682a4..58059d027c1 100644 --- a/app/assets/javascripts/discourse/app/controllers/publish-page.js +++ b/app/assets/javascripts/discourse/app/controllers/publish-page.js @@ -35,6 +35,7 @@ export default Controller.extend(ModalFunctionality, StateHelpers, { this.state === States.existing ); }), + showUnpublish: computed("state", function() { return this.state === States.existing || this.state === States.unpublishing; }), @@ -95,7 +96,7 @@ export default Controller.extend(ModalFunctionality, StateHelpers, { this.set("state", States.saving); return this.publishedPage - .update({ slug: this.publishedPage.slug }) + .update(this.publishedPage.getProperties("slug", "public")) .then(() => { this.set("state", States.existing); this.model.set("publishedPage", this.publishedPage); @@ -110,11 +111,17 @@ export default Controller.extend(ModalFunctionality, StateHelpers, { startNew() { this.setProperties({ state: States.new, - publishedPage: this.store.createRecord("published_page", { - id: this.model.id, - slug: this.model.slug - }) + publishedPage: this.store.createRecord( + "published_page", + this.model.getProperties("id", "slug", "public") + ) }); this.checkSlug(); + }, + + @action + onChangePublic(isPublic) { + this.publishedPage.set("public", isPublic); + this.publish(); } }); diff --git a/app/assets/javascripts/discourse/app/templates/modal/publish-page.hbs b/app/assets/javascripts/discourse/app/templates/modal/publish-page.hbs index 712682fe74e..9cd9a560879 100644 --- a/app/assets/javascripts/discourse/app/templates/modal/publish-page.hbs +++ b/app/assets/javascripts/discourse/app/templates/modal/publish-page.hbs @@ -6,8 +6,29 @@

{{i18n "topic.publish_page.description"}}

- - {{text-field value=publishedPage.slug onChange=(action "checkSlug") onChangeImmediate=(action "startCheckSlug") disabled=existing class="publish-slug"}} +
+ + {{text-field + value=publishedPage.slug + onChange=(action "checkSlug") + onChangeImmediate=(action "startCheckSlug") + disabled=existing + class="publish-slug" + }} +
+ +
+ + +

+ {{input + type="checkbox" + checked=publishedPage.public + click=(action "onChangePublic" value="target.checked") + }} + {{i18n "topic.publish_page.public_description"}} +

+
@@ -40,14 +61,15 @@
{{#if showUnpublish}} - {{d-button icon="times" label="close" action=(action "closeModal")}} - {{d-button label="topic.publish_page.unpublish" icon="trash-alt" class="btn-danger" isLoading=unpublishing - action=(action "unpublish") }} + action=(action "unpublish") + }} + + {{d-button class="close-publish-page" icon="times" label="close" action=(action "closeModal")}} {{else if unpublished}} {{d-button label="topic.publish_page.publishing_settings" action=(action "startNew")}} {{else}} diff --git a/app/assets/javascripts/discourse/app/templates/topic.hbs b/app/assets/javascripts/discourse/app/templates/topic.hbs index e6968f11b76..33f11793a80 100644 --- a/app/assets/javascripts/discourse/app/templates/topic.hbs +++ b/app/assets/javascripts/discourse/app/templates/topic.hbs @@ -90,6 +90,9 @@ {{#if model.publishedPage}}
+ {{#if model.publishedPage.public}} + {{i18n "topic.publish_page.public"}} + {{/if}} {{i18n "topic.publish_page.topic_published"}}
{{model.publishedPage.url}} diff --git a/app/assets/stylesheets/common/base/modal.scss b/app/assets/stylesheets/common/base/modal.scss index a15a7cf7c08..4e1a7946a15 100644 --- a/app/assets/stylesheets/common/base/modal.scss +++ b/app/assets/stylesheets/common/base/modal.scss @@ -689,24 +689,45 @@ } } -.publish-page-modal .modal-body { - p.publish-description { - margin-top: 0; - } - input.publish-slug { - width: 100%; - } +.publish-page-modal { + .modal-body { + p.publish-description { + margin-top: 0; + } + input.publish-slug { + width: 100%; + } - .publish-url { - margin-bottom: 1em; - .example-url, - .invalid-slug { - font-weight: bold; + .publish-url { + margin-bottom: 1em; + .example-url, + .invalid-slug { + font-weight: bold; + } + } + + .publish-slug:disabled { + cursor: not-allowed; + } + + .controls { + margin-bottom: 1em; + + .description { + margin: 0; + display: flex; + align-items: center; + } } } - .publish-slug:disabled { - cursor: not-allowed; + .modal-footer { + display: flex; + + .close-publish-page { + margin-left: auto; + margin-right: 0; + } } } diff --git a/app/assets/stylesheets/common/base/topic.scss b/app/assets/stylesheets/common/base/topic.scss index 02b267ab134..eee0faf51be 100644 --- a/app/assets/stylesheets/common/base/topic.scss +++ b/app/assets/stylesheets/common/base/topic.scss @@ -305,4 +305,13 @@ a.topic-featured-link { 2} ); align-items: center; + + .is-public { + padding: 0.25em 0.5em; + font-size: $font-down-2; + background: $tertiary; + color: $secondary; + border-radius: 3px; + text-transform: lowercase; + } } diff --git a/app/controllers/published_pages_controller.rb b/app/controllers/published_pages_controller.rb index 67d3fb8bdab..f61a0b00b2b 100644 --- a/app/controllers/published_pages_controller.rb +++ b/app/controllers/published_pages_controller.rb @@ -5,6 +5,7 @@ class PublishedPagesController < ApplicationController skip_before_action :preload_json skip_before_action :check_xhr, :verify_authenticity_token, only: [:show] before_action :ensure_publish_enabled + before_action :redirect_to_login_if_required, except: [:show] def show params.require(:slug) @@ -12,7 +13,22 @@ class PublishedPagesController < ApplicationController pp = PublishedPage.find_by(slug: params[:slug]) raise Discourse::NotFound unless pp - guardian.ensure_can_see!(pp.topic) + return if enforce_login_required! + + if !pp.public + begin + guardian.ensure_can_see!(pp.topic) + rescue Discourse::InvalidAccess => e + return rescue_discourse_actions( + :invalid_access, + 403, + include_ember: false, + custom_message: e.custom_message, + group: e.group + ) + end + end + @topic = pp.topic @canonical_url = @topic.url @@ -37,7 +53,15 @@ class PublishedPagesController < ApplicationController end def upsert - result, pp = PublishedPage.publish!(current_user, fetch_topic, params[:published_page][:slug].strip) + pp_params = params.require(:published_page) + + result, pp = PublishedPage.publish!( + current_user, + fetch_topic, + pp_params[:slug].strip, + pp_params.permit(:public) + ) + json_result(pp, serializer: PublishedPageSerializer) { result } end @@ -68,4 +92,13 @@ private raise Discourse::NotFound unless SiteSetting.enable_page_publishing? end + def enforce_login_required! + if SiteSetting.login_required? && + !current_user && + !SiteSetting.show_published_pages_login_required? && + redirect_to_login + true + end + end + end diff --git a/app/models/published_page.rb b/app/models/published_page.rb index 648f6b8b50c..5322efaea76 100644 --- a/app/models/published_page.rb +++ b/app/models/published_page.rb @@ -23,12 +23,13 @@ class PublishedPage < ActiveRecord::Base "#{Discourse.base_url}#{path}" end - def self.publish!(publisher, topic, slug) + def self.publish!(publisher, topic, slug, options = {}) pp = nil transaction do pp = find_or_initialize_by(topic: topic) pp.slug = slug.strip + pp.public = options[:public] || false if pp.save StaffActionLogger.new(publisher).log_published_page(topic.id, slug) @@ -56,6 +57,7 @@ end # slug :string not null # created_at :datetime not null # updated_at :datetime not null +# public :boolean default(FALSE), not null # # Indexes # diff --git a/app/serializers/published_page_serializer.rb b/app/serializers/published_page_serializer.rb index bdee8d14f70..c0b209ae0fd 100644 --- a/app/serializers/published_page_serializer.rb +++ b/app/serializers/published_page_serializer.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class PublishedPageSerializer < ApplicationSerializer - attributes :id, :slug + attributes :id, :slug, :public def id object.topic_id diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 2f3a8b00957..7e368a8d456 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -2513,11 +2513,14 @@ en: publish: "Publish" description: "When a topic is published as a page, its URL can be shared and it will displayed with custom styling." slug: "Slug" + public: "Public" + public_description: "People can see the page even if the associated topic is private." publish_url: "Your page has been published at:" topic_published: "Your topic has been published at:" preview_url: "Your page will be published at:" invalid_slug: "Sorry, you can't publish this page." unpublish: "Unpublish" + update: "Update" unpublished: "Your page has been unpublished and is no longer accessible." publishing_settings: "Publishing Settings" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index d2a0a9b10b1..c839d7b8c8e 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -2131,6 +2131,7 @@ en: returning_user_notice_tl: "Minimum trust level required to see returning user post notices." returning_users_days: "How many days should pass before a user is considered to be returning." enable_page_publishing: "Allow staff members to publish topics to new URLs with their own styling." + show_published_pages_login_required: "Anonymous users can see published pages, even when login is required." default_email_digest_frequency: "How often users receive summary emails by default." default_include_tl0_in_digests: "Include posts from new users in summary emails by default. Users can change this in their preferences." diff --git a/config/site_settings.yml b/config/site_settings.yml index 08e16c94f1f..2948b8793f2 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -947,6 +947,8 @@ posting: max: 36500 enable_page_publishing: default: false + show_published_pages_login_required: + default: false email: email_time_window_mins: diff --git a/db/migrate/20201006172701_add_public_field_to_published_pages.rb b/db/migrate/20201006172701_add_public_field_to_published_pages.rb new file mode 100644 index 00000000000..d02edd2bd9d --- /dev/null +++ b/db/migrate/20201006172701_add_public_field_to_published_pages.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddPublicFieldToPublishedPages < ActiveRecord::Migration[6.0] + def change + add_column :published_pages, :public, :boolean, null: false, default: false + end +end diff --git a/spec/fabricators/published_page_fabricator.rb b/spec/fabricators/published_page_fabricator.rb index 44f8da98da2..0c090ee5d97 100644 --- a/spec/fabricators/published_page_fabricator.rb +++ b/spec/fabricators/published_page_fabricator.rb @@ -2,5 +2,6 @@ Fabricator(:published_page) do topic - slug "published-page-test" + slug "published-page-test-#{SecureRandom.hex}" + public false end diff --git a/spec/requests/published_pages_controller_spec.rb b/spec/requests/published_pages_controller_spec.rb index 67937617912..0662554989b 100644 --- a/spec/requests/published_pages_controller_spec.rb +++ b/spec/requests/published_pages_controller_spec.rb @@ -59,6 +59,15 @@ RSpec.describe PublishedPagesController do expect(response.status).to eq(403) end + context "published page is public" do + fab!(:public_published_page) { Fabricate(:published_page, public: true) } + + it "returns 200 for a topic you can't see" do + get public_published_page.path + expect(response.status).to eq(200) + end + end + context "as an admin" do before do sign_in(admin) @@ -89,7 +98,42 @@ RSpec.describe PublishedPagesController do it "defines correct css classes on body" do get published_page.path - expect(response.body).to include("") + expect(response.body).to include("") + end + + context "login is required" do + before do + SiteSetting.login_required = true + SiteSetting.show_published_pages_login_required = false + end + + context "a user is connected" do + before do + sign_in(user) + end + + it "returns 200" do + get published_page.path + expect(response.status).to eq(200) + end + end + + context "no user connected" do + it "redirects to login page" do + expect(get(published_page.path)).to redirect_to("/login") + end + + context "show public pages with login required is enabled" do + before do + SiteSetting.show_published_pages_login_required = true + end + + it "returns 200" do + get published_page.path + expect(response.status).to eq(200) + end + end + end end end end @@ -113,6 +157,7 @@ RSpec.describe PublishedPagesController do expect(response).to be_successful expect(response.parsed_body['published_page']).to be_present expect(response.parsed_body['published_page']['slug']).to eq("i-hate-salt") + expect(response.parsed_body['published_page']['public']).to eq(false) expect(PublishedPage.exists?(topic_id: response.parsed_body['published_page']['id'])).to eq(true) expect(UserHistory.exists?( @@ -122,6 +167,16 @@ RSpec.describe PublishedPagesController do )).to be(true) end + it "allows to set public field" do + put "/pub/by-topic/#{topic.id}.json", params: { published_page: { slug: 'i-hate-salt', public: true } } + expect(response).to be_successful + expect(response.parsed_body['published_page']).to be_present + expect(response.parsed_body['published_page']['slug']).to eq("i-hate-salt") + expect(response.parsed_body['published_page']['public']).to eq(true) + + expect(PublishedPage.exists?(topic_id: response.parsed_body['published_page']['id'])).to eq(true) + end + it "returns an error if the slug is already taken" do PublishedPage.create!(slug: 'i-hate-salt', topic: Fabricate(:topic)) put "/pub/by-topic/#{topic.id}.json", params: { published_page: { slug: 'i-hate-salt' } }