Merge pull request #84 from SuperTux88/reshare-remove-public-flag

Reshare remove public flag

docs/_entities/comment.md

@@ -2,7 +2,8 @@
 title: Comment
 ---
 
-This entity represents a comment to some kind of [Post][post] ([StatusMessage][status_message] or [Reshare][reshare]).
+This entity represents a comment to some kind of [Post][post] ([StatusMessage][status_message]
+or currently still a [Reshare][reshare]).
 
 See also: [Relayable][relayable]
 

docs/_entities/like.md

@@ -6,7 +6,7 @@ This entity represents a like to some kind of post (e.g. status message).
 
 The `parent_type` can be one of:
 
-* [Post][post] ([StatusMessage][status_message] or [Reshare][reshare])
+* [Post][post] ([StatusMessage][status_message] or currently still a [Reshare][reshare])
 * [Comment][comment] (diaspora\* doesn't support this at the moment)
 
 See also: [Relayable][relayable]

docs/_entities/post.md

@@ -2,7 +2,14 @@
 title: Post
 ---
 
-This is the abstract parent type of [StatusMessage][status_message] and [Reshare][reshare].
+This is the abstract parent type of [StatusMessage][status_message] and currently still [Reshare][reshare].
+
+{% include warning_box.html
+   title="Future of reshares"
+   content="<p>Reshare will not inherit from Post in the future anymore! More information about this
+   can be found <a href=\"/diaspora_federation/entities/reshare.html\">here</a> or
+   in <a href=\"https://github.com/diaspora/diaspora_federation/issues/83\">this issue</a>.</p>"
+%}
 
 ## Common Properties
 

docs/_entities/reshare.md

@@ -2,15 +2,25 @@
 title: Reshare
 ---
 
-This entity represents a reshare of a [StatusMessage][status_message]. It inherits from [Post][post].
+This entity represents a reshare of a [StatusMessage][status_message].
+
+{% include warning_box.html
+   title="Future of reshares"
+   content="<p>Current versions of diaspora* handle reshares like they inherit from
+   <a href=\"/diaspora_federation/entities/post.html\">Post</a> and allow interactions (Comments and
+   Likes) on the reshare. In the future, the reshare entity will only be used to increase the spread of a
+   Post, more information about this can be found in
+   <a href=\"https://github.com/diaspora/diaspora_federation/issues/83\">this issue</a>.</p>
+   <p>There currently exists a special case for reshares with a deleted root post. It is valid when the
+   reshare doesn't include <code>root_author</code> and <code>root_guid</code>. If only one of
+   <code>root_author</code> and <code>root_guid</code> is present, the entity is not valid. Once
+   reshares are only used to increase the reach of a post, reshares without <code>root_author</code> and
+   <code>root_guid</code> will no longer be valid and reshares will be deleted if the original post is deleted.</p>"
+%}
 
 The recipient must [fetch][fetching] the root from `root_author` if the post is not already known.
 When the `root_guid` is already available locally, the recipient must validate that it's from `root_author`.
 
-A special case when the entity doesn't include `root_author` and `root_guid` is allowed. This case represents
-a reshare for a deleted post. If there is only one of `root_author` and `root_guid` is present, then the entity
-is not valid.
-
 ## Properties
 
 | Property      | Type                         | Description                                                   |
@@ -21,13 +31,6 @@ is not valid.
 | `root_author` | [diaspora\* ID][diaspora-id] | The diaspora\* ID of the author of the reshared [Post][post]. |
 | `root_guid`   | [GUID][guid]                 | The GUID of the reshared [Post][post].                        |
 
-## Optional Properties
-
-| Property                | Type (Length)          | Description                                                                                |
-| ----------------------- | ---------------------- | ------------------------------------------------------------------------------------------ |
-| `provider_display_name` | [String][string] (255) | The means by which the author has posted the reshare.                                      |
-| `public`                | [Boolean][boolean]     | `false` if the reshare is not public (diaspora\* currenlty only supports public reshares). |
-
 ## Example
 
 ~~~xml
@@ -35,10 +38,8 @@ is not valid.
   <author>alice@example.org</author>
   <guid>a0b53e5029f6013487753131731751e9</guid>
   <created_at>2016-07-12T00:36:42Z</created_at>
-  <provider_display_name/>
   <root_author>bob@example.com</root_author>
   <root_guid>a0b53bc029f6013487753131731751e9</root_guid>
-  <public>true</public>
 </reshare>
 ~~~
 

