From f9105b2661405ca17eb648e6c9cc435126207dd7 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Mon, 11 Sep 2023 21:55:32 +0200 Subject: [PATCH 1/4] Initial commit --- .../js/pages/homepage/components/postlist/PostList.js | 4 ++-- src/newsreader/js/pages/homepage/reducers/posts.js | 6 ++---- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/src/newsreader/js/pages/homepage/components/postlist/PostList.js b/src/newsreader/js/pages/homepage/components/postlist/PostList.js index 168ca8e..136df40 100644 --- a/src/newsreader/js/pages/homepage/components/postlist/PostList.js +++ b/src/newsreader/js/pages/homepage/components/postlist/PostList.js @@ -6,7 +6,6 @@ import { fetchPostsBySection, fetchSavedPosts } from '../../actions/posts.js'; import { SAVED_TYPE } from '../../constants.js'; import { filterPosts } from './filters.js'; -import LoadingIndicator from '../../../../components/LoadingIndicator.js'; import PostItem from './PostItem.js'; class PostList extends React.Component { @@ -56,8 +55,9 @@ class PostList extends React.Component { } render() { + const isLastItem = this.props.postsByType.length - 1 == index; + const postItems = this.props.postsByType.map((item, index) => { - const isLastItem = this.props.postsByType.length - 1 == index; const defaultProps = { key: index, post: item, diff --git a/src/newsreader/js/pages/homepage/reducers/posts.js b/src/newsreader/js/pages/homepage/reducers/posts.js index dd795a0..d422ca7 100644 --- a/src/newsreader/js/pages/homepage/reducers/posts.js +++ b/src/newsreader/js/pages/homepage/reducers/posts.js @@ -1,5 +1,3 @@ -import { isEqual } from 'lodash'; - import { objectsFromArray } from '../../../utils.js'; import { CATEGORY_TYPE, RULE_TYPE } from '../constants.js'; @@ -12,8 +10,6 @@ import { TOGGLING_POST, TOGGLED_POST, } from '../actions/posts.js'; -import { SELECT_CATEGORY } from '../actions/categories.js'; -import { SELECT_RULE } from '../actions/rules.js'; import { MARK_SECTION_READ } from '../actions/selected.js'; const defaultState = { items: {}, isFetching: false, isUpdating: false }; @@ -22,11 +18,13 @@ export const posts = (state = { ...defaultState }, action) => { switch (action.type) { case REQUEST_POSTS: return { ...state, isFetching: true }; + // TODO: save post per category/rule in corresponding reducers case RECEIVE_POST: return { ...state, items: { ...state.items, [action.post.id]: { ...action.post } }, }; + // TODO: save posts per category/rule in corresponding reducers case RECEIVE_POSTS: const receivedItems = objectsFromArray(action.posts, 'id'); From 6e94890e0ed06c011004f09eead811b9170e839f Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Mon, 25 Sep 2023 19:58:32 +0200 Subject: [PATCH 2/4] Update react components --- src/newsreader/js/pages/homepage/App.js | 2 +- .../homepage/components/postlist/PostItem.js | 2 +- .../homepage/components/postlist/PostList.js | 5 +- .../homepage/components/postlist/filters.js | 42 ++----------- .../js/pages/homepage/reducers/posts.js | 62 +++++++++++++------ src/newsreader/news/core/serializers.py | 3 + 6 files changed, 55 insertions(+), 61 deletions(-) diff --git a/src/newsreader/js/pages/homepage/App.js b/src/newsreader/js/pages/homepage/App.js index 5b5acde..1834bee 100644 --- a/src/newsreader/js/pages/homepage/App.js +++ b/src/newsreader/js/pages/homepage/App.js @@ -56,7 +56,7 @@ const mapStateToProps = state => { const { error } = state.error; if (!isEqual(state.selected.post, {})) { - const ruleId = state.selected.post.rule; + const ruleId = state.selected.post.rule.id; const rule = state.rules.items[ruleId]; const category = state.categories.items[rule.category]; diff --git a/src/newsreader/js/pages/homepage/components/postlist/PostItem.js b/src/newsreader/js/pages/homepage/components/postlist/PostItem.js index ffb6832..ddd60db 100644 --- a/src/newsreader/js/pages/homepage/components/postlist/PostItem.js +++ b/src/newsreader/js/pages/homepage/components/postlist/PostItem.js @@ -16,7 +16,7 @@ import { formatDatetime } from '../../../../utils.js'; class PostItem extends React.Component { render() { const rule = { ...this.props.post.rule }; - const post = { ...this.props.post, rule: rule.id }; + const post = { ...this.props.post }; const token = Cookies.get('csrftoken'); const publicationDate = formatDatetime(post.publicationDate); diff --git a/src/newsreader/js/pages/homepage/components/postlist/PostList.js b/src/newsreader/js/pages/homepage/components/postlist/PostList.js index 136df40..197d81b 100644 --- a/src/newsreader/js/pages/homepage/components/postlist/PostList.js +++ b/src/newsreader/js/pages/homepage/components/postlist/PostList.js @@ -6,6 +6,7 @@ import { fetchPostsBySection, fetchSavedPosts } from '../../actions/posts.js'; import { SAVED_TYPE } from '../../constants.js'; import { filterPosts } from './filters.js'; +import LoadingIndicator from '../../../../components/LoadingIndicator.js'; import PostItem from './PostItem.js'; class PostList extends React.Component { @@ -55,7 +56,7 @@ class PostList extends React.Component { } render() { - const isLastItem = this.props.postsByType.length - 1 == index; + const isLastItem = this.props.postsByType.toReversed().find(item => !item.read); const postItems = this.props.postsByType.map((item, index) => { const defaultProps = { @@ -68,7 +69,7 @@ class PostList extends React.Component { timezone: this.props.timezone, }; - if (isLastItem && !item.read) { + if (isLastItem?.id === item.id) { return ; } else { return ; diff --git a/src/newsreader/js/pages/homepage/components/postlist/filters.js b/src/newsreader/js/pages/homepage/components/postlist/filters.js index 8439fc9..b3dd16a 100644 --- a/src/newsreader/js/pages/homepage/components/postlist/filters.js +++ b/src/newsreader/js/pages/homepage/components/postlist/filters.js @@ -4,31 +4,10 @@ const isEmpty = (object = {}) => { return Object.keys(object).length === 0; }; -const sortOrdering = (firstPost, secondPost) => { - const dateOrdering = - new Date(firstPost.publicationDate) < new Date(secondPost.publicationDate); - - if (firstPost.read && !secondPost.read) { - return 1; - } else if (secondPost.read && !firstPost.read) { - return -1; - } - - return dateOrdering; -}; - -const savedOrdering = (firstPost, secondPost) => { - return new Date(firstPost.publicationDate) < new Date(secondPost.publicationDate); -}; - export const filterPostsByRule = (rule = {}, posts = []) => { - const filteredPosts = posts.filter(post => { - return post.rule === rule.id; + return posts.filter(post => { + return post.rule.id === rule.id; }); - - const filteredData = filteredPosts.map(post => ({ ...post, rule: { ...rule } })); - - return filteredData.sort(sortOrdering); }; export const filterPostsByCategory = (category = {}, rules = [], posts = []) => { @@ -36,24 +15,13 @@ export const filterPostsByCategory = (category = {}, rules = [], posts = []) => return rule.category === category.id; }); - const filteredData = filteredRules.map(rule => { - const filteredPosts = posts.filter(post => { - return post.rule === rule.id; - }); + const ruleIds = filteredRules.map(rule => rule.id); - return filteredPosts.map(post => ({ ...post, rule: { ...rule } })); - }); - - const sortedPosts = [...filteredData.flat()].sort(sortOrdering); - - return sortedPosts; + return [...posts].filter(post => ruleIds.includes(post.rule.id)); }; export const filterPostsBySaved = (rules = [], posts = []) => { - const filteredPosts = posts.filter(post => post.saved); - return filteredPosts - .map(post => ({ ...post, rule: { ...rules.find(rule => rule.id === post.rule) } })) - .sort(savedOrdering); + return [...posts].filter(post => post.saved); }; export const filterPosts = state => { diff --git a/src/newsreader/js/pages/homepage/reducers/posts.js b/src/newsreader/js/pages/homepage/reducers/posts.js index d422ca7..cdd44fa 100644 --- a/src/newsreader/js/pages/homepage/reducers/posts.js +++ b/src/newsreader/js/pages/homepage/reducers/posts.js @@ -12,56 +12,78 @@ import { } from '../actions/posts.js'; import { MARK_SECTION_READ } from '../actions/selected.js'; -const defaultState = { items: {}, isFetching: false, isUpdating: false }; +const sortOrdering = (firstPost, secondPost) => { + const dateOrdering = + new Date(firstPost.publicationDate) < new Date(secondPost.publicationDate); + + if (firstPost.read && !secondPost.read) { + return 1; + } else if (secondPost.read && !firstPost.read) { + return -1; + } + + return dateOrdering; +}; + +const defaultState = { items: [], isFetching: false, isUpdating: false }; export const posts = (state = { ...defaultState }, action) => { switch (action.type) { case REQUEST_POSTS: return { ...state, isFetching: true }; - // TODO: save post per category/rule in corresponding reducers case RECEIVE_POST: + const postIndex = state.items.findIndex(post => post.id === action.post.id); + let newItems = [...state.items]; + + if (postIndex >= 0) { + newItems.splice(postIndex, 1, { ...action.post }); + } else { + newItems = [...state.items, { ...action.post }]; + } + + newItems.sort(sortOrdering); + return { ...state, - items: { ...state.items, [action.post.id]: { ...action.post } }, + items: newItems, }; - // TODO: save posts per category/rule in corresponding reducers case RECEIVE_POSTS: - const receivedItems = objectsFromArray(action.posts, 'id'); + const receivedPosts = objectsFromArray(action.posts, 'id'); + + let mergedPosts = { ...objectsFromArray(state.items, 'id'), ...receivedPosts }; + let sortedPosts = Object.values(mergedPosts).sort(sortOrdering); return { ...state, isFetching: false, - items: { ...state.items, ...receivedItems }, + items: sortedPosts, }; case MARK_SECTION_READ: - const updatedPosts = {}; - let relatedPosts = []; + let posts = []; + // TODO: test these cases switch (action.section.type) { case CATEGORY_TYPE: - relatedPosts = Object.values({ ...state.items }).filter(post => { - return post.rule in { ...action.section.rules }; + posts = [...state.items].forEach(post => { + if (!(post.rule.id in { ...action.section.rules })) return; + + post.read = true; }); break; case RULE_TYPE: - relatedPosts = Object.values({ ...state.items }).filter(post => { - return post.rule === action.section.id; + posts = [...state.items].forEach(post => { + if (post.rule.id != action.section.id) return; + + post.read = true; }); break; } - relatedPosts.forEach(post => { - updatedPosts[post.id] = { ...post, read: true }; - }); - return { ...state, - items: { - ...state.items, - ...updatedPosts, - }, + items: posts, }; case MARKING_POST: return { ...state, isUpdating: true }; diff --git a/src/newsreader/news/core/serializers.py b/src/newsreader/news/core/serializers.py index 38619a1..d77469a 100644 --- a/src/newsreader/news/core/serializers.py +++ b/src/newsreader/news/core/serializers.py @@ -1,5 +1,6 @@ from rest_framework import serializers +from newsreader.news.collection.serializers import RuleSerializer from newsreader.news.core.models import Category, Post @@ -9,6 +10,8 @@ class PostSerializer(serializers.ModelSerializer): ) remoteIdentifier = serializers.CharField(source="remote_identifier", required=False) + rule = RuleSerializer() + class Meta: model = Post fields = ( From c12ba968b8d0c688ba682e5c505a62b2b21f3e56 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Mon, 25 Sep 2023 20:27:52 +0200 Subject: [PATCH 3/4] Fix/update tests --- .../news/collection/tests/endpoints/rule/list/tests.py | 2 +- src/newsreader/news/core/serializers.py | 2 +- .../news/core/tests/endpoints/category/list/tests.py | 4 ++-- src/newsreader/news/core/tests/endpoints/post/detail/tests.py | 1 + 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/newsreader/news/collection/tests/endpoints/rule/list/tests.py b/src/newsreader/news/collection/tests/endpoints/rule/list/tests.py index 5a34dbc..2ae3810 100644 --- a/src/newsreader/news/collection/tests/endpoints/rule/list/tests.py +++ b/src/newsreader/news/collection/tests/endpoints/rule/list/tests.py @@ -202,7 +202,7 @@ class NestedRuleListViewTestCase(TestCase): for post in data["results"]: with self.subTest(post=post): - self.assertEqual(post["rule"], rule.pk) + self.assertEqual(post["rule"]["id"], rule.pk) def test_unread_posts(self): rule = FeedFactory.create(user=self.user) diff --git a/src/newsreader/news/core/serializers.py b/src/newsreader/news/core/serializers.py index d77469a..c375dde 100644 --- a/src/newsreader/news/core/serializers.py +++ b/src/newsreader/news/core/serializers.py @@ -10,7 +10,7 @@ class PostSerializer(serializers.ModelSerializer): ) remoteIdentifier = serializers.CharField(source="remote_identifier", required=False) - rule = RuleSerializer() + rule = RuleSerializer(read_only=True) class Meta: model = Post diff --git a/src/newsreader/news/core/tests/endpoints/category/list/tests.py b/src/newsreader/news/core/tests/endpoints/category/list/tests.py index c822950..65f4433 100644 --- a/src/newsreader/news/core/tests/endpoints/category/list/tests.py +++ b/src/newsreader/news/core/tests/endpoints/category/list/tests.py @@ -510,8 +510,8 @@ class NestedCategoryPostView(TestCase): self.assertEqual(response.status_code, 200) self.assertEqual(len(data["results"]), 2) - self.assertEqual(posts[0]["rule"], guardian_rule.pk) - self.assertEqual(posts[1]["rule"], guardian_rule.pk) + self.assertEqual(posts[0]["rule"]["id"], guardian_rule.pk) + self.assertEqual(posts[1]["rule"]["id"], guardian_rule.pk) def test_unread_posts(self): category = CategoryFactory.create(user=self.user) diff --git a/src/newsreader/news/core/tests/endpoints/post/detail/tests.py b/src/newsreader/news/core/tests/endpoints/post/detail/tests.py index 92444cc..295f92f 100644 --- a/src/newsreader/news/core/tests/endpoints/post/detail/tests.py +++ b/src/newsreader/news/core/tests/endpoints/post/detail/tests.py @@ -111,6 +111,7 @@ class PostDetailViewTestCase(TestCase): data=json.dumps({"title": "This title is very accurate"}), content_type="application/json", ) + data = response.json() self.assertEqual(response.status_code, 200) From 3de915bb64c5096cbf20b8e8e99b4e41cc314ee3 Mon Sep 17 00:00:00 2001 From: Sonny Bakker Date: Wed, 27 Sep 2023 22:43:12 +0200 Subject: [PATCH 4/4] Update post reducer & tests --- .../js/pages/homepage/reducers/posts.js | 9 +- .../js/tests/homepage/reducers/post.test.js | 113 +++++++++++++----- 2 files changed, 90 insertions(+), 32 deletions(-) diff --git a/src/newsreader/js/pages/homepage/reducers/posts.js b/src/newsreader/js/pages/homepage/reducers/posts.js index cdd44fa..2c1ff86 100644 --- a/src/newsreader/js/pages/homepage/reducers/posts.js +++ b/src/newsreader/js/pages/homepage/reducers/posts.js @@ -61,10 +61,11 @@ export const posts = (state = { ...defaultState }, action) => { case MARK_SECTION_READ: let posts = []; - // TODO: test these cases switch (action.section.type) { case CATEGORY_TYPE: - posts = [...state.items].forEach(post => { + posts = [...state.items]; + + posts.forEach(post => { if (!(post.rule.id in { ...action.section.rules })) return; post.read = true; @@ -72,7 +73,9 @@ export const posts = (state = { ...defaultState }, action) => { break; case RULE_TYPE: - posts = [...state.items].forEach(post => { + posts = [...state.items]; + + posts.forEach(post => { if (post.rule.id != action.section.id) return; post.read = true; diff --git a/src/newsreader/js/tests/homepage/reducers/post.test.js b/src/newsreader/js/tests/homepage/reducers/post.test.js index adb8983..249a9f1 100644 --- a/src/newsreader/js/tests/homepage/reducers/post.test.js +++ b/src/newsreader/js/tests/homepage/reducers/post.test.js @@ -6,7 +6,7 @@ import * as actions from '../../../pages/homepage/actions/posts.js'; import * as selectedActions from '../../../pages/homepage/actions/selected.js'; import * as constants from '../../../pages/homepage/constants.js'; -const defaultState = { items: {}, isFetching: false }; +const defaultState = { items: [], isFetching: false }; describe('post actions', () => { it('should return state after requesting posts', () => { @@ -42,7 +42,7 @@ describe('post actions', () => { ...defaultState, isFetching: false, isUpdating: false, - items: { [post.id]: post }, + items: [post], }; expect(reducer(undefined, action)).toEqual(expectedState); @@ -85,12 +85,11 @@ describe('post actions', () => { posts, }; - const expectedPosts = objectsFromArray(posts, 'id'); const expectedState = { ...defaultState, isFetching: false, isUpdating: false, - items: expectedPosts, + items: posts, }; expect(reducer(undefined, action)).toEqual(expectedState); @@ -108,7 +107,14 @@ describe('post actions', () => { author: 'Kyle Orland', publicationDate: '2020-01-24T19:50:12Z', url: 'https://arstechnica.com/?p=1648607', - rule: 5, + rule: { + id: 5, + name: 'Ars Technica', + url: 'http://feeds.arstechnica.com/arstechnica/index?fmt=xml', + favicon: 'https://cdn.arstechnica.net/favicon.ico', + category: 9, + unread: 544, + }, read: false, }, 2141: { @@ -120,7 +126,14 @@ describe('post actions', () => { author: 'WIRED', publicationDate: '2020-01-25T11:06:46Z', url: 'https://arstechnica.com/?p=1648757', - rule: 5, + rule: { + id: 5, + name: 'Ars Technica', + url: 'http://feeds.arstechnica.com/arstechnica/index?fmt=xml', + favicon: 'https://cdn.arstechnica.net/favicon.ico', + category: 9, + unread: 544, + }, read: false, }, 4637: { @@ -132,7 +145,9 @@ describe('post actions', () => { author: null, publicationDate: '2020-01-29T19:08:25Z', url: 'https://www.bbc.co.uk/news/world-asia-china-51299195', - rule: 4, + rule: { + id: 4, + }, read: false, saved: false, }, @@ -145,7 +160,9 @@ describe('post actions', () => { author: null, publicationDate: '2020-01-29T18:27:56Z', url: 'https://www.bbc.co.uk/news/world-europe-51294305', - rule: 4, + rule: { + id: 4, + }, read: false, saved: false, }, @@ -165,16 +182,17 @@ describe('post actions', () => { section: { ...rule, type: constants.RULE_TYPE }, }; - const state = { ...defaultState, items: { ...posts } }; + const state = { ...defaultState, items: Object.values(posts) }; const expectedState = { ...defaultState, isFetching: false, - items: { - ...posts, - 2067: { ...posts[2067], read: true }, - 2141: { ...posts[2141], read: true }, - }, + items: [ + { ...posts[2067], read: true }, + { ...posts[2141], read: true }, + { ...posts[4637] }, + { ...posts[4638] }, + ], }; expect(reducer(state, action)).toEqual(expectedState); @@ -192,7 +210,15 @@ describe('post actions', () => { author: 'Kyle Orland', publicationDate: '2020-01-24T19:50:12Z', url: 'https://arstechnica.com/?p=1648607', - rule: 5, + rule: { + id: 5, + name: 'BBC', + url: 'http://feeds.bbci.co.uk/news/world/rss.xml', + favicon: + 'https://m.files.bbci.co.uk/modules/bbc-morph-news-waf-page-meta/2.5.2/apple-touch-icon-57x57-precomposed.png', + category: 8, + unread: 632, + }, read: false, saved: false, }, @@ -205,7 +231,15 @@ describe('post actions', () => { author: 'WIRED', publicationDate: '2020-01-25T11:06:46Z', url: 'https://arstechnica.com/?p=1648757', - rule: 5, + rule: { + id: 5, + name: 'BBC', + url: 'http://feeds.bbci.co.uk/news/world/rss.xml', + favicon: + 'https://m.files.bbci.co.uk/modules/bbc-morph-news-waf-page-meta/2.5.2/apple-touch-icon-57x57-precomposed.png', + category: 8, + unread: 632, + }, read: false, saved: false, }, @@ -218,7 +252,15 @@ describe('post actions', () => { author: null, publicationDate: '2020-01-29T19:08:25Z', url: 'https://www.bbc.co.uk/news/world-asia-china-51299195', - rule: 4, + rule: { + id: 4, + name: 'BBC', + url: 'http://feeds.bbci.co.uk/news/world/rss.xml', + favicon: + 'https://m.files.bbci.co.uk/modules/bbc-morph-news-waf-page-meta/2.5.2/apple-touch-icon-57x57-precomposed.png', + category: 8, + unread: 321, + }, read: false, saved: false, }, @@ -231,7 +273,15 @@ describe('post actions', () => { author: null, publicationDate: '2020-01-29T18:27:56Z', url: 'https://www.bbc.co.uk/news/world-europe-51294305', - rule: 4, + rule: { + id: 4, + name: 'BBC', + url: 'http://feeds.bbci.co.uk/news/world/rss.xml', + favicon: + 'https://m.files.bbci.co.uk/modules/bbc-morph-news-waf-page-meta/2.5.2/apple-touch-icon-57x57-precomposed.png', + category: 8, + unread: 321, + }, read: false, saved: false, }, @@ -245,7 +295,9 @@ describe('post actions', () => { publicationDate: '2020-01-29T19:03:01Z', url: 'https://tweakers.net/nieuws/162878/analyse-nintendo-verdiende-miljard-dollar-aan-mobiele-games.html', - rule: 7, + rule: { + id: 7, + }, read: false, saved: false, }, @@ -259,7 +311,9 @@ describe('post actions', () => { publicationDate: '2020-01-29T16:29:40Z', url: 'https://tweakers.net/nieuws/162870/samsung-kondigt-eerste-tablet-met-5g-aan.html', - rule: 7, + rule: { + id: 7, + }, read: false, saved: false, }, @@ -276,7 +330,7 @@ describe('post actions', () => { unread: 321, }, 5: { - id: 4, + id: 5, name: 'BBC', url: 'http://feeds.bbci.co.uk/news/world/rss.xml', favicon: @@ -301,18 +355,19 @@ describe('post actions', () => { }, }; - const state = { ...defaultState, items: { ...posts } }; + const state = { ...defaultState, items: Object.values(posts) }; const expectedState = { ...defaultState, isFetching: false, - items: { - ...posts, - 2067: { ...posts[2067], read: true }, - 2141: { ...posts[2141], read: true }, - 4637: { ...posts[4637], read: true }, - 4638: { ...posts[4638], read: true }, - }, + items: [ + { ...posts[2067], read: true }, + { ...posts[2141], read: true }, + { ...posts[4589] }, + { ...posts[4594] }, + { ...posts[4637], read: true }, + { ...posts[4638], read: true }, + ], }; expect(reducer(state, action)).toEqual(expectedState);