From a5d9afe397990fafd1616458366b35ac6f010cbe Mon Sep 17 00:00:00 2001 From: Robin Ward Date: Fri, 12 Apr 2019 09:55:27 -0400 Subject: [PATCH] FEATURE: Include a user's pending posts in the topic view Also includes a refactor to TopicView's serializer which was not building our attributes using serializers properly. --- .../discourse/controllers/composer.js.es6 | 7 + .../discourse/controllers/topic.js.es6 | 8 + .../javascripts/discourse/templates/topic.hbs | 35 ++++- app/assets/stylesheets/common/base/topic.scss | 14 ++ app/controllers/reviewables_controller.rb | 9 ++ app/models/reviewable.rb | 9 +- app/models/reviewable_flagged_post.rb | 9 +- app/models/reviewable_queued_post.rb | 34 +++-- app/models/reviewable_score.rb | 1 + app/models/reviewable_user.rb | 9 +- app/models/topic_link.rb | 35 +++-- app/serializers/new_post_result_serializer.rb | 12 +- .../topic_pending_post_serializer.rb | 12 ++ .../topic_view_details_serializer.rb | 140 ++++++++++++++++++ app/serializers/topic_view_serializer.rb | 95 ++---------- config/locales/client.en.yml | 2 + config/routes.rb | 1 + ...430_add_created_by_index_to_reviewables.rb | 5 + lib/guardian.rb | 6 + lib/topic_view.rb | 57 ++++++- spec/requests/posts_controller_spec.rb | 4 + spec/requests/reviewables_controller_spec.rb | 31 ++++ .../serializers/topic_view_serializer_spec.rb | 85 ++++++++++- .../acceptance/composer-test.js.es6 | 4 + test/javascripts/fixtures/topic.js.es6 | 1 + .../helpers/create-pretender.js.es6 | 9 +- 26 files changed, 503 insertions(+), 131 deletions(-) create mode 100644 app/serializers/topic_pending_post_serializer.rb create mode 100644 app/serializers/topic_view_details_serializer.rb create mode 100644 db/migrate/20190412161430_add_created_by_index_to_reviewables.rb diff --git a/app/assets/javascripts/discourse/controllers/composer.js.es6 b/app/assets/javascripts/discourse/controllers/composer.js.es6 index 0641f1120ee..be239a81e73 100644 --- a/app/assets/javascripts/discourse/controllers/composer.js.es6 +++ b/app/assets/javascripts/discourse/controllers/composer.js.es6 @@ -679,6 +679,13 @@ export default Ember.Controller.extend({ .then(result => { if (result.responseJson.action === "enqueued") { this.send("postWasEnqueued", result.responseJson); + if (result.responseJson.pending_post) { + let pendingPosts = this.get("topicController.model.pending_posts"); + if (pendingPosts) { + pendingPosts.pushObject(result.responseJson.pending_post); + } + } + this.destroyDraft(); this.close(); this.appEvents.trigger("post-stream:refresh"); diff --git a/app/assets/javascripts/discourse/controllers/topic.js.es6 b/app/assets/javascripts/discourse/controllers/topic.js.es6 index a51c2dda9a6..7c0ba8b6e2f 100644 --- a/app/assets/javascripts/discourse/controllers/topic.js.es6 +++ b/app/assets/javascripts/discourse/controllers/topic.js.es6 @@ -202,6 +202,14 @@ export default Ember.Controller.extend(bufferedProperty("model"), { }, actions: { + deletePending(pending) { + return ajax(`/review/${pending.id}`, { type: "DELETE" }) + .then(() => { + this.get("model.pending_posts").removeObject(pending); + }) + .catch(popupAjaxError); + }, + showPostFlags(post) { return this.send("showFlags", post); }, diff --git a/app/assets/javascripts/discourse/templates/topic.hbs b/app/assets/javascripts/discourse/templates/topic.hbs index 9b09579936f..ae8c8e58d24 100644 --- a/app/assets/javascripts/discourse/templates/topic.hbs +++ b/app/assets/javascripts/discourse/templates/topic.hbs @@ -211,10 +211,41 @@ {{#conditional-loading-spinner condition=model.postStream.loadingFilter}} {{#if loadedAllPosts}} - {{#if model.pending_posts_count}} + {{#if model.pending_posts}} +
+ {{#each model.pending_posts as |pending|}} +
+
+ + {{i18n "review.awaiting_approval"}} + + + {{age-with-tooltip pending.created_at}} + +
+
+ {{reviewable-created-by user=currentUser tagName=''}} +
+ {{reviewable-created-by-name user=currentUser tagName=''}} +
{{cook-text pending.raw}}
+
+
+
+ {{d-button + class="btn-danger" + label="review.delete" + icon="trash-alt" + action=(action "deletePending" pending) }} +
+
+ {{/each}} +
+ {{/if}} + + {{#if model.queued_posts_count}}
- {{{i18n "review.topic_has_pending" count=model.pending_posts_count}}} + {{{i18n "review.topic_has_pending" count=model.queued_posts_count}}}
{{#link-to 'review' (query-params topic_id=model.id type="ReviewableQueuedPost" status="pending")}} diff --git a/app/assets/stylesheets/common/base/topic.scss b/app/assets/stylesheets/common/base/topic.scss index b54bca6097b..dd9fcb5e272 100644 --- a/app/assets/stylesheets/common/base/topic.scss +++ b/app/assets/stylesheets/common/base/topic.scss @@ -266,3 +266,17 @@ a.topic-featured-link { margin-right: 3px; } } + +.topic-area { + .pending-posts { + max-width: calc( + #{$topic-body-width} + #{$topic-avatar-width} + #{$topic-body-width-padding * + 2} + ); + .reviewable-item { + .post-body { + max-height: unset; + } + } + } +} diff --git a/app/controllers/reviewables_controller.rb b/app/controllers/reviewables_controller.rb index f91dcbf6cf2..3551c9ddecf 100644 --- a/app/controllers/reviewables_controller.rb +++ b/app/controllers/reviewables_controller.rb @@ -96,6 +96,15 @@ class ReviewablesController < ApplicationController ) end + def destroy + reviewable = Reviewable.find_by(id: params[:reviewable_id], created_by: current_user) + raise Discourse::NotFound.new if reviewable.blank? + + reviewable.perform(current_user, :delete) + + render json: success_json + end + def update reviewable = find_reviewable editable = reviewable.editable_for(guardian) diff --git a/app/models/reviewable.rb b/app/models/reviewable.rb index 5debd5ba4cf..5e9969a00d8 100644 --- a/app/models/reviewable.rb +++ b/app/models/reviewable.rb @@ -482,8 +482,9 @@ end # # Indexes # -# index_reviewables_on_status_and_created_at (status,created_at) -# index_reviewables_on_status_and_score (status,score) -# index_reviewables_on_status_and_type (status,type) -# index_reviewables_on_type_and_target_id (type,target_id) UNIQUE +# index_reviewables_on_status_and_created_at (status,created_at) +# index_reviewables_on_status_and_score (status,score) +# index_reviewables_on_status_and_type (status,type) +# index_reviewables_on_topic_id_and_status_and_created_by_id (topic_id,status,created_by_id) +# index_reviewables_on_type_and_target_id (type,target_id) UNIQUE # diff --git a/app/models/reviewable_flagged_post.rb b/app/models/reviewable_flagged_post.rb index 67ae263386a..be98034fb56 100644 --- a/app/models/reviewable_flagged_post.rb +++ b/app/models/reviewable_flagged_post.rb @@ -304,8 +304,9 @@ end # # Indexes # -# index_reviewables_on_status_and_created_at (status,created_at) -# index_reviewables_on_status_and_score (status,score) -# index_reviewables_on_status_and_type (status,type) -# index_reviewables_on_type_and_target_id (type,target_id) UNIQUE +# index_reviewables_on_status_and_created_at (status,created_at) +# index_reviewables_on_status_and_score (status,score) +# index_reviewables_on_status_and_type (status,type) +# index_reviewables_on_topic_id_and_status_and_created_by_id (topic_id,status,created_by_id) +# index_reviewables_on_type_and_target_id (type,target_id) UNIQUE # diff --git a/app/models/reviewable_queued_post.rb b/app/models/reviewable_queued_post.rb index c60fd5d9358..8f8bf123cb8 100644 --- a/app/models/reviewable_queued_post.rb +++ b/app/models/reviewable_queued_post.rb @@ -9,18 +9,20 @@ class ReviewableQueuedPost < Reviewable end def build_actions(actions, guardian, args) - return unless guardian.is_staff? + if guardian.is_staff? + actions.add(:approve) unless approved? + actions.add(:reject) unless rejected? - actions.add(:approve) unless approved? - actions.add(:reject) unless rejected? - - if pending? && guardian.can_delete_user?(created_by) - actions.add(:delete_user) do |action| - action.icon = 'trash-alt' - action.label = 'reviewables.actions.delete_user.title' - action.confirm_message = 'reviewables.actions.delete_user.confirm' + if pending? && guardian.can_delete_user?(created_by) + actions.add(:delete_user) do |action| + action.icon = 'trash-alt' + action.label = 'reviewables.actions.delete_user.title' + action.confirm_message = 'reviewables.actions.delete_user.confirm' + end end end + + actions.add(:delete) if guardian.can_delete?(self) end def build_editable_fields(fields, guardian, args) @@ -82,6 +84,11 @@ class ReviewableQueuedPost < Reviewable create_result(:success, :rejected) end + def perform_delete(performed_by, args) + create_result(:success, :deleted) + end + + def perform_delete_user(performed_by, args) delete_options = { context: I18n.t('reviewables.actions.delete_user.reason'), @@ -126,8 +133,9 @@ end # # Indexes # -# index_reviewables_on_status_and_created_at (status,created_at) -# index_reviewables_on_status_and_score (status,score) -# index_reviewables_on_status_and_type (status,type) -# index_reviewables_on_type_and_target_id (type,target_id) UNIQUE +# index_reviewables_on_status_and_created_at (status,created_at) +# index_reviewables_on_status_and_score (status,score) +# index_reviewables_on_status_and_type (status,type) +# index_reviewables_on_topic_id_and_status_and_created_by_id (topic_id,status,created_by_id) +# index_reviewables_on_type_and_target_id (type,target_id) UNIQUE # diff --git a/app/models/reviewable_score.rb b/app/models/reviewable_score.rb index 057afb5612b..144b5ac9d33 100644 --- a/app/models/reviewable_score.rb +++ b/app/models/reviewable_score.rb @@ -86,6 +86,7 @@ end # meta_topic_id :integer # created_at :datetime not null # updated_at :datetime not null +# reason :string # # Indexes # diff --git a/app/models/reviewable_user.rb b/app/models/reviewable_user.rb index 40185a35dcc..8feefedf612 100644 --- a/app/models/reviewable_user.rb +++ b/app/models/reviewable_user.rb @@ -87,8 +87,9 @@ end # # Indexes # -# index_reviewables_on_status_and_created_at (status,created_at) -# index_reviewables_on_status_and_score (status,score) -# index_reviewables_on_status_and_type (status,type) -# index_reviewables_on_type_and_target_id (type,target_id) UNIQUE +# index_reviewables_on_status_and_created_at (status,created_at) +# index_reviewables_on_status_and_score (status,score) +# index_reviewables_on_status_and_type (status,type) +# index_reviewables_on_topic_id_and_status_and_created_by_id (topic_id,status,created_by_id) +# index_reviewables_on_type_and_target_id (type,target_id) UNIQUE # diff --git a/app/models/topic_link.rb b/app/models/topic_link.rb index a7be7fa00c3..43c41d563e4 100644 --- a/app/models/topic_link.rb +++ b/app/models/topic_link.rb @@ -38,30 +38,29 @@ class TopicLink < ActiveRecord::Base def self.topic_map(guardian, topic_id) # Sam: complicated reports are really hard in AR - builder = DB.build <<-SQL - SELECT ftl.url, - COALESCE(ft.title, ftl.title) AS title, - ftl.link_topic_id, - ftl.reflection, - ftl.internal, - ftl.domain, - MIN(ftl.user_id) AS user_id, - SUM(clicks) AS clicks - FROM topic_links AS ftl - LEFT JOIN topics AS ft ON ftl.link_topic_id = ft.id - LEFT JOIN categories AS c ON c.id = ft.category_id - /*where*/ - GROUP BY ftl.url, ft.title, ftl.title, ftl.link_topic_id, ftl.reflection, ftl.internal, ftl.domain - ORDER BY clicks DESC, count(*) DESC - LIMIT 50 -SQL + builder = DB.build(<<~SQL) + SELECT ftl.url, + COALESCE(ft.title, ftl.title) AS title, + ftl.link_topic_id, + ftl.reflection, + ftl.internal, + ftl.domain, + MIN(ftl.user_id) AS user_id, + SUM(clicks) AS clicks + FROM topic_links AS ftl + LEFT JOIN topics AS ft ON ftl.link_topic_id = ft.id + LEFT JOIN categories AS c ON c.id = ft.category_id + /*where*/ + GROUP BY ftl.url, ft.title, ftl.title, ftl.link_topic_id, ftl.reflection, ftl.internal, ftl.domain + ORDER BY clicks DESC, count(*) DESC + LIMIT 50 + SQL builder.where('ftl.topic_id = :topic_id', topic_id: topic_id) builder.where('ft.deleted_at IS NULL') # note that ILIKE means "case insensitive LIKE" builder.where("NOT(ftl.url ILIKE '%.png' OR ftl.url ILIKE '%.jpg' OR ftl.url ILIKE '%.gif')") builder.where("COALESCE(ft.archetype, 'regular') <> :archetype", archetype: Archetype.private_message) - # do not show links with 0 click builder.where("clicks > 0") builder.secure_category(guardian.secure_category_ids) diff --git a/app/serializers/new_post_result_serializer.rb b/app/serializers/new_post_result_serializer.rb index 068f3a9c9d7..39da1ecfe25 100644 --- a/app/serializers/new_post_result_serializer.rb +++ b/app/serializers/new_post_result_serializer.rb @@ -8,6 +8,8 @@ class NewPostResultSerializer < ApplicationSerializer :pending_count, :reason + has_one :pending_post, serializer: TopicPendingPostSerializer, root: false, embed: :objects + def post post_serializer = PostSerializer.new(object.post, scope: scope, root: false) post_serializer.draft_sequence = DraftSequence.current(scope.user, object.post.topic.draft_key) @@ -39,7 +41,7 @@ class NewPostResultSerializer < ApplicationSerializer end def include_reason? - reason.present? + scope.is_staff? && reason.present? end def action @@ -50,6 +52,14 @@ class NewPostResultSerializer < ApplicationSerializer object.pending_count end + def pending_post + object.reviewable + end + + def include_pending_post? + object.reviewable.present? + end + def include_pending_count? pending_count.present? end diff --git a/app/serializers/topic_pending_post_serializer.rb b/app/serializers/topic_pending_post_serializer.rb new file mode 100644 index 00000000000..568f9b6eab4 --- /dev/null +++ b/app/serializers/topic_pending_post_serializer.rb @@ -0,0 +1,12 @@ +class TopicPendingPostSerializer < ApplicationSerializer + attributes :id, :raw, :created_at + + def raw + object.payload['raw'] + end + + def include_raw? + object.payload && object.payload['raw'].present? + end + +end diff --git a/app/serializers/topic_view_details_serializer.rb b/app/serializers/topic_view_details_serializer.rb new file mode 100644 index 00000000000..8ad7a74e9a9 --- /dev/null +++ b/app/serializers/topic_view_details_serializer.rb @@ -0,0 +1,140 @@ +class TopicViewDetailsSerializer < ApplicationSerializer + + def self.can_attributes + [:can_move_posts, + :can_edit, + :can_delete, + :can_recover, + :can_remove_allowed_users, + :can_invite_to, + :can_invite_via_email, + :can_create_post, + :can_reply_as_new_topic, + :can_flag_topic, + :can_convert_topic] + end + + attributes( + :notification_level, + :notifications_reason_id, + *can_attributes, + :can_remove_self_id, + :participants, + :allowed_users + ) + + has_one :created_by, serializer: BasicUserSerializer, embed: :objects + has_one :last_poster, serializer: BasicUserSerializer, embed: :objects + has_many :links, serializer: TopicLinkSerializer, embed: :objects + has_many :participants, serializer: TopicPostCountSerializer, embed: :objects + has_many :allowed_users, serializer: BasicUserSerializer, embed: :objects + has_many :allowed_groups, serializer: BasicGroupSerializer, embed: :objects + + def participants + object.post_counts_by_user.reject { |p| object.participants[p].blank? }.map do |pc| + { user: object.participants[pc[0]], post_count: pc[1] } + end + end + + def include_participants? + object.post_counts_by_user.present? + end + + def include_links? + object.links.present? + end + + def created_by + object.topic.user + end + + def last_poster + object.topic.last_poster + end + + def notification_level + object.topic_user&.notification_level || TopicUser.notification_levels[:regular] + end + + def notifications_reason_id + object.topic_user.notifications_reason_id + end + + def include_notifications_reason_id? + object.topic_user.present? + end + + # confusingly this is an id, not a bool like all other `can` methods + def can_remove_self_id + scope.user.id + end + + def include_can_remove_self_id? + scope.can_remove_allowed_users?(object.topic, scope.user) + end + + can_attributes.each do |ca| + define_method(ca) { true } + end + + def include_can_move_posts? + scope.can_move_posts?(object.topic) + end + + def include_can_edit? + scope.can_edit?(object.topic) + end + + def include_can_delete? + scope.can_delete?(object.topic) + end + + def include_can_recover? + scope.can_recover_topic?(object.topic) + end + + def include_can_remove_allowed_users? + scope.can_remove_allowed_users?(object.topic) + end + + def include_can_invite_to? + scope.can_invite_to?(object.topic) + end + + def include_can_invite_via_email? + scope.can_invite_via_email?(object.topic) + end + + def include_can_create_post? + scope.can_create?(Post, object.topic) + end + + def include_can_reply_as_new_topic? + scope.can_reply_as_new_topic?(object.topic) + end + + def include_can_flag_topic? + object.actions_summary.any? { |a| a[:can_act] } + end + + def include_can_convert_topic? + scope.can_convert_topic?(object.topic) + end + + def allowed_users + object.topic.allowed_users.reject { |user| object.group_allowed_user_ids.include?(user.id) } + end + + def include_allowed_users? + object.personal_message + end + + def allowed_groups + object.topic.allowed_groups + end + + def include_allowed_groups? + object.personal_message + end + +end diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 8a7105bb91b..48ffd7d4bca 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -50,7 +50,6 @@ class TopicViewSerializer < ApplicationSerializer :posted, :unpinned, :pinned, - :details, :current_post_number, :highest_post_number, :last_read_post_number, @@ -70,66 +69,14 @@ class TopicViewSerializer < ApplicationSerializer :participant_count, :destination_category_id, :pm_with_non_human_user, - :pending_posts_count + :queued_posts_count ) - # TODO: Split off into proper object / serializer + has_one :details, serializer: TopicViewDetailsSerializer, root: false, embed: :objects + has_many :pending_posts, serializer: TopicPendingPostSerializer, root: false, embed: :objects + def details - topic = object.topic - - result = { - created_by: BasicUserSerializer.new(topic.user, scope: scope, root: false), - last_poster: BasicUserSerializer.new(topic.last_poster, scope: scope, root: false) - } - - if private_message?(topic) - allowed_user_ids = Set.new - - result[:allowed_groups] = object.topic.allowed_groups.map do |group| - allowed_user_ids.merge(GroupUser.where(group: group).pluck(:user_id)) - BasicGroupSerializer.new(group, scope: scope, root: false) - end - - result[:allowed_users] = object.topic.allowed_users.select do |user| - !allowed_user_ids.include?(user.id) - end.map! do |user| - BasicUserSerializer.new(user, scope: scope, root: false) - end - end - - if object.post_counts_by_user.present? - participants = object.post_counts_by_user.reject { |p| object.participants[p].blank? }.map do |pc| - TopicPostCountSerializer.new({ user: object.participants[pc[0]], post_count: pc[1] }, scope: scope, root: false) - end - result[:participants] = participants if participants.length > 0 - end - - if object.links.present? - result[:links] = object.links.map do |user| - TopicLinkSerializer.new(user, scope: scope, root: false) - end - end - - if has_topic_user? - result[:notification_level] = object.topic_user.notification_level - result[:notifications_reason_id] = object.topic_user.notifications_reason_id - else - result[:notification_level] = TopicUser.notification_levels[:regular] - end - - result[:can_move_posts] = true if scope.can_move_posts?(object.topic) - result[:can_edit] = true if scope.can_edit?(object.topic) - result[:can_delete] = true if scope.can_delete?(object.topic) - result[:can_recover] = true if scope.can_recover_topic?(object.topic) - result[:can_remove_allowed_users] = true if scope.can_remove_allowed_users?(object.topic) - result[:can_remove_self_id] = scope.user.id if scope.can_remove_allowed_users?(object.topic, scope.user) - result[:can_invite_to] = true if scope.can_invite_to?(object.topic) - result[:can_invite_via_email] = true if scope.can_invite_via_email?(object.topic) - result[:can_create_post] = true if scope.can_create?(Post, object.topic) - result[:can_reply_as_new_topic] = true if scope.can_reply_as_new_topic?(object.topic) - result[:can_flag_topic] = actions_summary.any? { |a| a[:can_act] } - result[:can_convert_topic] = true if scope.can_convert_topic?(object.topic) - result + object end def message_bus_last_id @@ -141,7 +88,7 @@ class TopicViewSerializer < ApplicationSerializer end def is_warning - private_message?(object.topic) && object.topic.subtype == TopicSubtype.moderator_warning + object.personal_message && object.topic.subtype == TopicSubtype.moderator_warning end def include_is_warning? @@ -161,7 +108,7 @@ class TopicViewSerializer < ApplicationSerializer end def include_message_archived? - private_message?(object.topic) + object.personal_message end def message_archived @@ -214,16 +161,7 @@ class TopicViewSerializer < ApplicationSerializer end def actions_summary - result = [] - return [] unless post = object.posts&.first - PostActionType.topic_flag_types.each do |sym, id| - result << { id: id, - count: 0, - hidden: false, - can_act: scope.post_can_act?(post, sym) } - # TODO: other keys? :can_defer_flags, :acted, :can_undo - end - result + object.actions_summary end def has_deleted @@ -276,7 +214,7 @@ class TopicViewSerializer < ApplicationSerializer end def include_pm_with_non_human_user? - private_message?(object.topic) + object.personal_message end def pm_with_non_human_user @@ -297,18 +235,15 @@ class TopicViewSerializer < ApplicationSerializer object.topic.shared_draft.present? end - def pending_posts_count - ReviewableQueuedPost.viewable_by(scope.user).where(topic_id: object.topic.id).pending.count + def include_pending_posts? + scope.authenticated? && object.queued_posts_enabled end - def include_pending_posts_count? - scope.is_staff? && NewPostManager.queue_enabled? + def queued_posts_count + object.queued_posts_count end - private - - def private_message?(topic) - @private_message ||= topic.private_message? + def include_queued_posts_count? + scope.is_staff? && object.queued_posts_enabled end - end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index e48bbae6be0..9084946b016 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -359,6 +359,8 @@ en: placeholder: "type the message title here" review: + awaiting_approval: "Awaiting Approval" + delete: "Delete" settings: saved: "Saved" save_changes: "Save Changes" diff --git a/config/routes.rb b/config/routes.rb index 9cc0354d66a..d02121714f6 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -325,6 +325,7 @@ Discourse::Application.routes.draw do action_id: /[a-z\_]+/ } put "review/:reviewable_id" => "reviewables#update", constraints: { reviewable_id: /\d+/ } + delete "review/:reviewable_id" => "reviewables#destroy", constraints: { reviewable_id: /\d+/ } get "session/sso" => "session#sso" get "session/sso_login" => "session#sso_login" diff --git a/db/migrate/20190412161430_add_created_by_index_to_reviewables.rb b/db/migrate/20190412161430_add_created_by_index_to_reviewables.rb new file mode 100644 index 00000000000..243a0a1f3c1 --- /dev/null +++ b/db/migrate/20190412161430_add_created_by_index_to_reviewables.rb @@ -0,0 +1,5 @@ +class AddCreatedByIndexToReviewables < ActiveRecord::Migration[5.2] + def change + add_index :reviewables, [:topic_id, :status, :created_by_id] + end +end diff --git a/lib/guardian.rb b/lib/guardian.rb index 5207989df1a..558de2cfbed 100644 --- a/lib/guardian.rb +++ b/lib/guardian.rb @@ -174,6 +174,12 @@ class Guardian SiteSetting.enable_badges && is_staff? end + def can_delete_reviewable_queued_post?(reviewable) + reviewable.present? && + authenticated? && + reviewable.created_by_id == @user.id + end + def can_see_group?(group) return false if group.blank? return true if group.visibility_level == Group.visibility_levels[:public] diff --git a/lib/topic_view.rb b/lib/topic_view.rb index 66da453ba05..af81eb703f2 100644 --- a/lib/topic_view.rb +++ b/lib/topic_view.rb @@ -6,8 +6,26 @@ require_dependency 'gaps' class TopicView MEGA_TOPIC_POSTS_COUNT = 10000 - attr_reader :topic, :posts, :guardian, :filtered_posts, :chunk_size, :print, :message_bus_last_id - attr_accessor :draft, :draft_key, :draft_sequence, :user_custom_fields, :post_custom_fields, :post_number + attr_reader( + :topic, + :posts, + :guardian, + :filtered_posts, + :chunk_size, + :print, + :message_bus_last_id, + :queued_posts_enabled, + :personal_message + ) + + attr_accessor( + :draft, + :draft_key, + :draft_sequence, + :user_custom_fields, + :post_custom_fields, + :post_number + ) def self.print_chunk_size 1000 @@ -81,6 +99,9 @@ class TopicView @draft_key = @topic.draft_key @draft_sequence = DraftSequence.current(@user, @draft_key) + + @queued_posts_enabled = NewPostManager.queue_enabled? + @personal_message = @topic.private_message? end def canonical_path @@ -378,6 +399,13 @@ class TopicView end end + def group_allowed_user_ids + return @group_allowed_user_ids unless @group_allowed_user_ids.nil? + + group_ids = @topic.allowed_groups.map(&:id) + @group_allowed_user_ids = Set.new(GroupUser.where(group_id: group_ids).pluck('distinct user_id')) + end + def all_post_actions @all_post_actions ||= PostAction.counts_for(@posts, @user) end @@ -390,6 +418,27 @@ class TopicView @links ||= TopicLink.topic_map(@guardian, @topic.id) end + def pending_posts + ReviewableQueuedPost.pending.where(created_by: @user, topic: @topic).order(:created_at) + end + + def actions_summary + return @actions_summary unless @actions_summary.nil? + + @actions_summary = [] + return @actions_summary unless post = posts&.first + PostActionType.topic_flag_types.each do |sym, id| + @actions_summary << { + id: id, + count: 0, + hidden: false, + can_act: @guardian.post_can_act?(post, sym) + } + end + + @actions_summary + end + def link_counts @link_counts ||= TopicLink.counts_for(@guardian, @topic, posts) end @@ -493,6 +542,10 @@ class TopicView end end + def queued_posts_count + ReviewableQueuedPost.viewable_by(@user).where(topic_id: @topic.id).pending.count + end + protected def read_posts_set diff --git a/spec/requests/posts_controller_spec.rb b/spec/requests/posts_controller_spec.rb index 7e5e65557c6..5a0ebd2c21e 100644 --- a/spec/requests/posts_controller_spec.rb +++ b/spec/requests/posts_controller_spec.rb @@ -803,6 +803,10 @@ describe PostsController do rp = ReviewableQueuedPost.find_by(created_by: user) expect(rp.reviewable_scores.first.reason).to eq('fast_typer') + expect(parsed['pending_post']).to be_present + expect(parsed['pending_post']['id']).to eq(rp.id) + expect(parsed['pending_post']['raw']).to eq("this is the test content") + mod = Fabricate(:moderator) rp.perform(mod, :approve) diff --git a/spec/requests/reviewables_controller_spec.rb b/spec/requests/reviewables_controller_spec.rb index bf061003025..f88a872d673 100644 --- a/spec/requests/reviewables_controller_spec.rb +++ b/spec/requests/reviewables_controller_spec.rb @@ -17,6 +17,11 @@ describe ReviewablesController do get "/review/settings.json" expect(response.code).to eq("403") end + + it "denies deleting" do + delete "/review/123" + expect(response.code).to eq("403") + end end context "regular user" do @@ -452,6 +457,32 @@ describe ReviewablesController do end + context "#destroy" do + let(:user) { Fabricate(:user) } + + before do + sign_in(user) + end + + it "returns 404 if the reviewable doesn't exist" do + delete "/review/1234.json" + expect(response.code).to eq("404") + end + + it "returns 404 if the user can't see the reviewable" do + queued_post = Fabricate(:reviewable_queued_post) + delete "/review/#{queued_post.id}.json" + expect(response.code).to eq("404") + end + + it "returns 200 if the user can delete the reviewable" do + queued_post = Fabricate(:reviewable_queued_post, created_by: user) + delete "/review/#{queued_post.id}.json" + expect(response.code).to eq("200") + expect(queued_post.reload).to be_deleted + end + end + end end diff --git a/spec/serializers/topic_view_serializer_spec.rb b/spec/serializers/topic_view_serializer_spec.rb index 5b50522b890..645750ffb14 100644 --- a/spec/serializers/topic_view_serializer_spec.rb +++ b/spec/serializers/topic_view_serializer_spec.rb @@ -3,7 +3,8 @@ require 'rails_helper' describe TopicViewSerializer do def serialize_topic(topic, user_arg) topic_view = TopicView.new(topic.id, user_arg) - TopicViewSerializer.new(topic_view, scope: Guardian.new(user_arg), root: false).as_json + serializer = TopicViewSerializer.new(topic_view, scope: Guardian.new(user_arg), root: false).as_json + JSON.parse(MultiJson.dump(serializer)).deep_symbolize_keys! end before do @@ -53,7 +54,7 @@ describe TopicViewSerializer do it 'should include suggested topics' do json = serialize_topic(topic, user) - expect(json[:suggested_topics].first.id).to eq(topic2.id) + expect(json[:suggested_topics].first[:id]).to eq(topic2.id) end end @@ -130,4 +131,84 @@ describe TopicViewSerializer do expect(json[:tags]).to eq([]) end end + + describe "pending posts" do + context "when the queue is enabled" do + before do + SiteSetting.approve_post_count = 1 + end + + let!(:queued_post) do + ReviewableQueuedPost.needs_review!( + topic: topic, + payload: { raw: "hello my raw contents" }, + created_by: user + ) + end + + it "returns a pending_posts_count when the queue is enabled" do + json = serialize_topic(topic, admin) + expect(json[:queued_posts_count]).to eq(1) + end + + it "returns a user's pending posts" do + json = serialize_topic(topic, user) + expect(json[:queued_posts_count]).to be_nil + + post = json[:pending_posts].find { |p| p[:id] = queued_post.id } + expect(post[:raw]).to eq("hello my raw contents") + expect(post).to be_present + end + end + end + + context "without an enabled queue" do + it "returns nil for the count" do + json = serialize_topic(topic, admin) + expect(json[:queued_posts_count]).to be_nil + expect(json[:pending_posts]).to be_nil + end + end + + context "details" do + it "returns the details object" do + PostCreator.create!(user, topic_id: topic.id, raw: "this is my post content") + topic.topic_links.create!(user: user, url: 'https://discourse.org', domain: 'discourse.org', clicks: 100) + json = serialize_topic(topic, admin) + + details = json[:details] + expect(details).to be_present + expect(details[:created_by][:id]).to eq(topic.user_id) + expect(details[:last_poster][:id]).to eq(user.id) + expect(details[:notification_level]).to be_present + expect(details[:can_move_posts]).to eq(true) + expect(details[:can_flag_topic]).to eq(true) + expect(details[:links][0][:clicks]).to eq(100) + + participant = details[:participants].find { |p| p[:id] == user.id } + expect(participant[:post_count]).to eq(1) + end + + it "returns extra fields for a personal message" do + group = Fabricate(:group) + GroupUser.create(group: group, user: user) + GroupUser.create(group: group, user: admin) + + group2 = Fabricate(:group) + GroupUser.create(group: group2, user: user) + + pm = Fabricate(:private_message_topic) + pm.update(archetype: 'private_message') + pm.topic_allowed_groups.create!(group: group) + pm.topic_allowed_groups.create!(group: group2) + + json = serialize_topic(pm, admin) + + details = json[:details] + expect(details[:can_remove_self_id]).to eq(admin.id) + expect(details[:allowed_users].find { |au| au[:id] == pm.user_id }).to be_present + expect(details[:allowed_groups].find { |ag| ag[:id] == group.id }).to be_present + end + end + end diff --git a/test/javascripts/acceptance/composer-test.js.es6 b/test/javascripts/acceptance/composer-test.js.es6 index e7e801e22d6..c93d166adc4 100644 --- a/test/javascripts/acceptance/composer-test.js.es6 +++ b/test/javascripts/acceptance/composer-test.js.es6 @@ -255,6 +255,8 @@ QUnit.test("Posting on a different topic", async assert => { QUnit.test("Create an enqueued Reply", async assert => { await visit("/t/internationalization-localization/280"); + assert.notOk(find(".pending-posts .reviewable-item").length); + await click("#topic-footer-buttons .btn.create"); assert.ok(exists(".d-editor-input"), "the composer input is visible"); assert.ok(!exists("#reply-title"), "there is no title since this is a reply"); @@ -270,6 +272,8 @@ QUnit.test("Create an enqueued Reply", async assert => { await click(".modal-footer button"); assert.ok(invisible(".d-modal"), "the modal can be dismissed"); + + assert.ok(find(".pending-posts .reviewable-item").length); }); QUnit.test("Edit the first post", async assert => { diff --git a/test/javascripts/fixtures/topic.js.es6 b/test/javascripts/fixtures/topic.js.es6 index f4b1459d7a8..7467e71a1e9 100644 --- a/test/javascripts/fixtures/topic.js.es6 +++ b/test/javascripts/fixtures/topic.js.es6 @@ -1,6 +1,7 @@ /*jshint maxlen:10000000 */ export default { "/t/280/1.json": { + pending_posts: [], post_stream: { posts: [ { diff --git a/test/javascripts/helpers/create-pretender.js.es6 b/test/javascripts/helpers/create-pretender.js.es6 index ed46639feef..a7313a8a369 100644 --- a/test/javascripts/helpers/create-pretender.js.es6 +++ b/test/javascripts/helpers/create-pretender.js.es6 @@ -430,7 +430,14 @@ export default function() { } if (data.raw === "enqueue this content please") { - return response(200, { success: true, action: "enqueued" }); + return response(200, { + success: true, + action: "enqueued", + pending_post: { + id: 1234, + raw: data.raw + } + }); } return response(200, {