docs/assets/css/main.scss

@@ -48,6 +48,10 @@ body {
     }
   }
 
+  .panel-body p:last-child {
+    margin-bottom: 0;
+  }
+
   /**
    * Based on the Twitter Bootstrap .table styles. See
    * https://github.com/twbs/bootstrap/blob/master/less/tables.less

lib/diaspora_federation/entities/post.rb

@@ -19,6 +19,10 @@ module DiasporaFederation
       #   Post entity creation time
       #   @return [Time] creation time
       #
+      # @!attribute [r] public
+      #   Shows whether the post is visible to everyone or only to some aspects
+      #   @return [Boolean] is it public
+      #
       # @!attribute [r] provider_display_name
       #   A string that describes a means by which a user has posted the post
       #   @return [String] provider display name
@@ -29,6 +33,7 @@ module DiasporaFederation
           property :author, :string, xml_name: :diaspora_handle
           property :guid, :string
           property :created_at, :timestamp, default: -> { Time.now.utc }
+          property :public, :boolean, default: false
           property :provider_display_name, :string, optional: true
         end
       end

lib/diaspora_federation/entities/reshare.rb

@@ -4,7 +4,22 @@ module DiasporaFederation
     #
     # @see Validators::ReshareValidator
     class Reshare < Entity
-      include Post
+      # @!attribute [r] author
+      #   The diaspora* ID of the person who reshares the post
+      #   @see Person#author
+      #   @return [String] diaspora* ID
+      property :author, :string, xml_name: :diaspora_handle
+
+      # @!attribute [r] guid
+      #   A random string of at least 16 chars
+      #   @see Validation::Rule::Guid
+      #   @return [String] status message guid
+      property :guid, :string
+
+      # @!attribute [r] created_at
+      #   Post entity creation time
+      #   @return [Time] creation time
+      property :created_at, :timestamp, default: -> { Time.now.utc }
 
       # @!attribute [r] root_author
       #   The diaspora* ID of the person who posted the original post
@@ -18,11 +33,6 @@ module DiasporaFederation
       #   @return [String] root guid
       property :root_guid, :string, optional: true
 
-      # @!attribute [r] public
-      #   Has no meaning at the moment
-      #   @return [Boolean] public
-      property :public, :boolean, optional: true, default: true # always true? (we only reshare public posts)
-
       # @return [String] string representation of this object
       def to_s
         "#{super}:#{root_guid}"
@@ -30,6 +40,11 @@ module DiasporaFederation
 
       # Fetch and receive root post from remote, if not available locally
       # and validates if it's from the correct author
+      # TODO: after reshares are only used to increase the reach of a post (and
+      # legacy reshares with own interactions are migrated to the new form),
+      # root_author and root_guid aren't allowed to be empty anymore, so a
+      # not_nil check should be added to the validator and the first few lines
+      # here can be removed.
       def validate_root
         return if root_author.nil? && root_guid.nil?
 

lib/diaspora_federation/entities/status_message.rb

@@ -31,11 +31,6 @@ module DiasporaFederation
       #   @return [Entities::Event] event
       entity :event, Entities::Event, optional: true
 
-      # @!attribute [r] public
-      #   Shows whether the status message is visible to everyone or only to some aspects
-      #   @return [Boolean] is it public
-      property :public, :boolean, default: false
-
       private
 
       def validate

lib/diaspora_federation/schemas/federation_entities.json

@@ -215,28 +215,26 @@
     },
 
     "reshare": {
-      "allOf": [
+      "type": "object",
-        {"$ref": "#/definitions/post"},
+      "properties": {
-        {
+        "entity_type": {
+          "type": "string",
+          "pattern": "^reshare$"
+        },
+
+        "entity_data": {
           "type": "object",
           "properties": {
-            "entity_type": {
+            "author": { "type": "string" },
-              "type": "string",
+            "guid": { "$ref": "#/definitions/guid" },
-              "pattern": "^reshare$"
+            "created_at": { "type": "string" },
-            },
+            "root_author": {"type": "string"},
-
+            "root_guid": {"$ref": "#/definitions/guid"}
-            "entity_data": {
+          },
-              "type": "object",
+          "required": ["author", "guid", "created_at", "root_author", "root_guid"]
-              "properties": {
+        },
-                "root_author": {"type": "string"},
+        "required": ["entity_type", "entity_data"]
-                "root_guid": {"$ref": "#/definitions/guid"}
+      }
-              },
-
-              "required": ["root_author", "root_guid"]
-            }
-          }
-        }
-      ]
     },
 
     "profile": {

lib/diaspora_federation/test/factories.rb

@@ -150,9 +150,7 @@ module DiasporaFederation
         root_guid { Fabricate.sequence(:guid) }
         guid { Fabricate.sequence(:guid) }
         author { Fabricate.sequence(:diaspora_id) }
-        public true
         created_at { Time.now.utc }
-        provider_display_name { "the testsuite" }
       end
 
       Fabricator(:retraction_entity, class_name: DiasporaFederation::Entities::Retraction) do

lib/diaspora_federation/validators/reshare_validator.rb

@@ -11,8 +11,6 @@ module DiasporaFederation
       rule :author, :diaspora_id
 
       rule :guid, :guid
-
-      rule :public, :boolean
     end
   end
 end

spec/lib/diaspora_federation/entities/reshare_spec.rb

@@ -8,10 +8,8 @@ module DiasporaFederation
   <author>#{data[:author]}</author>
   <guid>#{data[:guid]}</guid>
   <created_at>#{data[:created_at].utc.iso8601}</created_at>
-  <provider_display_name>#{data[:provider_display_name]}</provider_display_name>
   <root_author>#{data[:root_author]}</root_author>
   <root_guid>#{data[:root_guid]}</root_guid>
-  <public>#{data[:public]}</public>
 </reshare>
 XML
 
@@ -22,10 +20,8 @@ XML
     "author": "#{data[:author]}",
     "guid": "#{data[:guid]}",
     "created_at": "#{data[:created_at].utc.iso8601}",
-    "provider_display_name": "#{data[:provider_display_name]}",
     "root_author": "#{data[:root_author]}",
-    "root_guid": "#{data[:root_guid]}",
+    "root_guid": "#{data[:root_guid]}"
-    "public": #{data[:public]}
   }
 }
 JSON
@@ -38,24 +34,6 @@ JSON
 
     it_behaves_like "a JSON Entity"
 
-    context "default values" do
-      it "uses default values" do
-        minimal_xml = <<-XML
-<reshare>
-  <author>#{data[:author]}</author>
-  <guid>#{data[:guid]}</guid>
-  <created_at>#{data[:created_at]}</created_at>
-  <root_diaspora_id>#{data[:root_author]}</root_diaspora_id>
-  <root_guid>#{data[:root_guid]}</root_guid>
-</reshare>
-XML
-
-        parsed_instance = DiasporaFederation::Salmon::XmlPayload.unpack(Nokogiri::XML(minimal_xml).root)
-        expect(parsed_instance.public).to be_truthy
-        expect(parsed_instance.provider_display_name).to be_nil
-      end
-    end
-
     context "parse xml" do
       describe "#validate_root" do
         it "fetches the root post if it is not available already" do

spec/lib/diaspora_federation/entities/status_message_spec.rb

@@ -19,6 +19,7 @@ module DiasporaFederation
   <author>#{data[:author]}</author>
   <guid>#{data[:guid]}</guid>
   <created_at>#{data[:created_at].utc.iso8601}</created_at>
+  <public>#{data[:public]}</public>
   <provider_display_name>#{data[:provider_display_name]}</provider_display_name>
   <text>#{data[:text]}</text>
   <photo>
@@ -50,7 +51,6 @@ module DiasporaFederation
     <lat>#{location.lat}</lat>
     <lng>#{location.lng}</lng>
   </location>
-  <public>#{data[:public]}</public>
 </status_message>
 XML
 
@@ -61,6 +61,7 @@ XML
     "author": "#{data[:author]}",
     "guid": "#{data[:guid]}",
     "created_at": "#{data[:created_at].utc.iso8601}",
+    "public": #{data[:public]},
     "provider_display_name": "#{data[:provider_display_name]}",
     "text": "#{data[:text]}",
     "photos": [
@@ -102,8 +103,7 @@ XML
         "lat": "#{location.lat}",
         "lng": "#{location.lng}"
       }
-    },
+    }
-    "public": #{data[:public]}
   }
 }
 JSON

spec/lib/diaspora_federation/validators/reshare_validator_spec.rb

@@ -28,9 +28,5 @@ module DiasporaFederation
         let(:correct_values) { [nil, "alice@example.org"] }
       end
     end
-
-    it_behaves_like "a boolean validator" do
-      let(:property) { :public }
-    end
   end
 end