Add Commentable Interface #882

beagleknight · 2017-02-10T08:20:09Z

🎩 What? Why?

I added two flags for managing comments in a specific feature:

comments_enabled: A global setting to enable or disable comments for a feature.
comments_blocked: A step setting to prevent new comments to be added.

Then I created the Commentable interface so any model can implement it. Since it include all the logic needed to render the comments component, the comment helper has been simplified:

<%= comments_for @some_commentable%>

This changed pushed a few changes into the API:

Get commentable's comments:

commentable(id: $id, type: $type) {
  comments {
    ...
  }
}

Add a new comment:

commentable(id: $id, type: $type) {
  addComment(body: $body) {
    ...
  }
}

These changes will be more effective when we start implementing other types for our current models like Proposal, Meeting and so on.

📌 Related Issues

Close Conditionally show comments #553

📋 Subtasks

Remove comments_always_enabled general setting
Add Commentable interface
Add Commentable type and interface for graphql
Refactor components
Fix AddCommentFormComponent
Add comments_enabled and comments_blocked logic for all features using comments
Add blocked comments callout
Add sign-in to comment message

📷 Screenshots (optional)

👻 GIF

oriolgual · 2017-02-13T09:22:24Z

decidim-budgets/lib/decidim/budgets/feature.rb

@@ -22,11 +22,11 @@
  feature.settings(:global) do |settings|
    settings.attribute :total_budget, type: :integer, default: 100_000_000
    settings.attribute :vote_threshold_percent, type: :integer, default: 70
-    settings.attribute :comments_always_enabled, type: :boolean, default: true
+    settings.attribute :comments_enabled, type: :boolean, default: true


We need a migration for these settings and current features

@oriolgual If the setting is not found it uses the default value so the migration is not needed.

oriolgual · 2017-02-13T09:26:52Z

decidim-comments/app/queries/decidim/comments/sorted_comments.rb

-        if comment.replies.size.positive?
-          comment.replies.size + comment.replies.map { |reply| count_replies(reply) }.sum
+        if comment.comments.size.positive?
+          comment.comments.size + comment.comments.map { |reply| count_replies(reply) }.sum


No need to map & sum, you can sum directly

Oh, I didn't know that! Great

oriolgual · 2017-02-13T09:27:21Z

decidim-comments/config/locales/ca.yml

@@ -21,6 +21,7 @@ ca:
            subject: Tens una nova resposta del teu comentari
    components:
      add_comment_form:
+        account_message: "<a href=\"%{sign_in_url}\">Entra amb el teu compte</a> o <a href=\"%{sign_up_url}\">regístra't</a> per a deixar un comentari."


oriolgual · 2017-02-13T09:27:31Z

decidim-comments/config/locales/ca.yml

@@ -47,6 +48,7 @@ ca:
      comment_thread:
        title: Conversa amb %{authorName}
      comments:
+        blocked_comments_warning: Els comentaris estan desactivats a la fase actual pero pots llegir els comentaris de les fases anteriors.


oriolgual · 2017-02-13T09:27:49Z

decidim-comments/config/locales/en.yml

@@ -22,6 +22,8 @@ en:
            subject: You have a new reply of your comment
    components:
      add_comment_form:
+        account_message: "<a href=\"%{sign_in_url}\">Sign in with your account</a> or <a href=\"%{sign_up_url}\">sign up</a> to leave your comment.


add your comment

oriolgual · 2017-02-13T09:28:08Z

decidim-comments/config/locales/es.yml

@@ -21,6 +21,7 @@ es:
            subject: Uno de tus comentarios ha recibido respuesta
    components:
      add_comment_form:
+        account_message: "<a href=\"%{sign_in_url}\">Entra con tu cuenta</a> o <a href=\"%{sign_up_url}\">regístratet</a> para dejar tu comentario."


regístrate

oriolgual · 2017-02-13T09:29:00Z

decidim-comments/lib/decidim/comments/commentable.rb

+
+        has_many :comments, as: :commentable, foreign_key: "decidim_commentable_id", foreign_type: "decidim_commentable_type", class_name: "Decidim::Comments::Comment"
+
+        def commentable?


