From b9d4edd91a0f1c87a9227df58b04912e7471e925 Mon Sep 17 00:00:00 2001 From: Sam Date: Thu, 10 Apr 2014 10:56:56 +1000 Subject: [PATCH] FEATURE: display unpinned state, allow unpinning by clicking on pin --- .../components/topic_status_component.js | 40 ++++++++++++++++--- .../javascripts/discourse/models/topic.js | 23 +++++++++++ app/assets/stylesheets/common.scss | 2 +- .../stylesheets/common/base/_topic-list.scss | 3 ++ app/controllers/topics_controller.rb | 8 ++++ app/models/topic.rb | 5 +++ app/serializers/listable_topic_serializer.rb | 7 +++- app/serializers/topic_view_serializer.rb | 9 ++++- config/locales/client.en.yml | 2 + config/routes.rb | 1 + lib/pinned_check.rb | 25 +++++------- spec/components/pinned_check_spec.rb | 15 ++++--- 12 files changed, 107 insertions(+), 33 deletions(-) create mode 100644 app/assets/stylesheets/common/base/_topic-list.scss diff --git a/app/assets/javascripts/discourse/components/topic_status_component.js b/app/assets/javascripts/discourse/components/topic_status_component.js index 2bc42f50075..4724b4b0428 100644 --- a/app/assets/javascripts/discourse/components/topic_status_component.js +++ b/app/assets/javascripts/discourse/components/topic_status_component.js @@ -9,24 +9,54 @@ Discourse.TopicStatusComponent = Ember.Component.extend({ classNames: ['topic-statuses'], - hasDisplayableStatus: Em.computed.or('topic.closed', 'topic.pinned', 'topic.invisible', 'topic.archetypeObject.notDefault'), - shouldRerender: Discourse.View.renderIfChanged('topic.closed', 'topic.pinned', 'topic.visible'), + hasDisplayableStatus: Em.computed.or('topic.closed', 'topic.pinned', 'topic.unpinned', 'topic.invisible', 'topic.archetypeObject.notDefault'), + shouldRerender: Discourse.View.renderIfChanged('topic.closed', 'topic.pinned', 'topic.visible', 'topic.unpinned'), + + didInsertElement: function(){ + var topic = this.get('topic'); + + // could be passed in a controller + if(topic.constructor.toString() !== 'Discourse.Topic') { + topic = topic.get('model'); + } + + this.$('a').click(function(){ + // only pin unpin for now + if (topic.get('pinned')) { + topic.clearPin(); + } else { + topic.rePin(); + } + + return false; + }); + }, render: function(buffer) { if (!this.get('hasDisplayableStatus')) { return; } var self = this, - renderIconIf = function(conditionProp, name, key) { + renderIconIf = function(conditionProp, name, key, actionable) { if (!self.get(conditionProp)) { return; } var title = I18n.t("topic_statuses." + key + ".help"); - buffer.push(""); + + var startTag = actionable ? "a href='#'" : "span"; + var endTag = actionable ? "a" : "span"; + + buffer.push("<" + startTag + + " title='" + title +"' class='topic-status'>"); }; // Allow a plugin to add a custom icon to a topic this.trigger('addCustomIcon', buffer); + var togglePin = function(){ + + }; + renderIconIf('topic.closed', 'lock', 'locked'); - renderIconIf('topic.pinned', 'thumb-tack', 'pinned'); + renderIconIf('topic.pinned', 'thumb-tack', 'pinned', togglePin); + renderIconIf('topic.unpinned', 'thumb-tack unpinned', 'unpinned', togglePin); renderIconIf('topic.invisible', 'eye-slash', 'invisible'); } }); diff --git a/app/assets/javascripts/discourse/models/topic.js b/app/assets/javascripts/discourse/models/topic.js index a68b5155a3e..802641623fe 100644 --- a/app/assets/javascripts/discourse/models/topic.js +++ b/app/assets/javascripts/discourse/models/topic.js @@ -271,12 +271,35 @@ Discourse.Topic = Discourse.Model.extend({ // Clear the pin optimistically from the object topic.set('pinned', false); + topic.set('unpinned', true); Discourse.ajax("/t/" + this.get('id') + "/clear-pin", { type: 'PUT' }).then(null, function() { // On error, put the pin back topic.set('pinned', true); + topic.set('unpinned', false); + }); + }, + + /** + Re-pins a topic with a cleared pin + + @method rePin + **/ + rePin: function() { + var topic = this; + + // Clear the pin optimistically from the object + topic.set('pinned', true); + topic.set('unpinned', false); + + Discourse.ajax("/t/" + this.get('id') + "/re-pin", { + type: 'PUT' + }).then(null, function() { + // On error, put the pin back + topic.set('pinned', true); + topic.set('unpinned', false); }); }, diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index 69064e1750d..51b3f565c7f 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -8,6 +8,6 @@ @import "common/components/*"; @import "common/admin/*"; @import "common/input_tip"; - +@import "common/base/*"; /* This file doesn't actually exist, it is injected by DiscourseSassImporter. */ @import "plugins"; diff --git a/app/assets/stylesheets/common/base/_topic-list.scss b/app/assets/stylesheets/common/base/_topic-list.scss new file mode 100644 index 00000000000..f2f01d73329 --- /dev/null +++ b/app/assets/stylesheets/common/base/_topic-list.scss @@ -0,0 +1,3 @@ +.fa-thumb-tack.unpinned { + @include fa-icon-rotate(315deg, 1); +} diff --git a/app/controllers/topics_controller.rb b/app/controllers/topics_controller.rb index 0f1c5885911..bb0dc41e630 100644 --- a/app/controllers/topics_controller.rb +++ b/app/controllers/topics_controller.rb @@ -20,6 +20,7 @@ class TopicsController < ApplicationController :move_posts, :merge_topic, :clear_pin, + :re_pin, :autoclose, :bulk, :reset_new, @@ -281,6 +282,13 @@ class TopicsController < ApplicationController render nothing: true end + def re_pin + topic = Topic.where(id: params[:topic_id].to_i).first + guardian.ensure_can_see!(topic) + topic.re_pin_for(current_user) + render nothing: true + end + def timings PostTiming.process_timings( current_user, diff --git a/app/models/topic.rb b/app/models/topic.rb index bf7a6b9dc28..b6dc18deabd 100644 --- a/app/models/topic.rb +++ b/app/models/topic.rb @@ -618,6 +618,11 @@ class Topic < ActiveRecord::Base TopicUser.change(user.id, id, cleared_pinned_at: Time.now) end + def re_pin_for(user) + return unless user.present? + TopicUser.change(user.id, id, cleared_pinned_at: nil) + end + def update_pinned(status, global=false) update_column(:pinned_at, status ? Time.now : nil) update_column(:pinned_globally, global) diff --git a/app/serializers/listable_topic_serializer.rb b/app/serializers/listable_topic_serializer.rb index b82e2a124ad..713ca632713 100644 --- a/app/serializers/listable_topic_serializer.rb +++ b/app/serializers/listable_topic_serializer.rb @@ -14,6 +14,7 @@ class ListableTopicSerializer < BasicTopicSerializer :unread, :new_posts, :pinned, + :unpinned, :excerpt, :visible, :closed, @@ -65,7 +66,11 @@ class ListableTopicSerializer < BasicTopicSerializer end def pinned - PinnedCheck.new(object, object.user_data).pinned? + PinnedCheck.pinned?(object, object.user_data) + end + + def unpinned + PinnedCheck.unpinned?(object, object.user_data) end protected diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb index 503ff305af8..dd162dcafca 100644 --- a/app/serializers/topic_view_serializer.rb +++ b/app/serializers/topic_view_serializer.rb @@ -31,6 +31,7 @@ class TopicViewSerializer < ApplicationSerializer :draft_sequence, :starred, :posted, + :unpinned, :pinned, # Is topic pinned and viewer hasn't cleared the pin? :pinned_at, # Ignores clear pin :details, @@ -41,7 +42,7 @@ class TopicViewSerializer < ApplicationSerializer :expandable_first_post # Define a delegator for each attribute of the topic we want - attributes *topic_attributes + attributes(*topic_attributes) topic_attributes.each do |ta| class_eval %{def #{ta} object.topic.#{ta} @@ -145,7 +146,11 @@ class TopicViewSerializer < ApplicationSerializer alias_method :include_posted?, :has_topic_user? def pinned - PinnedCheck.new(object.topic, object.topic_user).pinned? + PinnedCheck.pinned?(object.topic, object.topic_user) + end + + def unpinned + PinnedCheck.unpinned?(object.topic, object.topic_user) end def pinned_at diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index f14463be6a8..a7622ad8fa3 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1145,6 +1145,8 @@ en: topic_statuses: locked: help: "this topic is closed; it no longer accepts new replies" + unpinned: + help: "this topic is unpinned; it will displayed in default ordering" pinned: help: "this topic is pinned; it will display at the top of its category" archived: diff --git a/config/routes.rb b/config/routes.rb index 243b7028872..9a1df2aa7db 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -315,6 +315,7 @@ Discourse::Application.routes.draw do put "t/:slug/:topic_id/status" => "topics#status", constraints: {topic_id: /\d+/} put "t/:topic_id/status" => "topics#status", constraints: {topic_id: /\d+/} put "t/:topic_id/clear-pin" => "topics#clear_pin", constraints: {topic_id: /\d+/} + put "t/:topic_id/re-pin" => "topics#re_pin", constraints: {topic_id: /\d+/} put "t/:topic_id/mute" => "topics#mute", constraints: {topic_id: /\d+/} put "t/:topic_id/unmute" => "topics#unmute", constraints: {topic_id: /\d+/} put "t/:topic_id/autoclose" => "topics#autoclose", constraints: {topic_id: /\d+/} diff --git a/lib/pinned_check.rb b/lib/pinned_check.rb index d1890c58b37..b1a25a4d5db 100644 --- a/lib/pinned_check.rb +++ b/lib/pinned_check.rb @@ -2,23 +2,16 @@ # taking into account anonymous users and users who have dismissed it class PinnedCheck - def initialize(topic, topic_user=nil) - @topic, @topic_user = topic, topic_user + def self.unpinned?(topic,topic_user=nil) + topic.pinned_at && + topic_user && + topic_user.cleared_pinned_at && + topic_user.cleared_pinned_at > topic.pinned_at end - def pinned? - - # If the topic isn't pinned the answer is false - return false if @topic.pinned_at.blank? - - # If the user is anonymous or hasn't entered the topic, the value is always true - return true if @topic_user.blank? - - # If the user hasn't cleared the pin, it's true - return true if @topic_user.cleared_pinned_at.blank? - - # The final check is to see whether the cleared the pin before or after it was last pinned - @topic_user.cleared_pinned_at < @topic.pinned_at + def self.pinned?(topic, topic_user=nil) + !!topic.pinned_at && + !unpinned?(topic,topic_user) end -end \ No newline at end of file +end diff --git a/spec/components/pinned_check_spec.rb b/spec/components/pinned_check_spec.rb index 2cab45ae9f6..f858c936cf9 100644 --- a/spec/components/pinned_check_spec.rb +++ b/spec/components/pinned_check_spec.rb @@ -1,9 +1,8 @@ +require 'spec_helper' require 'pinned_check' describe PinnedCheck do - #let(:topic) { Fabricate.build(:topic) } - let(:pinned_at) { 12.hours.ago } let(:unpinned_topic) { Fabricate.build(:topic) } let(:pinned_topic) { Fabricate.build(:topic, pinned_at: pinned_at) } @@ -11,11 +10,11 @@ describe PinnedCheck do context "without a topic_user record (either anonymous or never been in the topic)" do it "returns false if the topic is not pinned" do - PinnedCheck.new(unpinned_topic).should_not be_pinned + PinnedCheck.pinned?(unpinned_topic).should be_false end it "returns true if the topic is pinned" do - PinnedCheck.new(unpinned_topic).should_not be_pinned + PinnedCheck.pinned?(unpinned_topic).should be_false end end @@ -28,7 +27,7 @@ describe PinnedCheck do let(:topic_user) { TopicUser.new(topic: unpinned_topic, user: user) } it "returns false" do - PinnedCheck.new(unpinned_topic, topic_user).should_not be_pinned + PinnedCheck.pinned?(unpinned_topic, topic_user).should be_false end end @@ -37,17 +36,17 @@ describe PinnedCheck do let(:topic_user) { TopicUser.new(topic: pinned_topic, user: user) } it "is pinned if the topic_user's cleared_pinned_at is blank" do - PinnedCheck.new(pinned_topic, topic_user).should be_pinned + PinnedCheck.pinned?(pinned_topic, topic_user).should be_true end it "is not pinned if the topic_user's cleared_pinned_at is later than when it was pinned_at" do topic_user.cleared_pinned_at = (pinned_at + 1.hour) - PinnedCheck.new(pinned_topic, topic_user).should_not be_pinned + PinnedCheck.pinned?(pinned_topic, topic_user).should be_false end it "is pinned if the topic_user's cleared_pinned_at is earlier than when it was pinned_at" do topic_user.cleared_pinned_at = (pinned_at - 3.hours) - PinnedCheck.new(pinned_topic, topic_user).should be_pinned + PinnedCheck.pinned?(pinned_topic, topic_user).should be_true end end