Hide 'Save' button on posts that belong to the current user (#17293)

* implement change

* struggling with tests

* extend implementation to relevant views

* update Article.test.jsx snapshot

* fix failing specs

* query userData more robustly

* default saveable value in SaveButton component

* fix spec

* fetch currentUser async
This commit is contained in:
Arit Amana 2022-04-28 12:33:01 -04:00 committed by GitHub
parent 89fcd84f9f
commit 0d023163ba
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 86 additions and 13 deletions

View file

@ -207,6 +207,8 @@ function insertArticles(articles) {
var list = document.getElementById('substories');
var newArticlesHTML = '';
var el = document.getElementById('home-articles-object');
var currentUser = userData();
var currentUserId = currentUser && currentUser.id;
if (el) {
el.outerHTML = '';
}
@ -226,9 +228,12 @@ function insertArticles(articles) {
existingEl.parentElement.classList.contains('crayons-story') &&
!document.getElementById('video-player-' + article.id)
) {
existingEl.parentElement.outerHTML = buildArticleHTML(article);
existingEl.parentElement.outerHTML = buildArticleHTML(
article,
currentUserId,
);
} else if (!existingEl) {
var newHTML = buildArticleHTML(article);
var newHTML = buildArticleHTML(article, currentUserId);
newArticlesHTML += newHTML;
initializeReadingListIcons();
}

View file

@ -2,7 +2,7 @@
/* eslint-disable no-multi-str */
function buildArticleHTML(article) {
function buildArticleHTML(article, currentUserId = null) {
var tagIcon = `<svg width="24" height="24" viewBox="0 0 24 24" class="crayons-icon" xmlns="http://www.w3.org/2000/svg"><path d="M7.784 14l.42-4H4V8h4.415l.525-5h2.011l-.525 5h3.989l.525-5h2.011l-.525 5H20v2h-3.784l-.42 4H20v2h-4.415l-.525 5h-2.011l.525-5H9.585l-.525 5H7.049l.525-5H4v-2h3.784zm2.011 0h3.99l.42-4h-3.99l-.42 4z"/></svg>`;
if (article && article.class_name === 'Tag') {
return `<article class="crayons-story">
@ -278,7 +278,9 @@ function buildArticleHTML(article) {
}
var saveButton = '';
if (article.class_name === 'Article') {
// "!=" instead of "!==" used to compare user_id and currentUserId because
// currentUserId is a String while user_id is an Integer
if (article.class_name === 'Article' && article.user_id != currentUserId) {
saveButton =
'<button type="button" id="article-save-button-' +
article.id +

View file

@ -23,6 +23,7 @@ export const Article = ({
bookmarkClick,
feedStyle,
pinned,
saveable,
}) => {
if (article && article.type_of === 'podcast_episodes') {
return <PodcastArticle article={article} />;
@ -133,6 +134,7 @@ export const Article = ({
article={article}
isBookmarked={isBookmarked}
onClick={bookmarkClick}
saveable={saveable}
/>
</div>
</div>
@ -155,6 +157,7 @@ Article.defaultProps = {
isBookmarked: false,
isFeatured: false,
feedStyle: 'basic',
saveable: true,
};
Article.propTypes = {
@ -164,4 +167,5 @@ Article.propTypes = {
feedStyle: PropTypes.string,
bookmarkClick: PropTypes.func.isRequired,
pinned: PropTypes.bool,
saveable: PropTypes.bool.isRequired,
};

View file

@ -263,4 +263,25 @@ describe('<Article /> component', () => {
expect(queryByText('person', { selector: 'span' })).toBeDefined();
});
it('should show bookmark button when article is saveable (default)', () => {
const { queryByTestId } = render(
<Article {...commonProps} isBookmarked={false} article={article} />,
);
expect(queryByTestId(`article-save-button-${article.id}`)).toBeDefined();
});
it('should hide bookmark button when article is not saveable', () => {
const { queryByTestId } = render(
<Article
{...commonProps}
isBookmarked={false}
article={article}
saveable={false}
/>,
);
expect(queryByTestId(`article-save-button-${article.id}`)).toBeNull();
});
});

View file

@ -15,7 +15,7 @@ export class SaveButton extends Component {
render() {
const { buttonText } = this.state;
const { article, isBookmarked, onClick } = this.props;
const { article, isBookmarked, onClick, saveable = true } = this.props;
const mouseMove = (_e) => {
this.setState({ buttonText: isBookmarked ? 'Unsave' : 'Save' });
@ -33,7 +33,7 @@ export class SaveButton extends Component {
});
};
if (article.class_name === 'Article') {
if (article.class_name === 'Article' && saveable) {
return (
<button
type="button"
@ -74,6 +74,7 @@ SaveButton.propTypes = {
article: articlePropTypes.isRequired,
isBookmarked: PropTypes.bool.isRequired,
onClick: PropTypes.func.isRequired,
saveable: PropTypes.bool.isRequired,
};
SaveButton.displayName = 'SaveButton';

View file

@ -3,6 +3,16 @@ import { fireEvent, render } from '@testing-library/preact';
import { axe } from 'jest-axe';
import { SaveButton } from '../SaveButton';
it('should not show bookmark button when saveable is false', () => {
const article = { class_name: 'Article', id: 1 };
const { queryByText } = render(
<SaveButton article={article} saveable={false} />,
);
const saveButton = queryByText('Saved');
expect(saveButton).toBeNull();
});
it('should have no a11y violations', async () => {
const article = { class_name: 'Article', id: 1 };
const { container } = render(<SaveButton article={article} />);

View file

@ -0,0 +1,12 @@
import { getUserDataAndCsrfToken } from '@utilities/getUserDataAndCsrfToken';
getUserDataAndCsrfToken().then(({ currentUser }) => {
const currentUserId = currentUser && currentUser.id;
document.querySelectorAll('.bookmark-button').forEach((button) => {
const { articleAuthorId } = button.dataset;
if (currentUserId && articleAuthorId == currentUserId) {
button.classList.add('hidden');
}
});
});

View file

@ -4,6 +4,7 @@ import { Article, LoadingArticle } from '../articles';
import { Feed } from '../articles/Feed';
import { TodaysPodcasts, PodcastEpisode } from '../podcasts';
import { articlePropTypes } from '../common-prop-types';
import { getUserDataAndCsrfToken } from '@utilities/getUserDataAndCsrfToken';
/**
* Sends analytics about the featured article.
@ -59,9 +60,12 @@ PodcastEpisodes.propTypes = {
/**
* Renders the main feed.
*/
export const renderFeed = (timeFrame) => {
export const renderFeed = async (timeFrame) => {
const feedContainer = document.getElementById('homepage-feed');
const { currentUser } = await getUserDataAndCsrfToken();
const currentUserId = currentUser && currentUser.id;
render(
<Feed
timeFrame={timeFrame}
@ -92,6 +96,8 @@ export const renderFeed = (timeFrame) => {
// 2. Show the featured story next
// 3. Podcast episodes out today
// 4. Rest of the stories for the feed
// For "saveable", "!=" is used instead of "!==" to compare user_id
// and currentUserId because currentUserId is a String while user_id is an Integer
return (
<div>
{timeFrame === '' && pinnedArticle && (
@ -101,6 +107,7 @@ export const renderFeed = (timeFrame) => {
pinned={true}
feedStyle={feedStyle}
isBookmarked={bookmarkedFeedItems.has(pinnedArticle.id)}
saveable={pinnedArticle.user_id != currentUserId}
/>
)}
{featuredStory && (
@ -110,6 +117,7 @@ export const renderFeed = (timeFrame) => {
isFeatured
feedStyle={feedStyle}
isBookmarked={bookmarkedFeedItems.has(featuredStory.id)}
saveable={featuredStory.user_id != currentUserId}
/>
)}
{podcastEpisodes.length > 0 && (
@ -122,6 +130,7 @@ export const renderFeed = (timeFrame) => {
article={story}
feedStyle={feedStyle}
isBookmarked={bookmarkedFeedItems.has(story.id)}
saveable={story.user_id != currentUserId}
/>
))}
</div>

View file

@ -185,8 +185,10 @@ function search(query, filters, sortBy, sortDirection) {
.then((response) => response.json())
.then((content) => {
const resultDivs = [];
const currentUser = userData();
const currentUserId = currentUser && currentUser.id;
content.result.forEach((story) => {
resultDivs.push(buildArticleHTML(story));
resultDivs.push(buildArticleHTML(story, currentUserId));
});
document.getElementById('substories').innerHTML = resultDivs.join('');
initializeReadingListIcons();

View file

@ -147,7 +147,14 @@
<small class="crayons-story__tertiary fs-xs mr-2">
<%= t("views.articles.reading_time", count: story.reading_time < 1 ? 1 : story.reading_time) %>
</small>
<button type="button" id="article-save-button-<%= story.id %>" class="crayons-btn crayons-btn--secondary crayons-btn--s w-max bookmark-button" data-reactable-id="<%= story.id %>" aria-label="<%= t("views.articles.save.aria_label") %>" title="<%= t("views.articles.save.title") %>">
<button
type="button"
id="article-save-button-<%= story.id %>"
class="crayons-btn crayons-btn--secondary crayons-btn--s w-max bookmark-button"
data-reactable-id="<%= story.id %>"
data-article-author-id="<%= story.user_id %>"
aria-label="<%= t("views.articles.save.aria_label") %>"
title="<%= t("views.articles.save.title") %>">
<span class="bm-initial"><%= t("views.articles.save.initial") %></span>
<span class="bm-success"><%= t("views.articles.save.success") %></span>
</button>

View file

@ -17,7 +17,7 @@
<% @articles.each do |article| %>
<%= render "articles/single_story", story: article.decorate, featured: article.main_image.present? %>
<% end %>
<%= javascript_packs_with_chunks_tag "followButtons", "feedPreviewCards", defer: true %>
<%= javascript_packs_with_chunks_tag "followButtons", "feedPreviewCards", "hideBookmarkButtons", defer: true %>
<% else %>
<p><%= t("views.series.list.empty") %></p>
<% end %>

View file

@ -24,4 +24,4 @@
<%= render "organizations/sidebar_additional" %>
</div>
<%= javascript_packs_with_chunks_tag "storiesList", "followButtons", defer: true %>
<%= javascript_packs_with_chunks_tag "storiesList", "followButtons", "hideBookmarkButtons", defer: true %>

View file

@ -130,5 +130,5 @@
<%= render "stories/tagged_articles/sidebar_additional" %>
</div>
<%= javascript_packs_with_chunks_tag "storiesList", "followButtons", "feedPreviewCards", defer: true %>
<%= javascript_packs_with_chunks_tag "storiesList", "followButtons", "feedPreviewCards", "hideBookmarkButtons", defer: true %>
<% end %>

View file

@ -151,4 +151,4 @@
</div>
</main>
<%= javascript_packs_with_chunks_tag "storiesList", "followButtons", "feedPreviewCards", defer: true %>
<%= javascript_packs_with_chunks_tag "storiesList", "followButtons", "feedPreviewCards", "hideBookmarkButtons", defer: true %>