Should all these methods be implemented in the models were we include this? If so I'd raise NotImplementedError

I think there is no need to implement those methods. They are just returning boolean values and not doing anything else. The default ones are just standard: enable comments without extra options. What do you think?

oriolgual · 2017-02-13T09:29:27Z

decidim-comments/lib/decidim/comments/comments_helper.rb

-        commentable_id = resource.id.to_s
-        node_id = "comments-for-#{commentable_type.demodulize}-#{commentable_id}"
+      def comments_for(resource)
+        if resource.commentable?


return unless resource.commentable?

oriolgual · 2017-02-13T09:31:26Z

decidim-comments/spec/types/commentable_mutation_type_spec.rb

+          "{ addComment(body: \"#{body}\", alignment: #{alignment}) { body } }"
+        }
+
+        it "should call CreateComment command" do


codecov-io · 2017-02-13T09:51:10Z

Codecov Report

Merging #882 into master will decrease coverage by -0.02%.
The diff coverage is 99.29%.

@@            Coverage Diff             @@
##           master     #882      +/-   ##
==========================================
- Coverage   97.25%   97.23%   -0.02%     
==========================================
  Files         370      374       +4     
  Lines        6075     6248     +173     
==========================================
+ Hits         5908     6075     +167     
- Misses        167      173       +6

Impacted Files	Coverage Δ
...m-comments/lib/decidim/comments/comments_helper.rb	`100% <100%> (ø)`	✅
...-comments/lib/decidim/comments/api/comment_type.rb	`100% <100%> (ø)`	✅
...im-comments/app/models/decidim/comments/comment.rb	`100% <100%> (ø)`	✅
...idim-budgets/app/models/decidim/budgets/project.rb	`100% <100%> (ø)`	✅
...nts/app/types/decidim/comments/commentable_type.rb	`100% <100%> (ø)`
decidim-pages/app/models/decidim/pages/page.rb	`100% <100%> (ø)`	✅
decidim-results/lib/decidim/results/feature.rb	`95.45% <100%> (ø)`	✅
...ypes/decidim/comments/commentable_mutation_type.rb	`100% <100%> (ø)`
decidim-proposals/lib/decidim/proposals/feature.rb	`100% <100%> (ø)`	✅
...p/controllers/concerns/decidim/feature_settings.rb	`100% <100%> (ø)`	✅
... and 17 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d16d4ad...bd15299. Read the comment docs.

mention-bot · 2017-02-14T08:50:37Z

@beagleknight, thanks for your PR! By analyzing the history of the files in this pull request, we identified @josepjaume and @oriolgual to be potential reviewers.

josepjaume · 2017-02-14T09:07:09Z

decidim-comments/app/frontend/comments/comment.fragment.graphql

