From 4cb48768f93f1fa8d4015c56b950c6679af6fda7 Mon Sep 17 00:00:00 2001 From: Akash Srivastava Date: Wed, 3 Nov 2021 09:38:04 +0530 Subject: [PATCH] Added collapsed hidden comments to author's article/podcast episode view (#14018) * Handling hiding/collapsing hidden comments client side * Updated comment quality text * Added placeholder for comments against podcast episodes * Added logic for encompassing co-author-ids in articles * Removed rspecs validating non-presence of hidden comments in dom * Fixed hiding flow on comments page for a commentable * Fixed e2e specs * Addressed feedback on e2e tests --- .../initializeCommentsPage.js.erb | 83 ++++++++++++++++--- app/views/articles/show.html.erb | 1 + app/views/comments/_comment.html.erb | 5 +- .../comments/_comment_quality_marker.html.erb | 6 +- app/views/comments/index.html.erb | 4 +- app/views/podcast_episodes/show.html.erb | 2 +- config/locales/views/comments/en.yml | 4 +- .../articleFlows/hideArticleComments.spec.js | 24 ++++++ .../viewArticleDiscussion.spec.js | 10 +++ spec/requests/comments_spec.rb | 18 ---- spec/support/seeds/seeds_e2e.rb | 33 ++++++++ 11 files changed, 153 insertions(+), 37 deletions(-) create mode 100644 cypress/integration/seededFlows/articleFlows/hideArticleComments.spec.js diff --git a/app/assets/javascripts/initializers/initializeCommentsPage.js.erb b/app/assets/javascripts/initializers/initializeCommentsPage.js.erb index 5f4295778..f24141889 100644 --- a/app/assets/javascripts/initializers/initializeCommentsPage.js.erb +++ b/app/assets/javascripts/initializers/initializeCommentsPage.js.erb @@ -197,6 +197,8 @@ function initializeCommentsPage() { } } listenForDetailsToggle(); + + handleHiddenComments(commentableType); } function toggleCodeOfConduct() { @@ -554,24 +556,27 @@ function handleImageUpload(event, randomIdNumber) { } } +function updateItemSummaryHtml(item) { + var itemSummaryContent = item.getElementsByClassName("js-collapse-comment-content")[0]; + var usernames = item.getElementsByClassName("js-comment-username"); + var number = ""; + if (usernames.length > 1) { + number = " + " + (usernames.length - 1) + " replies" + } + var itemUsername = usernames[0].textContent + number + if (item.open) { + itemSummaryContent.innerHTML = ""; + } else { + itemSummaryContent.innerHTML = itemUsername; + } +} + function listenForDetailsToggle() { var detailItems = document.querySelectorAll(".js-comment-wrapper"); for (var i = 0; i < detailItems.length; i++) { detailItems[i].addEventListener("toggle", event => { var item = event.target; - var itemSummaryContent = item.getElementsByClassName("js-collapse-comment-content")[0]; - var usernames = item.getElementsByClassName("js-comment-username"); - var number = ""; - if (usernames.length > 1) { - number = " + " + (usernames.length - 1) + " replies" - } - var itemUsername = usernames[0].textContent + number - if (item.open) { - itemSummaryContent.innerHTML = ""; - } else { - itemSummaryContent.innerHTML = itemUsername; - } - item.getElementsByTagName("SUMMARY")[0].blur(); + updateItemSummaryHtml(item); }); } } @@ -589,3 +594,55 @@ function updateCommentsCount() { commentsCountDiv.dataset.commentsCount = commentsCountData; commentsCountDiv.innerHTML = `(${commentsCountData})` } + +function handleHiddenComments(commentableType){ + const currentUser = userData(); + const commentableAuthorIds = []; + let coAuthorIds = ''; + if(commentableType === "Article"){ + const articleContainer = document.querySelector('#article-show-container'); + if(articleContainer){ + commentableAuthorIds.push(articleContainer?.dataset?.authorId); + coAuthorIds = articleContainer?.dataset?.coAuthorIds; + if(coAuthorIds){ + coAuthorIds.split(',').forEach(coAuthorId => { + commentableAuthorIds.push(coAuthorId); + }); + } + } + else { + const commentsContainer = document.querySelector('#comments-container'); + if(commentsContainer){ + commentableAuthorIds.push(commentsContainer?.dataset?.commentableAuthorId); + coAuthorIds = commentsContainer?.dataset?.commentableCoAuthorIds; + if(coAuthorIds){ + coAuthorIds.split(',').forEach(coAuthorId => { + commentableAuthorIds.push(coAuthorId); + }); + } + } + } + } else if(commentableType === "PodcastEpisode"){ + const podCastEpisodeContainer = document.querySelector('.podcast-episode-container'); + if(podCastEpisodeContainer){ + commentableAuthorIds.push(podCastEpisodeContainer.dataset.creatorId); + } + } + if (commentableAuthorIds.includes(currentUser?.id?.toString())){ + collapseCommentsHiddenByCommentableUser(); + } + else { + document.querySelectorAll('.comment-hidden-by-commentable-user, .details-comment-hidden-by-commentable-user').forEach(element => { + element.classList.add('hidden'); + }) + } +} + +function collapseCommentsHiddenByCommentableUser() { + document.querySelectorAll(".js-comment-wrapper.details-comment-hidden-by-commentable-user").forEach(item => { + if (item.querySelectorAll('.comment-form').length === 0){ + item.open = false; + updateItemSummaryHtml(item); + } + }) +} diff --git a/app/views/articles/show.html.erb b/app/views/articles/show.html.erb index 67247505e..670d259b4 100644 --- a/app/views/articles/show.html.erb +++ b/app/views/articles/show.html.erb @@ -82,6 +82,7 @@ id="article-show-container" data-article-id="<%= @article.id %>" data-author-id="<%= @article.user_id %>" + data-co-author-ids="<%= @article.co_author_ids.join(",") %>" data-path="<%= @article.path %>" data-published="<%= @article.published? %>" data-pin-path="<%= stories_feed_pinned_article_path %>" diff --git a/app/views/comments/_comment.html.erb b/app/views/comments/_comment.html.erb index 3e26ba4c7..dd623eafb 100644 --- a/app/views/comments/_comment.html.erb +++ b/app/views/comments/_comment.html.erb @@ -1,6 +1,6 @@ -<% if comment && comment.user && !should_be_hidden?(comment, @root_comment) %> +<% if comment && comment.user %> <% if comment.depth < 3 %> -
+
" open> <%= inline_svg_tag("collapse.svg", aria: true, class: "crayons-icon expanded", title: t("views.comments.collapse")) %> @@ -17,6 +17,7 @@ <%= comment_class(comment, is_view_root: is_view_root) %> comment--deep-<%= comment.depth %> <%= "comment--too-deep" if comment.depth > 3 %> + <%= should_be_hidden?(comment, @root_comment) ? "comment-hidden-by-commentable-user" : "" %> " data-comment-id="<%= comment.id %>" data-path="<%= commentable&.path %>/comments/<%= comment.id_code_generated %>" diff --git a/app/views/comments/_comment_quality_marker.html.erb b/app/views/comments/_comment_quality_marker.html.erb index 0c81cd930..095128385 100644 --- a/app/views/comments/_comment_quality_marker.html.erb +++ b/app/views/comments/_comment_quality_marker.html.erb @@ -7,6 +7,10 @@ <% if comment.hidden_by_commentable_user %>
<%= inline_svg_tag("info.svg", aria: true, class: "crayons-icon mr-2", title: t("views.comments.quality.hidden.icon")) %> - <%= t("views.comments.quality.hidden.text") %> + <% if @root_comment %> + <%= t("views.comments.quality.hidden.text.visible_in_permalink") %> + <% else %> + <%= t("views.comments.quality.hidden.text.accessible_via_permalink") %> + <% end %>
<% end %> diff --git a/app/views/comments/index.html.erb b/app/views/comments/index.html.erb index 6bbd459c4..99f9c1cfe 100644 --- a/app/views/comments/index.html.erb +++ b/app/views/comments/index.html.erb @@ -124,7 +124,9 @@ class="crayons-card text-padding min-w-0 z-elevate" id="comments-container" data-commentable-id="<%= @commentable&.id %>" - data-commentable-type="<%= @commentable&.class&.name %>"> + data-commentable-type="<%= @commentable&.class&.name %>" + data-commentable-author-id="<%= @commentable&.user_id %>" + data-commentable-co-author-ids="<%= @commentable&.co_author_ids&.join(",") %>"> <% unless @root_comment %> <%= render "form", diff --git a/app/views/podcast_episodes/show.html.erb b/app/views/podcast_episodes/show.html.erb index 39b340e79..798ce0d51 100644 --- a/app/views/podcast_episodes/show.html.erb +++ b/app/views/podcast_episodes/show.html.erb @@ -28,7 +28,7 @@ <% end %> -
+

diff --git a/config/locales/views/comments/en.yml b/config/locales/views/comments/en.yml index ee0b5c7ca..b0ab08c0d 100644 --- a/config/locales/views/comments/en.yml +++ b/config/locales/views/comments/en.yml @@ -72,7 +72,9 @@ en: conduct: View Code of Conduct hidden: icon: Info - text: Comment hidden by post author - thread only visible in this permalink + text: + visible_in_permalink: Comment hidden by post author - thread only visible in this permalink + accessible_via_permalink: Comment hidden by post author - thread only accessible via permalink write: errors: one: '1 error prohibited this comment from being saved:' diff --git a/cypress/integration/seededFlows/articleFlows/hideArticleComments.spec.js b/cypress/integration/seededFlows/articleFlows/hideArticleComments.spec.js new file mode 100644 index 000000000..8897d71fd --- /dev/null +++ b/cypress/integration/seededFlows/articleFlows/hideArticleComments.spec.js @@ -0,0 +1,24 @@ +describe('Hiding/unhiding comments on an article', () => { + beforeEach(() => { + cy.testSetup(); + cy.fixture('users/adminUser.json').as('user'); + + cy.get('@user').then((user) => { + cy.loginAndVisit(user, '/admin_mcadmin/test-article-slug'); + }); + }); + + describe('Admin visits the article authored by them', () => { + it('Hides a comment and then unhides it from the same screen', () => { + cy.findByRole('button', { name: 'Toggle dropdown menu' }).click(); + cy.findByRole('link', { name: "Hide Admin McAdmin's comment" }).click(); + cy.findByRole('button', { name: 'Toggle dropdown menu' }).should( + 'not.be.visible', + ); + cy.findByRole('img', { name: 'Expand' }).click(); + cy.findByRole('button', { name: 'Toggle dropdown menu' }).click(); + cy.findByRole('link', { name: "Unhide Admin McAdmin's comment" }).click(); + cy.findByRole('img', { name: 'Expand' }).should('not.exist'); + }); + }); +}); diff --git a/cypress/integration/seededFlows/articleFlows/viewArticleDiscussion.spec.js b/cypress/integration/seededFlows/articleFlows/viewArticleDiscussion.spec.js index 4164d046a..8b2af82e1 100644 --- a/cypress/integration/seededFlows/articleFlows/viewArticleDiscussion.spec.js +++ b/cypress/integration/seededFlows/articleFlows/viewArticleDiscussion.spec.js @@ -50,4 +50,14 @@ describe('View article discussion', () => { .first() .findByRole('button', { name: 'Edit profile' }); }); + + it('does not see hidden comments on an article not authored by them', () => { + cy.visit('/admin_mcadmin/test-article-with-hidden-comments-slug'); + cy.findByText(/Some comments have been hidden by the post's author/).should( + 'exist', + ); + cy.findByRole('button', { name: 'Toggle dropdown menu' }).should( + 'not.exist', + ); + }); }); diff --git a/spec/requests/comments_spec.rb b/spec/requests/comments_spec.rb index 5122bad83..e614d78e2 100644 --- a/spec/requests/comments_spec.rb +++ b/spec/requests/comments_spec.rb @@ -71,12 +71,6 @@ RSpec.describe "Comments", type: :request do expect(response.body).to include(CGI.escapeHTML(comment.title(150))) expect(response.body).to include(child.processed_html) end - - it "does not display the comment if it is hidden" do - child.update(hidden_by_commentable_user: true) - get comment.path - expect(response.body).not_to include child.processed_html - end end context "when the comment is two levels nested and hidden" do # child of a child @@ -139,23 +133,11 @@ RSpec.describe "Comments", type: :request do expect(response.body).not_to include(third_level_child.processed_html) end - it "does not show the hidden comment's children in the article's comments section" do - fourth_level_child - get "#{article.path}/comments" - expect(response.body).not_to include(fourth_level_child.processed_html) - end - it "does not show the hidden comment in its parent's permalink" do get second_level_child.path expect(response.body).not_to include(third_level_child.processed_html) end - it "does not show the hidden comment's child in its parent's permalink" do - fourth_level_child - get second_level_child.path - expect(response.body).not_to include(fourth_level_child.processed_html) - end - it "shows the comment in the permalink" do get third_level_child.path expect(response.body).to include(third_level_child.processed_html) diff --git a/spec/support/seeds/seeds_e2e.rb b/spec/support/seeds/seeds_e2e.rb index 1c5b3a92f..a2e9a644b 100644 --- a/spec/support/seeds/seeds_e2e.rb +++ b/spec/support/seeds/seeds_e2e.rb @@ -375,6 +375,39 @@ end ############################################################################## +seeder.create_if_doesnt_exist(Article, "slug", "test-article-with-hidden-comments-slug") do + markdown = <<~MARKDOWN + --- + title: Test article with hidden comments + published: true + cover_image: #{Faker::Company.logo} + --- + #{Faker::Hipster.paragraph(sentence_count: 2)} + #{Faker::Markdown.random} + #{Faker::Hipster.paragraph(sentence_count: 2)} + MARKDOWN + article = Article.create!( + body_markdown: markdown, + featured: true, + show_comments: true, + user_id: admin_user.id, + slug: "test-article-with-hidden-comments-slug", + any_comments_hidden: true, + ) + + comment_attributes = { + body_markdown: Faker::Hipster.paragraph(sentence_count: 1), + user_id: admin_user.id, + commentable_id: article.id, + commentable_type: "Article", + hidden_by_commentable_user: true + } + + Comment.create!(comment_attributes) +end + +############################################################################## + seeder.create_if_doesnt_exist(Article, "title", "Organization test article") do markdown = <<~MARKDOWN ---