@@ -1,12 +1,12 @@
 fragment Comment on Comment {
  ...CommentData
-  replies {
+  comments {


As a future opportunity, maybe we could flatten this query and build the tree it afterwards, and/or use query batching

I agree. I think we can use another approach in order to allow infinite nesting (as long as the ui supports it).

josepjaume · 2017-02-14T09:08:17Z

decidim-comments/app/frontend/comments/comments.component.test.jsx

@@ -46,7 +52,8 @@ describe('<Comments />', () => {

  stubComponent(AddCommentForm, {
    fragments: {
-      user: addCommentFragment
+      session: addCommentFragmentSession,


Shouldn't this be addCommentSessionFragment?

No! It should be addCommentFormSessionFragment 😂

josepjaume · 2017-02-14T09:08:26Z

decidim-comments/app/frontend/comments/comments.component.test.jsx

@@ -46,7 +52,8 @@ describe('<Comments />', () => {

  stubComponent(AddCommentForm, {
    fragments: {
-      user: addCommentFragment
+      session: addCommentFragmentSession,
+      commentable: addCommentFragmentCommentable


Same as up above.

josepjaume · 2017-02-14T09:10:55Z

decidim-comments/app/types/decidim/comments/commentable_type.rb

+      description "A commentable object"
+
+      interfaces [
+        Decidim::Comments::CommentableInterface


ULTRA NICE 🐇 🌈 🎈

josepjaume · 2017-02-14T09:15:26Z

decidim-comments/lib/decidim/comments/commentable.rb

+
+        has_many :comments, as: :commentable, foreign_key: "decidim_commentable_id", foreign_type: "decidim_commentable_type", class_name: "Decidim::Comments::Comment"
+
+        def commentable?


We should add docs indicating what all these defaults mean and what effect they have on comments.

beagleknight self-assigned this Feb 10, 2017

beagleknight added in-progress module: comments labels Feb 10, 2017

beagleknight force-pushed the feature/comments/enabled branch from 8e6a98b to 7406e0e Compare February 13, 2017 09:04

beagleknight changed the title ~~Add comments_enabled step setting logic~~ Add Commentable Interface Feb 13, 2017

oriolgual suggested changes Feb 13, 2017

View reviewed changes

oriolgual approved these changes Feb 13, 2017

View reviewed changes

beagleknight force-pushed the feature/comments/enabled branch from ce6efc2 to 1e6e76b Compare February 13, 2017 15:20

beagleknight added in-review and removed in-progress labels Feb 14, 2017

josepjaume suggested changes Feb 14, 2017

View reviewed changes

beagleknight added 13 commits February 14, 2017 11:09

Remove comments_always_enabled global setting for each feature

c3730a1

Add Commentable interfaces

b6fa9cc

Keep working on using commentable instead of root query and mutation

c57183f

Fix api and components

4a6b8bd

Fix broken specs

6edcf66

Migrate options to methods

cdf3f7d

Create comments_enabled and comments_blocked flags

57d3051

Naming

051ef1a

Add settings to dummy feature

b953a72

Add message to sign in and blocked comments warning

9472f10

Add feedback

82156d2

Fix broken specs

0a79e4c

Add feedback

bd15299

beagleknight force-pushed the feature/comments/enabled branch from 1e6e76b to bd15299 Compare February 14, 2017 10:10

oriolgual requested a review from josepjaume February 14, 2017 10:11

josepjaume approved these changes Feb 14, 2017

View reviewed changes

josepjaume merged commit 18a1e26 into master Feb 14, 2017

josepjaume deleted the feature/comments/enabled branch February 14, 2017 14:12

josepjaume removed the in-review label Feb 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Commentable Interface #882

Add Commentable Interface #882

beagleknight commented Feb 10, 2017 •

edited

Loading

oriolgual Feb 13, 2017

beagleknight Feb 13, 2017

oriolgual Feb 13, 2017

beagleknight Feb 13, 2017

oriolgual Feb 13, 2017

oriolgual Feb 13, 2017

oriolgual Feb 13, 2017

oriolgual Feb 13, 2017

oriolgual Feb 13, 2017

beagleknight Feb 13, 2017

oriolgual Feb 13, 2017

oriolgual Feb 13, 2017

codecov-io commented Feb 13, 2017 •

edited

Loading

mention-bot commented Feb 14, 2017

josepjaume Feb 14, 2017

beagleknight Feb 14, 2017

josepjaume Feb 14, 2017

beagleknight Feb 14, 2017

josepjaume Feb 14, 2017

josepjaume Feb 14, 2017

josepjaume Feb 14, 2017


		has_many :comments, as: :commentable, foreign_key: "decidim_commentable_id", foreign_type: "decidim_commentable_type", class_name: "Decidim::Comments::Comment"

		def commentable?

Add Commentable Interface #882

Add Commentable Interface #882

Conversation

beagleknight commented Feb 10, 2017 • edited Loading

🎩 What? Why?

📌 Related Issues

📋 Subtasks

📷 Screenshots (optional)

👻 GIF

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov-io commented Feb 13, 2017 • edited Loading

Codecov Report

mention-bot commented Feb 14, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

beagleknight commented Feb 10, 2017 •

edited

Loading

codecov-io commented Feb 13, 2017 •

edited

Loading